Message ID | 2925145.E3lLFJfWJW@wasted.cogentembedded.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Hi Sergei, Thank you for the patch. On Friday 26 June 2015 01:40:56 Sergei Shtylyov wrote: > The PFC driver causes the kernel to hang on the R-Car gen2 SoC based boards > when the CPU_ALL_PORT() macro is fixed to reflect the reality, i.e. when > the GPIO space becomes actually sparse. This happens because the > _GP_GPIO() macro includes an indexed initializer which causes the "holes" > (array entries filled with all 0s) between the groups of the existing > GPIOs; and the driver can't cope with that. There seems to be no reason > to use the indexed initializer, so we can remove the index specifier and so > avoid the "holes". > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> I initially thought that this patch looked too good to be true. The fix is so simple, there must have been a reason why _GP_GPIO used indexed initializers. I then tried to find that reason and failed. I still feel that this is too simple to be true, but I have no objective reason to push back, so Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> for the whole series, provided you have tested it, and paid attention to pins after the holes. > --- > Changes in version 3: > - new patch. > > drivers/pinctrl/sh-pfc/sh_pfc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Index: linux-pinctrl/drivers/pinctrl/sh-pfc/sh_pfc.h > =================================================================== > --- linux-pinctrl.orig/drivers/pinctrl/sh-pfc/sh_pfc.h > +++ linux-pinctrl/drivers/pinctrl/sh-pfc/sh_pfc.h > @@ -224,7 +224,7 @@ struct sh_pfc_soc_info { > > /* PINMUX_GPIO_GP_ALL - Expand to a list of sh_pfc_pin entries */ > #define _GP_GPIO(bank, _pin, _name, sfx) \ > - [(bank * 32) + _pin] = { \ > + { \ > .pin = (bank * 32) + _pin, \ > .name = __stringify(_name), \ > .enum_id = _name##_DATA, \
On Tue, Jul 7, 2015 at 7:39 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Friday 26 June 2015 01:40:56 Sergei Shtylyov wrote: >> The PFC driver causes the kernel to hang on the R-Car gen2 SoC based boards >> when the CPU_ALL_PORT() macro is fixed to reflect the reality, i.e. when >> the GPIO space becomes actually sparse. This happens because the >> _GP_GPIO() macro includes an indexed initializer which causes the "holes" >> (array entries filled with all 0s) between the groups of the existing >> GPIOs; and the driver can't cope with that. There seems to be no reason >> to use the indexed initializer, so we can remove the index specifier and so >> avoid the "holes". >> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > I initially thought that this patch looked too good to be true. The fix is so > simple, there must have been a reason why _GP_GPIO used indexed initializers. > I then tried to find that reason and failed. > > I still feel that this is too simple to be true, but I have no objective > reason to push back, so :-) > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > for the whole series, provided you have tested it, and paid attention to pins > after the holes. On r8a7791/koelsch, the switches (gpio banks 5 and 7) and LEDs (gpio bank 2) still work, so Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 26, 2015 at 12:40 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > The PFC driver causes the kernel to hang on the R-Car gen2 SoC based boards > when the CPU_ALL_PORT() macro is fixed to reflect the reality, i.e. when the > GPIO space becomes actually sparse. This happens because the _GP_GPIO() macro > includes an indexed initializer which causes the "holes" (array entries filled > with all 0s) between the groups of the existing GPIOs; and the driver can't > cope with that. There seems to be no reason to use the indexed initializer, > so we can remove the index specifier and so avoid the "holes". > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Patch applied for fixes. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-pinctrl/drivers/pinctrl/sh-pfc/sh_pfc.h =================================================================== --- linux-pinctrl.orig/drivers/pinctrl/sh-pfc/sh_pfc.h +++ linux-pinctrl/drivers/pinctrl/sh-pfc/sh_pfc.h @@ -224,7 +224,7 @@ struct sh_pfc_soc_info { /* PINMUX_GPIO_GP_ALL - Expand to a list of sh_pfc_pin entries */ #define _GP_GPIO(bank, _pin, _name, sfx) \ - [(bank * 32) + _pin] = { \ + { \ .pin = (bank * 32) + _pin, \ .name = __stringify(_name), \ .enum_id = _name##_DATA, \
The PFC driver causes the kernel to hang on the R-Car gen2 SoC based boards when the CPU_ALL_PORT() macro is fixed to reflect the reality, i.e. when the GPIO space becomes actually sparse. This happens because the _GP_GPIO() macro includes an indexed initializer which causes the "holes" (array entries filled with all 0s) between the groups of the existing GPIOs; and the driver can't cope with that. There seems to be no reason to use the indexed initializer, so we can remove the index specifier and so avoid the "holes". Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- Changes in version 3: - new patch. drivers/pinctrl/sh-pfc/sh_pfc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html