| Message ID | 1466404822-2540-1-git-send-email-wsa@the-dreams.de (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Wolfram, On Mon, Jun 20, 2016 at 8:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > On group configuration, bail out if setting one of the individual pins > fails. We don't need to roll-back, the pinctrl core will do this for us. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > Tested on a Lager and Salvator-X without problems. Needs probably more testing > on various HW to avoid regressions? Your patch causes the display to fail on r8a7740/armadillo. Disabling the error propagation and adding more debug prints reveals: +sh_pfc_pinconf_validate: pin 58: 15 not supported +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for pin 0 of group lcd0_data24_0 +sh_pfc_pinconf_validate: pin 57: 15 not supported +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for pin 1 of group lcd0_data24_0 +sh_pfc_pinconf_validate: pin 56: 15 not supported +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for pin 2 of group lcd0_data24_0 ... 15 = PIN_CONFIG_OUTPUT No idea why this is set for all pins of the lcd0_data24_0 group (and not for any other pin or group on any other SoC). 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
> > On group configuration, bail out if setting one of the individual pins > > fails. We don't need to roll-back, the pinctrl core will do this for us. > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > --- > > > > Tested on a Lager and Salvator-X without problems. Needs probably more testing > > on various HW to avoid regressions? > > Your patch causes the display to fail on r8a7740/armadillo. I would have been surprised if this patch didn't uncover some hidden issues. > 15 = PIN_CONFIG_OUTPUT > > No idea why this is set for all pins of the lcd0_data24_0 group (and not for any > other pin or group on any other SoC). Me neither. Sadly no bandwith here to dig into it.
Hi Wolfram, On Tue, Jun 21, 2016 at 3:11 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Mon, Jun 20, 2016 at 8:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote: >> On group configuration, bail out if setting one of the individual pins >> fails. We don't need to roll-back, the pinctrl core will do this for us. >> >> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> >> --- >> >> Tested on a Lager and Salvator-X without problems. Needs probably more testing >> on various HW to avoid regressions? > > Your patch causes the display to fail on r8a7740/armadillo. > > Disabling the error propagation and adding more debug prints reveals: > > +sh_pfc_pinconf_validate: pin 58: 15 not supported > +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed > +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for > pin 0 of group lcd0_data24_0 > +sh_pfc_pinconf_validate: pin 57: 15 not supported > +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed > +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for > pin 1 of group lcd0_data24_0 > +sh_pfc_pinconf_validate: pin 56: 15 not supported > +sh_pfc_pinconf_set:694: config 0: sh_pfc_pinconf_validate() failed > +sh_pfc_pinconf_group_set:781: sh_pfc_pinconf_set() failed -524 for > pin 2 of group lcd0_data24_0 > ... > > 15 = PIN_CONFIG_OUTPUT > > No idea why this is set for all pins of the lcd0_data24_0 group (and not for any > other pin or group on any other SoC). This is caused by the "output-high" property for the GPIO hog. Due to both lcd0 pinmux and GPIO hog being part of the same device node, "output-high" is also applied to the lcd0 pins, which fails. RFC fix in "[PATCH/RFC] ARM: dts: armadillo800eva: Split LCD mux and gpio" (https://patchwork.kernel.org/patch/9745831/)/ 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
Hi Geert, > This is caused by the "output-high" property for the GPIO hog. > Due to both lcd0 pinmux and GPIO hog being part of the same device node, > "output-high" is also applied to the lcd0 pins, which fails. > > RFC fix in "[PATCH/RFC] ARM: dts: armadillo800eva: Split LCD mux and gpio" > (https://patchwork.kernel.org/patch/9745831/)/ Cool, thanks! So, if this issue could get resolved, there are chances to revive the original patch? Regards, Wolfram
Hi Wolfram, On Wed, May 24, 2017 at 2:21 PM, Wolfram Sang <wsa@the-dreams.de> wrote: >> This is caused by the "output-high" property for the GPIO hog. >> Due to both lcd0 pinmux and GPIO hog being part of the same device node, >> "output-high" is also applied to the lcd0 pins, which fails. >> >> RFC fix in "[PATCH/RFC] ARM: dts: armadillo800eva: Split LCD mux and gpio" >> (https://patchwork.kernel.org/patch/9745831/)/ > > Cool, thanks! So, if this issue could get resolved, there are chances to > revive the original patch? Yep, that's the plan. 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
On Mon, Jun 20, 2016 at 8:40 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > From: Wolfram Sang <wsa+renesas@sang-engineering.com> > > On group configuration, bail out if setting one of the individual pins > fails. We don't need to roll-back, the pinctrl core will do this for us. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > Tested on a Lager and Salvator-X without problems. Needs probably more testing > on various HW to avoid regressions? Queueing in sh-pfc-for-v4.14, now commit 13132b3f44d36009 ("ARM: dts: armadillo800eva: Split LCD mux and gpio") is in v4.13-rc1. 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 --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c index d4e65bc7dacd67..c52ca5873974f3 100644 --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -739,13 +739,16 @@ static int sh_pfc_pinconf_group_set(struct pinctrl_dev *pctldev, unsigned group, struct sh_pfc_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev); const unsigned int *pins; unsigned int num_pins; - unsigned int i; + unsigned int i, ret; pins = pmx->pfc->info->groups[group].pins; num_pins = pmx->pfc->info->groups[group].nr_pins; - for (i = 0; i < num_pins; ++i) - sh_pfc_pinconf_set(pctldev, pins[i], configs, num_configs); + for (i = 0; i < num_pins; ++i) { + ret = sh_pfc_pinconf_set(pctldev, pins[i], configs, num_configs); + if (ret) + return ret; + } return 0; }