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

2020-11-24 Thread Alexandre Belloni
On Wed, 18 Nov 2020 14:00:16 +0200, cristian.bir...@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,
-- 
Alexandre Belloni 


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

2020-11-18 Thread 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


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

2020-11-18 Thread 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

2020-11-18 Thread Ludovic Desroches
Hi Cristi,

Adding the gpio list.

On Wed, Nov 18, 2020 at 02:00:16PM +0200, cristian.bir...@microchip.com wrote:
> From: Cristian Birsan 
> 
> 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 

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 
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 
Link: 
https://lore.kernel.org/r/20200319122737.3063291-2-thierry.red...@gmail.com
Tested-by: Vidya Sagar 
Signed-off-by: Linus Walleij 

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
>