LPC32xx gpio driver broken by commit 762c2e46 in 4.9-rc1
Vladimir, Linus, Alexandre,
the current LPC32xx GPIO driver is broken by commit 762c2e46 (gpio: of: remove of_gpiochip_and_xlate() and struct gg_data).
A call to "of_get_named_gpio" to retrieve the GPIO will always return -EINVAL, except for the first GPIO bank.
Prior to this commit, the driver was working properly because of the side-effect of the match function called by "gpiochip_find" inside "of_get_named_gpiod_flags" function.
I think, the proper long-term solution is to replace the LPC32xx GPIO driver; an initial version was previously submitted, by Vladimir Zapolskiy, to the mailing list:
http://www.spinics.net/lists/linux-gpio/msg09746.html
Is there any short-term solution that can be done with the existing driver to keep the LPC32xx platform working properly in the 4.9 mainline kernel?
Regards,
Sylvain Lemieux
Hi Sylvain,
Quoted 4 lines · Sylvain Lemieux
Vladimir, Linus, Alexandre,
the current LPC32xx GPIO driver is broken by commit
762c2e46(gpio: of: removeof_gpiochip_and_xlate()and structgg_data).
I do confirm, as well I've noticed that the driver is broken on v4.9, however I didn't find time to bisect the problematic commit, thank you to pinning it out.
Quoted 11 lines
A call to "
of_get_named_gpio" to retrieve the GPIO will always return-EINVAL, except for the first GPIO bank.Prior to this commit, the driver was working properly because of the side-effect of the match function called by "
gpiochip_find" inside "of_get_named_gpiod_flags" function.I think, the proper long-term solution is to replace the LPC32xx GPIO driver; an initial version was previously submitted, by Vladimir Zapolskiy, to the mailing list:
http://www.spinics.net/lists/linux-gpio/msg09746.html
I still cherish a hope for submitting v2 for v4.10, the difference from v1 is expected to be relatively big (e.g. there will be 5 banks instead of 6, on hardware level banks P0 and P1 are on the single controller, there will be other lesser differences also).
Quoted 3 lines
Is there any short-term solution that can be done with the existing driver to keep the LPC32xx platform working properly in the 4.9 mainline kernel?
Unfortunately I didn't spend enough time to fix the problem, but in two words the root cause is that from the OF description there is only one on-SoC GPIO controller, but the GPIO controller driver registers multiple gpiochips (6 in this particular case), consumers specify a bank as a value in the first cell. The referenced commit simplifies the matter by assuming that a number of gpiochips for consumers is the same as the number of registered GPIO controllers from OF description.
I don't think that the problem is specific only to the legacy LPC32xx GPIO controller driver, but at the moment I don't have any more examples to share. Probably another 3-cell GPIO controller driver gpio-etraxfs.c is also broken, a good enough implicit indicator for potentially broken drivers might be if you see gpiochip_add_data() call inside a loop:
* gpio-sch311x.c
* gpio-ml-ioh.c
* gpio-etraxfs.c
* gpio-htc-egpio.c
* gpio-davinci.c
* gpio-lpc32xx.c
--
With best wishes,
Vladimir
Hi Vladimir,
Quoted 24 lines · Vladimir Zapolskiy
Hi Sylvain,
Quoted 4 lines · Sylvain Lemieux
Vladimir, Linus, Alexandre,
the current LPC32xx GPIO driver is broken by commit
762c2e46(gpio: of: removeof_gpiochip_and_xlate()and structgg_data).I do confirm, as well I've noticed that the driver is broken on v4.9, however I didn't find time to bisect the problematic commit, thank you to pinning it out.
Quoted 11 lines
A call to "
of_get_named_gpio" to retrieve the GPIO will always return-EINVAL, except for the first GPIO bank.Prior to this commit, the driver was working properly because of the side-effect of the match function called by "
gpiochip_find" inside "of_get_named_gpiod_flags" function.I think, the proper long-term solution is to replace the LPC32xx GPIO driver; an initial version was previously submitted, by Vladimir Zapolskiy, to the mailing list:
http://www.spinics.net/lists/linux-gpio/msg09746.html
Quoted 5 lines
I still cherish a hope for submitting v2 for v4.10, the difference from v1 is expected to be relatively big (e.g. there will be 5 banks instead of 6, on hardware level banks P0 and P1 are on the single controller, there will be other lesser differences also).
I will be available to test the new driver, once submitted on the mailing list.
Quoted 13 lines
Quoted 3 lines
Is there any short-term solution that can be done with the existing driver to keep the LPC32xx platform working properly in the 4.9 mainline kernel?
Unfortunately I didn't spend enough time to fix the problem, but in two words the root cause is that from the OF description there is only one on-SoC GPIO controller, but the GPIO controller driver registers multiple gpiochips (6 in this particular case), consumers specify a bank as a value in the first cell. The referenced commit simplifies the matter by assuming that a number of gpiochips for consumers is the same as the number of registered GPIO controllers from OF description.
Quoted 13 lines
I don't think that the problem is specific only to the legacy LPC32xx GPIO controller driver, but at the moment I don't have any more examples to share. Probably another 3-cell GPIO controller driver
gpio-etraxfs.cis also broken, a good enough implicit indicator for potentially broken drivers might be if you seegpiochip_add_data()call inside a loop:
*gpio-sch311x.c
*gpio-ml-ioh.c
*gpio-etraxfs.c
*gpio-htc-egpio.c
*gpio-davinci.c
*gpio-lpc32xx.c
As a temporary solution, locally I reverted the following commits to be able to have a working platform on 4.9-rc1:
* "gpio: of: factor out common code to a new helper function" (99468c1af913bb5662c223b68e783b4bf9200184)
* "gpio: of: remove of_gpiochip_and_xlate() and struct gg_data" (762c2e46c0591d207289105c8718e4adf29b2b34)
Regards,
Sylvain
Quoted 9 lines · Sylvain Lemieux
the current LPC32xx GPIO driver is broken by commit
762c2e46(gpio: of: removeof_gpiochip_and_xlate()and structgg_data).A call to "
of_get_named_gpio" to retrieve the GPIO will always return-EINVAL, except for the first GPIO bank.Prior to this commit, the driver was working properly because of the side-effect of the match function called by "
gpiochip_find" inside "of_get_named_gpiod_flags" function.
(...)
Quoted 3 lines
Is there any short-term solution that can be done with the existing driver to keep the LPC32xx platform working properly in the 4.9 mainline kernel?
Masahiro, what do you think is the best course to proceed here? Can we
- Restore the behaviour prior to the patches or
- Fix up all users or
- Do we have to revert these two patches?
I would prefer not to revert, because I liked the cleanup a lot ...
Yours,
Linus Walleij
Hi.
2016-10-24 9:46 GMT+09:00 Linus Walleij <[email protected]>:
Quoted 17 lines
Quoted 9 lines · Sylvain Lemieux
the current LPC32xx GPIO driver is broken by commit
762c2e46(gpio: of: removeof_gpiochip_and_xlate()and structgg_data).A call to "
of_get_named_gpio" to retrieve the GPIO will always return-EINVAL, except for the first GPIO bank.Prior to this commit, the driver was working properly because of the side-effect of the match function called by "
gpiochip_find" inside "of_get_named_gpiod_flags" function.(...)
Quoted 3 lines
Is there any short-term solution that can be done with the existing driver to keep the LPC32xx platform working properly in the 4.9 mainline kernel?
Quoted 8 lines
Masahiro, what do you think is the best course to proceed here? Can we
- Restore the behaviour prior to the patches or
- Fix up all users or
- Do we have to revert these two patches?I would prefer not to revert, because I liked the cleanup a lot ...
Personally, I do not want to revert, either.
I guess, this discussion comes down to
"is it justified to register multiple chips
associated to a single DT node?"
I feel like, DT properties such as
"gpio-hog", "gpio-ranges" assume one gpio_chip for one node.
We can register multi gpio_chip if we like, but
it looks odd to parse the same DT properties over and over again looping gpio_chips.
If we move forward to single gpio_chip solution,
please check my RFC
"gpio: of: fix GPIO drivers with multiple gpio_chip for a single node" as a long-term (but not too long) solution.
Signature · 2 lines
Best Regards Masahiro Yamada