| Message ID | 20201118120019.1257580-1-cristian.birsan@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ARM: dts: at91: add pincontrol node for USB Host | expand |
Hi Cristi, Adding the gpio list. On Wed, Nov 18, 2020 at 02:00:16PM +0200, cristian.birsan@microchip.com wrote: > From: Cristian Birsan <cristian.birsan@microchip.com> > > The pincontrol node is needed for USB Host since Linux v5.7-rc1. Without > it the driver probes but VBus is not powered because of wrong pincontrol > configuration. The patch was tested on SAM9x60EK, SAMA5D4-EK and SAMA5D3-EK. > No problem on my side with this set of patches, it's consistent with what we have. Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> I just want to share the full picture leading to this situation. You told me the breakage appears after this commit: commit 2ab73c6d8323fa1eb02c16c07c40ba2ed17da729 Author: Thierry Reding <treding@nvidia.com> Date: Thu Mar 19 13:27:29 2020 +0100 gpio: Support GPIO controllers without pin-ranges Wake gpiochip_generic_request() call into the pinctrl helpers only if a GPIO controller had any pin-ranges assigned to it. This allows a driver to unconditionally use this helper if it supports multiple devices of which only a subset have pin-ranges assigned to them. Signed-off-by: Thierry Reding <treding@nvidia.com> Link: https://lore.kernel.org/r/20200319122737.3063291-2-thierry.reding@gmail.com Tested-by: Vidya Sagar <vidyas@nvidia.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> We were used to defining pinctrl for all our pins. That is somewhat redundant when the pin is requested through the gpiolib. The pinctrl-at91 driver doesn't register any pin range. After this commit, the gpio_generic_request() fails. The consequence is if we forgot to define the pinctrl, the pin won't be muxed as a gpio. At first glance, there is no trivial way to register the pin range in the pinctrl-at91 driver. There is one driver for the pinctrl and one for the gpio. I am open to suggestions to fix it in the pinctrl-at91 driver as well if there is an elegant way (I have some in mind, but there are not) without having to refactor the driver. Regards Ludovic > Cristian Birsan (3): > ARM: dts: sam9x60: add pincontrol for USB Host > ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host > ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host > > arch/arm/boot/dts/at91-sam9x60ek.dts | 9 +++++++++ > arch/arm/boot/dts/at91-sama5d3_xplained.dts | 7 +++++++ > arch/arm/boot/dts/at91-sama5d4_xplained.dts | 7 +++++++ > 3 files changed, 23 insertions(+) > > -- > 2.25.1 >
Hello, On 18/11/2020 16:03:36+0100, Ludovic Desroches wrote: > At first glance, there is no trivial way to register the pin range in the > pinctrl-at91 driver. There is one driver for the pinctrl and one for the gpio. > I am open to suggestions to fix it in the pinctrl-at91 driver as well if there > is an elegant way (I have some in mind, but there are not) without having to > refactor the driver. > But shouldn't that driver be refactored at some point anyway? I know you are moving away with new SoCs but it causes real issues. For example, gpio hogs are not working, this is impacting some of your customers. The other thing is the weird probe order preventing a nice cleanup of the platform code.
On Wed, Nov 18, 2020 at 04:26:52PM +0100, Alexandre Belloni wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hello, > > On 18/11/2020 16:03:36+0100, Ludovic Desroches wrote: > > At first glance, there is no trivial way to register the pin range in the > > pinctrl-at91 driver. There is one driver for the pinctrl and one for the gpio. > > I am open to suggestions to fix it in the pinctrl-at91 driver as well if there > > is an elegant way (I have some in mind, but there are not) without having to > > refactor the driver. > > > > But shouldn't that driver be refactored at some point anyway? I know you > are moving away with new SoCs but it causes real issues. For example, > gpio hogs are not working, this is impacting some of your customers. > I agree, maintainance of this driver is difficult because of its design. Unfortunately, I doubt being able to hadnle a refactoring of this driver in a near future. > The other thing is the weird probe order preventing a nice cleanup of > the platform code. True. IMO, having gpio controlers probed before pinctrl is one of the reason which prevents a trivial fix. Regards Ludovic > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Wed, 18 Nov 2020 14:00:16 +0200, cristian.birsan@microchip.com wrote: > The pincontrol node is needed for USB Host since Linux v5.7-rc1. Without > it the driver probes but VBus is not powered because of wrong pincontrol > configuration. The patch was tested on SAM9x60EK, SAMA5D4-EK and SAMA5D3-EK. > > Cristian Birsan (3): > ARM: dts: sam9x60: add pincontrol for USB Host > ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host > ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host > > [...] Applied, thanks! [1/3] ARM: dts: at91: sam9x60: add pincontrol for USB Host commit: 5ba6291086d2ae8006be9e0f19bf2001a85c9dc1 [2/3] ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host commit: be4dd2d448816a27c1446f8f37fce375daf64148 [3/3] ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host commit: e1062fa7292f1e3744db0a487c4ac0109e09b03d Best regards,
From: Cristian Birsan <cristian.birsan@microchip.com> The pincontrol node is needed for USB Host since Linux v5.7-rc1. Without it the driver probes but VBus is not powered because of wrong pincontrol configuration. The patch was tested on SAM9x60EK, SAMA5D4-EK and SAMA5D3-EK. Cristian Birsan (3): ARM: dts: sam9x60: add pincontrol for USB Host ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host arch/arm/boot/dts/at91-sam9x60ek.dts | 9 +++++++++ arch/arm/boot/dts/at91-sama5d3_xplained.dts | 7 +++++++ arch/arm/boot/dts/at91-sama5d4_xplained.dts | 7 +++++++ 3 files changed, 23 insertions(+)