Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host

Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host

[Ludovic Desroches]

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
> 

Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host

[Alexandre Belloni]

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.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 0/3] ARM: dts: at91: add pincontrol node for USB Host

[Ludovic Desroches]

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