diff mbox series

[RFC,2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"

Message ID 1562576868-8124-3-git-send-email-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series treewide: modify sh-pfc and add support pwm duty zero | expand

Commit Message

Yoshihiro Shimoda July 8, 2019, 9:07 a.m. UTC
The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
Now if we fix the cfg->type condition, it gets worse because:
 - Some drivers might be deferred so that .set_mux() will be called
   multiple times.
 - In such the case, the sh-pfc driver returns -EBUSY even if
   the group is the same, and then that driver fails to probe.

Since the pinctrl subsystem already has such conditions according
to @set_mux and @gpio_request_enable, this patch just remove
the incomplete flag from sh-pfc/pinctrl.c.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pinctrl/sh-pfc/pinctrl.c | 26 --------------------------
 1 file changed, 26 deletions(-)

Comments

Linus Walleij July 28, 2019, 11:02 p.m. UTC | #1
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:

> The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> Now if we fix the cfg->type condition, it gets worse because:
>  - Some drivers might be deferred so that .set_mux() will be called
>    multiple times.
>  - In such the case, the sh-pfc driver returns -EBUSY even if
>    the group is the same, and then that driver fails to probe.
>
> Since the pinctrl subsystem already has such conditions according
> to @set_mux and @gpio_request_enable, this patch just remove
> the incomplete flag from sh-pfc/pinctrl.c.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

This looks like it should have a Fixes: tag as well.

Geert will decide what to do with this.

Can all the pinctrl patches be applied independently of the other
changes so Geert can apply and send me those patches in his pull
requests?

Yours,
Linus Walleij
Yoshihiro Shimoda July 29, 2019, 5:16 a.m. UTC | #2
Hi Linus, Geert,

> From: Linus Walleij, Sent: Monday, July 29, 2019 8:02 AM
> 
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> 
> > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > Now if we fix the cfg->type condition, it gets worse because:
> >  - Some drivers might be deferred so that .set_mux() will be called
> >    multiple times.
> >  - In such the case, the sh-pfc driver returns -EBUSY even if
> >    the group is the same, and then that driver fails to probe.
> >
> > Since the pinctrl subsystem already has such conditions according
> > to @set_mux and @gpio_request_enable, this patch just remove
> > the incomplete flag from sh-pfc/pinctrl.c.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> This looks like it should have a Fixes: tag as well.

I got it. The Fixes tag should be:

Fixes: c58d9c1b26e3 ("sh-pfc: Implement generic pinconf support")

> Geert will decide what to do with this.

I got it.

> Can all the pinctrl patches be applied independently of the other
> changes so Geert can apply and send me those patches in his pull
> requests?

The pinctrl patches (1/7 through 3/7) can be applied on next-20190726
so I think Geert can apply these patches into his repo.

Geert, if I should resend the pinctrl patches, please let me know!

Best regards,
Yoshihiro Shimoda

> Yours,
> Linus Walleij
Geert Uytterhoeven Aug. 6, 2019, 8:49 a.m. UTC | #3
Hi Shimoda-san,

On Mon, Jul 29, 2019 at 7:16 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> > From: Linus Walleij, Sent: Monday, July 29, 2019 8:02 AM
> >
> > On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> > <yoshihiro.shimoda.uh@renesas.com> wrote:
> >
> > > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > > Now if we fix the cfg->type condition, it gets worse because:
> > >  - Some drivers might be deferred so that .set_mux() will be called
> > >    multiple times.
> > >  - In such the case, the sh-pfc driver returns -EBUSY even if
> > >    the group is the same, and then that driver fails to probe.
> > >
> > > Since the pinctrl subsystem already has such conditions according
> > > to @set_mux and @gpio_request_enable, this patch just remove
> > > the incomplete flag from sh-pfc/pinctrl.c.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> >
> > This looks like it should have a Fixes: tag as well.
>
> I got it. The Fixes tag should be:
>
> Fixes: c58d9c1b26e3 ("sh-pfc: Implement generic pinconf support")

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > Geert will decide what to do with this.
>
> I got it.
>
> > Can all the pinctrl patches be applied independently of the other
> > changes so Geert can apply and send me those patches in his pull
> > requests?
>
> The pinctrl patches (1/7 through 3/7) can be applied on next-20190726
> so I think Geert can apply these patches into his repo.

Looks mostly OK to me (I have some comments on 3/7).
I'll apply it to my local tree, so it will receive some testing on all
boards I have.

Thanks!

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Aug. 6, 2019, 9:23 a.m. UTC | #4
Hi Shimoda-san,

On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> Now if we fix the cfg->type condition, it gets worse because:
>  - Some drivers might be deferred so that .set_mux() will be called
>    multiple times.
>  - In such the case, the sh-pfc driver returns -EBUSY even if
>    the group is the same, and then that driver fails to probe.
>
> Since the pinctrl subsystem already has such conditions according
> to @set_mux and @gpio_request_enable, this patch just remove
> the incomplete flag from sh-pfc/pinctrl.c.

Do we need to set sh_pfc_pinmux_ops.strict = true?

Gr{oetje,eeting}s,

                        Geert
Yoshihiro Shimoda Aug. 6, 2019, 11:48 a.m. UTC | #5
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, August 6, 2019 6:24 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> > support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> > sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> > Now if we fix the cfg->type condition, it gets worse because:
> >  - Some drivers might be deferred so that .set_mux() will be called
> >    multiple times.
> >  - In such the case, the sh-pfc driver returns -EBUSY even if
> >    the group is the same, and then that driver fails to probe.
> >
> > Since the pinctrl subsystem already has such conditions according
> > to @set_mux and @gpio_request_enable, this patch just remove
> > the incomplete flag from sh-pfc/pinctrl.c.
> 
> Do we need to set sh_pfc_pinmux_ops.strict = true?

If the .strict = true, the final pwm patch on this series failed with the following error:

[   11.453716] sh-pfc e6060000.pin-controller: pin GP_2_7 already requested by e6e31000.pwm; cannot claim for e6052000.gpio:459

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
diff mbox series

Patch

diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c
index 157b257..7e5aca2 100644
--- a/drivers/pinctrl/sh-pfc/pinctrl.c
+++ b/drivers/pinctrl/sh-pfc/pinctrl.c
@@ -26,7 +26,6 @@ 
 #include "../pinconf.h"
 
 struct sh_pfc_pin_config {
-	u32 type;
 	bool mux_set;
 	bool gpio_enabled;
 };
@@ -354,16 +353,6 @@  static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector,
 	spin_lock_irqsave(&pfc->lock, flags);
 
 	for (i = 0; i < grp->nr_pins; ++i) {
-		int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]);
-		struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
-
-		if (cfg->type != PINMUX_TYPE_NONE) {
-			ret = -EBUSY;
-			goto done;
-		}
-	}
-
-	for (i = 0; i < grp->nr_pins; ++i) {
 		ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION);
 		if (ret < 0)
 			goto done;
@@ -395,14 +384,6 @@  static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	spin_lock_irqsave(&pfc->lock, flags);
 
-	if (cfg->type != PINMUX_TYPE_NONE) {
-		dev_err(pfc->dev,
-			"Pin %u is busy, can't configure it as GPIO.\n",
-			offset);
-		ret = -EBUSY;
-		goto done;
-	}
-
 	if (!pfc->gpio) {
 		/* If GPIOs are handled externally the pin mux type need to be
 		 * set to GPIO here.
@@ -414,7 +395,6 @@  static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev,
 			goto done;
 	}
 
-	cfg->type = PINMUX_TYPE_GPIO;
 	cfg->gpio_enabled = true;
 
 	ret = 0;
@@ -436,7 +416,6 @@  static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev,
 	unsigned long flags;
 
 	spin_lock_irqsave(&pfc->lock, flags);
-	cfg->type = PINMUX_TYPE_NONE;
 	cfg->gpio_enabled = false;
 	spin_unlock_irqrestore(&pfc->lock, flags);
 }
@@ -450,7 +429,6 @@  static int sh_pfc_gpio_set_direction(struct pinctrl_dev *pctldev,
 	int new_type = input ? PINMUX_TYPE_INPUT : PINMUX_TYPE_OUTPUT;
 	int idx = sh_pfc_get_pin_index(pfc, offset);
 	const struct sh_pfc_pin *pin = &pfc->info->pins[idx];
-	struct sh_pfc_pin_config *cfg = &pmx->configs[idx];
 	unsigned long flags;
 	unsigned int dir;
 	int ret;
@@ -470,8 +448,6 @@  static int sh_pfc_gpio_set_direction(struct pinctrl_dev *pctldev,
 	if (ret < 0)
 		goto done;
 
-	cfg->type = new_type;
-
 done:
 	spin_unlock_irqrestore(&pfc->lock, flags);
 	return ret;
@@ -794,13 +770,11 @@  static int sh_pfc_map_pins(struct sh_pfc *pfc, struct sh_pfc_pinctrl *pmx)
 
 	for (i = 0; i < pfc->info->nr_pins; ++i) {
 		const struct sh_pfc_pin *info = &pfc->info->pins[i];
-		struct sh_pfc_pin_config *cfg = &pmx->configs[i];
 		struct pinctrl_pin_desc *pin = &pmx->pins[i];
 
 		/* If the pin number is equal to -1 all pins are considered */
 		pin->number = info->pin != (u16)-1 ? info->pin : i;
 		pin->name = info->name;
-		cfg->type = PINMUX_TYPE_NONE;
 	}
 
 	return 0;