| Message ID | 1447245413-32209-1-git-send-email-vz@mleia.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote: > Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes > OOPS on boot on LPC32xx boards: > > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 > Hardware name: LPC32XX SoC (Flattened Device Tree) > task: c381baa0 ti: c381e000 task.ti: c381e000 > PC is at strcmp+0x10/0x40 > LR is at gpiochip_add+0x3d0/0x4d4 > pc : [<>] lr : [<>] psr: a0000093 > sp : c381fd60 ip : c381fd70 fp : c381fd6c > > [snip] > > Backtrace: > [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) > [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) > [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) > [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) > [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) > [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) > [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) > [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) > [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) > [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) > [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) > [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) > [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) > [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) > [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) > > This is caused by the fact that at the moment some GPIO names are set > to NULL, there is a hole in linear representation of one GPI bank, see > drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. > > The same problem most probably affects also gpio-cs5535.c, see > cs5535_gpio_names[]. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > --- > Linus, Markus, > > I understand that LPC32xx GPIO driver is ugly (hopefully this > will be changed in future), but please account this problem. > > I don't ask for inclusion of this change, because my own analysis is > not done completely. > > Also please review, the same problem probably affects gpio-cs5535.c > as well, see cs5535_gpio_names[]. Thanks for this hint. But I think these patches for gpio_name_to_desc won't get mainline any time soon as this depends on the chardev interface and how it will look like. Best Regards, Markus > > drivers/gpio/gpiolib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index a18f00f..2a91f32 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) > for (i = 0; i != chip->ngpio; ++i) { > struct gpio_desc *gpio = &chip->desc[i]; > > - if (!gpio->name) > + if (!gpio->name || !name) > continue; > > if (!strcmp(gpio->name, name)) { >
Hi Markus, On 17.11.2015 11:24, Markus Pargmann wrote: > Hi, > > On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote: >> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes >> OOPS on boot on LPC32xx boards: >> >> Unable to handle kernel NULL pointer dereference at virtual address 00000000 >> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 >> Hardware name: LPC32XX SoC (Flattened Device Tree) >> task: c381baa0 ti: c381e000 task.ti: c381e000 >> PC is at strcmp+0x10/0x40 >> LR is at gpiochip_add+0x3d0/0x4d4 >> pc : [<>] lr : [<>] psr: a0000093 >> sp : c381fd60 ip : c381fd70 fp : c381fd6c >> >> [snip] >> >> Backtrace: >> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) >> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) >> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) >> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) >> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) >> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) >> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) >> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) >> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) >> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) >> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) >> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) >> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) >> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) >> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) >> >> This is caused by the fact that at the moment some GPIO names are set >> to NULL, there is a hole in linear representation of one GPI bank, see >> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. >> >> The same problem most probably affects also gpio-cs5535.c, see >> cs5535_gpio_names[]. >> >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> >> --- >> Linus, Markus, >> >> I understand that LPC32xx GPIO driver is ugly (hopefully this >> will be changed in future), but please account this problem. >> >> I don't ask for inclusion of this change, because my own analysis is >> not done completely. >> >> Also please review, the same problem probably affects gpio-cs5535.c >> as well, see cs5535_gpio_names[]. > > Thanks for this hint. But I think these patches for gpio_name_to_desc won't get > mainline any time soon as this depends on the chardev interface and how it will > look like. > sorry for misunderstanding, what do mean by "these patches"? I see two options here, one is to fix or revert your c0017ed71966 due to caused regressions, another one is to update affected GPIO drivers removing discontinuity in GPIO enumeration inside a bank. With best wishes, Vladimir >> >> drivers/gpio/gpiolib.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c >> index a18f00f..2a91f32 100644 >> --- a/drivers/gpio/gpiolib.c >> +++ b/drivers/gpio/gpiolib.c >> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) >> for (i = 0; i != chip->ngpio; ++i) { >> struct gpio_desc *gpio = &chip->desc[i]; >> >> - if (!gpio->name) >> + if (!gpio->name || !name) >> continue; >> >> if (!strcmp(gpio->name, name)) { >> > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vladimir, On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote: > Hi Markus, > > On 17.11.2015 11:24, Markus Pargmann wrote: > > Hi, > > > > On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote: > >> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes > >> OOPS on boot on LPC32xx boards: > >> > >> Unable to handle kernel NULL pointer dereference at virtual address 00000000 > >> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 > >> Hardware name: LPC32XX SoC (Flattened Device Tree) > >> task: c381baa0 ti: c381e000 task.ti: c381e000 > >> PC is at strcmp+0x10/0x40 > >> LR is at gpiochip_add+0x3d0/0x4d4 > >> pc : [<>] lr : [<>] psr: a0000093 > >> sp : c381fd60 ip : c381fd70 fp : c381fd6c > >> > >> [snip] > >> > >> Backtrace: > >> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) > >> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) > >> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) > >> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) > >> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) > >> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) > >> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) > >> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) > >> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) > >> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) > >> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) > >> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) > >> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) > >> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) > >> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) > >> > >> This is caused by the fact that at the moment some GPIO names are set > >> to NULL, there is a hole in linear representation of one GPI bank, see > >> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. > >> > >> The same problem most probably affects also gpio-cs5535.c, see > >> cs5535_gpio_names[]. > >> > >> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > >> --- > >> Linus, Markus, > >> > >> I understand that LPC32xx GPIO driver is ugly (hopefully this > >> will be changed in future), but please account this problem. > >> > >> I don't ask for inclusion of this change, because my own analysis is > >> not done completely. > >> > >> Also please review, the same problem probably affects gpio-cs5535.c > >> as well, see cs5535_gpio_names[]. > > > > Thanks for this hint. But I think these patches for gpio_name_to_desc won't get > > mainline any time soon as this depends on the chardev interface and how it will > > look like. > > > > sorry for misunderstanding, what do mean by "these patches"? > > I see two options here, one is to fix or revert your c0017ed71966 due to > caused regressions, another one is to update affected GPIO drivers > removing discontinuity in GPIO enumeration inside a bank. Sorry, seems I was on a wrong kernel version when checking this morning. Linus removed lots of my patches from his tree after some correct comments so I thought this patch was removed as well. As it is mainline your patch looks good to me. Perhaps a comment on why we need to check for !name at this point would be good. Best Regards, Markus > > With best wishes, > Vladimir > > >> > >> drivers/gpio/gpiolib.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > >> index a18f00f..2a91f32 100644 > >> --- a/drivers/gpio/gpiolib.c > >> +++ b/drivers/gpio/gpiolib.c > >> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) > >> for (i = 0; i != chip->ngpio; ++i) { > >> struct gpio_desc *gpio = &chip->desc[i]; > >> > >> - if (!gpio->name) > >> + if (!gpio->name || !name) > >> continue; > >> > >> if (!strcmp(gpio->name, name)) { > >> > > >
Hi Linus, On 17.11.2015 13:50, Markus Pargmann wrote: > Hi Vladimir, > > On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote: >> Hi Markus, >> >> On 17.11.2015 11:24, Markus Pargmann wrote: >>> Hi, >>> >>> On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote: >>>> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes >>>> OOPS on boot on LPC32xx boards: >>>> >>>> Unable to handle kernel NULL pointer dereference at virtual address 00000000 >>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 >>>> Hardware name: LPC32XX SoC (Flattened Device Tree) >>>> task: c381baa0 ti: c381e000 task.ti: c381e000 >>>> PC is at strcmp+0x10/0x40 >>>> LR is at gpiochip_add+0x3d0/0x4d4 >>>> pc : [<>] lr : [<>] psr: a0000093 >>>> sp : c381fd60 ip : c381fd70 fp : c381fd6c >>>> >>>> [snip] >>>> >>>> Backtrace: >>>> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) >>>> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) >>>> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) >>>> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) >>>> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) >>>> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) >>>> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) >>>> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) >>>> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) >>>> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) >>>> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) >>>> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) >>>> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) >>>> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) >>>> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) >>>> >>>> This is caused by the fact that at the moment some GPIO names are set >>>> to NULL, there is a hole in linear representation of one GPI bank, see >>>> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. >>>> >>>> The same problem most probably affects also gpio-cs5535.c, see >>>> cs5535_gpio_names[]. >>>> >>>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> >>>> --- >>>> Linus, Markus, >>>> >>>> I understand that LPC32xx GPIO driver is ugly (hopefully this >>>> will be changed in future), but please account this problem. >>>> >>>> I don't ask for inclusion of this change, because my own analysis is >>>> not done completely. >>>> >>>> Also please review, the same problem probably affects gpio-cs5535.c >>>> as well, see cs5535_gpio_names[]. >>> >>> Thanks for this hint. But I think these patches for gpio_name_to_desc won't get >>> mainline any time soon as this depends on the chardev interface and how it will >>> look like. >>> >> >> sorry for misunderstanding, what do mean by "these patches"? >> >> I see two options here, one is to fix or revert your c0017ed71966 due to >> caused regressions, another one is to update affected GPIO drivers >> removing discontinuity in GPIO enumeration inside a bank. > > Sorry, seems I was on a wrong kernel version when checking this morning. Linus > removed lots of my patches from his tree after some correct comments so I > thought this patch was removed as well. Linus, should I resend this fix for v4.4 or is it good enough for application? > As it is mainline your patch looks good to me. Perhaps a comment on why we need > to check for !name at this point would be good. There is such a description in the commit message, why this check is needed. With best wishes, Vladimir > Best Regards, > > Markus > >> >> With best wishes, >> Vladimir >> >>>> >>>> drivers/gpio/gpiolib.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c >>>> index a18f00f..2a91f32 100644 >>>> --- a/drivers/gpio/gpiolib.c >>>> +++ b/drivers/gpio/gpiolib.c >>>> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) >>>> for (i = 0; i != chip->ngpio; ++i) { >>>> struct gpio_desc *gpio = &chip->desc[i]; >>>> >>>> - if (!gpio->name) >>>> + if (!gpio->name || !name) >>>> continue; >>>> >>>> if (!strcmp(gpio->name, name)) { >>>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 11, 2015 at 1:36 PM, Vladimir Zapolskiy <vz@mleia.com> wrote: > Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes > OOPS on boot on LPC32xx boards: > > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 > Hardware name: LPC32XX SoC (Flattened Device Tree) > task: c381baa0 ti: c381e000 task.ti: c381e000 > PC is at strcmp+0x10/0x40 > LR is at gpiochip_add+0x3d0/0x4d4 > pc : [<>] lr : [<>] psr: a0000093 > sp : c381fd60 ip : c381fd70 fp : c381fd6c > > [snip] > > Backtrace: > [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) > [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) > [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) > [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) > [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) > [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) > [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) > [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) > [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) > [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) > [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) > [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) > [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) > [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) > [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) > > This is caused by the fact that at the moment some GPIO names are set > to NULL, there is a hole in linear representation of one GPI bank, see > drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. > > The same problem most probably affects also gpio-cs5535.c, see > cs5535_gpio_names[]. > > Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> Patch applied for fixes. Sorry for the delay. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Sunday 29 November 2015 14:44:30 Vladimir Zapolskiy wrote: > Hi Linus, > > On 17.11.2015 13:50, Markus Pargmann wrote: > > Hi Vladimir, > > > > On Tuesday 17 November 2015 13:20:40 Vladimir Zapolskiy wrote: > >> Hi Markus, > >> > >> On 17.11.2015 11:24, Markus Pargmann wrote: > >>> Hi, > >>> > >>> On Wednesday 11 November 2015 14:36:53 Vladimir Zapolskiy wrote: > >>>> Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes > >>>> OOPS on boot on LPC32xx boards: > >>>> > >>>> Unable to handle kernel NULL pointer dereference at virtual address 00000000 > >>>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 > >>>> Hardware name: LPC32XX SoC (Flattened Device Tree) > >>>> task: c381baa0 ti: c381e000 task.ti: c381e000 > >>>> PC is at strcmp+0x10/0x40 > >>>> LR is at gpiochip_add+0x3d0/0x4d4 > >>>> pc : [<>] lr : [<>] psr: a0000093 > >>>> sp : c381fd60 ip : c381fd70 fp : c381fd6c > >>>> > >>>> [snip] > >>>> > >>>> Backtrace: > >>>> [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) > >>>> [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) > >>>> [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) > >>>> [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) > >>>> [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) > >>>> [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) > >>>> [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) > >>>> [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) > >>>> [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) > >>>> [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) > >>>> [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) > >>>> [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) > >>>> [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) > >>>> [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) > >>>> [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) > >>>> > >>>> This is caused by the fact that at the moment some GPIO names are set > >>>> to NULL, there is a hole in linear representation of one GPI bank, see > >>>> drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. > >>>> > >>>> The same problem most probably affects also gpio-cs5535.c, see > >>>> cs5535_gpio_names[]. > >>>> > >>>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> > >>>> --- > >>>> Linus, Markus, > >>>> > >>>> I understand that LPC32xx GPIO driver is ugly (hopefully this > >>>> will be changed in future), but please account this problem. > >>>> > >>>> I don't ask for inclusion of this change, because my own analysis is > >>>> not done completely. > >>>> > >>>> Also please review, the same problem probably affects gpio-cs5535.c > >>>> as well, see cs5535_gpio_names[]. > >>> > >>> Thanks for this hint. But I think these patches for gpio_name_to_desc won't get > >>> mainline any time soon as this depends on the chardev interface and how it will > >>> look like. > >>> > >> > >> sorry for misunderstanding, what do mean by "these patches"? > >> > >> I see two options here, one is to fix or revert your c0017ed71966 due to > >> caused regressions, another one is to update affected GPIO drivers > >> removing discontinuity in GPIO enumeration inside a bank. > > > > Sorry, seems I was on a wrong kernel version when checking this morning. Linus > > removed lots of my patches from his tree after some correct comments so I > > thought this patch was removed as well. > > Linus, should I resend this fix for v4.4 or is it good enough for > application? > > > As it is mainline your patch looks good to me. Perhaps a comment on why we need > > to check for !name at this point would be good. > > There is such a description in the commit message, why this check is needed. I meant a code comment. But that's not too important. Best Regards, Markus > > With best wishes, > Vladimir > > > Best Regards, > > > > Markus > > > >> > >> With best wishes, > >> Vladimir > >> > >>>> > >>>> drivers/gpio/gpiolib.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > >>>> index a18f00f..2a91f32 100644 > >>>> --- a/drivers/gpio/gpiolib.c > >>>> +++ b/drivers/gpio/gpiolib.c > >>>> @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) > >>>> for (i = 0; i != chip->ngpio; ++i) { > >>>> struct gpio_desc *gpio = &chip->desc[i]; > >>>> > >>>> - if (!gpio->name) > >>>> + if (!gpio->name || !name) > >>>> continue; > >>>> > >>>> if (!strcmp(gpio->name, name)) { > >>>> > >>> > >> > > >
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index a18f00f..2a91f32 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -233,7 +233,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name) for (i = 0; i != chip->ngpio; ++i) { struct gpio_desc *gpio = &chip->desc[i]; - if (!gpio->name) + if (!gpio->name || !name) continue; if (!strcmp(gpio->name, name)) {
Commit c0017ed71966 ("gpio: Introduce gpio descriptor 'name'") causes OOPS on boot on LPC32xx boards: Unable to handle kernel NULL pointer dereference at virtual address 00000000 CPU: 0 PID: 1 Comm: swapper Not tainted 4.3.0+ #707 Hardware name: LPC32XX SoC (Flattened Device Tree) task: c381baa0 ti: c381e000 task.ti: c381e000 PC is at strcmp+0x10/0x40 LR is at gpiochip_add+0x3d0/0x4d4 pc : [<>] lr : [<>] psr: a0000093 sp : c381fd60 ip : c381fd70 fp : c381fd6c [snip] Backtrace: [<>] (strcmp) from [<>] (gpiochip_add+0x3d0/0x4d4) [<>] (gpiochip_add) from [<>] (lpc32xx_gpio_probe+0x44/0x60) [<>] (lpc32xx_gpio_probe) from [<>] (platform_drv_probe+0x40/0x8c) [<>] (platform_drv_probe) from [<>] (driver_probe_device+0x110/0x294) [<>] (driver_probe_device) from [<>] (__driver_attach+0x70/0x94) [<>] (__driver_attach) from [<>] (bus_for_each_dev+0x74/0x98) [<>] (bus_for_each_dev) from [<>] (driver_attach+0x20/0x28) [<>] (driver_attach) from [<>] (bus_add_driver+0xd4/0x1f0) [<>] (bus_add_driver) from [<>] (driver_register+0xa4/0xe8) [<>] (driver_register) from [<>] (__platform_driver_register+0x38/0x4c) [<>] (__platform_driver_register) from [<>] (lpc32xx_gpio_driver_init+0x18/0x20) [<>] (lpc32xx_gpio_driver_init) from [<>] (do_one_initcall+0x108/0x1c8) [<>] (do_one_initcall) from [<>] (kernel_init_freeable+0x10c/0x1d4) [<>] (kernel_init_freeable) from [<>] (kernel_init+0x10/0xec) [<>] (kernel_init) from [<>] (ret_from_fork+0x14/0x24) This is caused by the fact that at the moment some GPIO names are set to NULL, there is a hole in linear representation of one GPI bank, see drivers/gpio/gpio-lpc32xx.c / gpi_p3_names[] for details. The same problem most probably affects also gpio-cs5535.c, see cs5535_gpio_names[]. Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- Linus, Markus, I understand that LPC32xx GPIO driver is ugly (hopefully this will be changed in future), but please account this problem. I don't ask for inclusion of this change, because my own analysis is not done completely. Also please review, the same problem probably affects gpio-cs5535.c as well, see cs5535_gpio_names[]. drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)