2020-11-18 12:02:45

by Cristian Birsan

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

From: Cristian Birsan <[email protected]>

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(+)

--
2.25.1


2020-11-18 12:02:55

by Cristian Birsan

[permalink] [raw]
Subject: [PATCH 2/3] ARM: dts: at91: sama5d4_xplained: add pincontrol for USB Host

From: Cristian Birsan <[email protected]>

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.

Fixes: 38153a017896f ("ARM: at91/dt: sama5d4: add dts for sama5d4 xplained board")
Signed-off-by: Cristian Birsan <[email protected]>
---
arch/arm/boot/dts/at91-sama5d4_xplained.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
index e5974a17374c..0b3ad1b580b8 100644
--- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
@@ -134,6 +134,11 @@ pinctrl_usba_vbus: usba_vbus {
atmel,pins =
<AT91_PIOE 31 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH>;
};
+ pinctrl_usb_default: usb_default {
+ atmel,pins =
+ <AT91_PIOE 11 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+ AT91_PIOE 14 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+ };
pinctrl_key_gpio: key_gpio_0 {
atmel,pins =
<AT91_PIOE 8 AT91_PERIPH_GPIO AT91_PINCTRL_PULL_UP_DEGLITCH>;
@@ -159,6 +164,8 @@ usb1: ohci@500000 {
&pioE 11 GPIO_ACTIVE_HIGH
&pioE 14 GPIO_ACTIVE_HIGH
>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_default>;
status = "okay";
};

--
2.25.1

2020-11-18 12:03:48

by Cristian Birsan

[permalink] [raw]
Subject: [PATCH 1/3] ARM: dts: sam9x60: add pincontrol for USB Host

From: Cristian Birsan <[email protected]>

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.

Fixes: 1e5f532c2737 ("ARM: dts: at91: sam9x60: add device tree for soc and board")
Signed-off-by: Cristian Birsan <[email protected]>
---
arch/arm/boot/dts/at91-sam9x60ek.dts | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts b/arch/arm/boot/dts/at91-sam9x60ek.dts
index eae28b82c7fd..0e3b6147069f 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -569,6 +569,13 @@ pinctrl_usba_vbus: usba_vbus {
atmel,pins = <AT91_PIOB 16 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
};
};
+
+ usb1 {
+ pinctrl_usb_default: usb_default {
+ atmel,pins = <AT91_PIOD 15 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+ AT91_PIOD 16 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+ };
+ };
}; /* pinctrl */

&pmc {
@@ -684,6 +691,8 @@ &usb1 {
atmel,vbus-gpio = <0
&pioD 15 GPIO_ACTIVE_HIGH
&pioD 16 GPIO_ACTIVE_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_default>;
status = "okay";
};

--
2.25.1

2020-11-18 12:06:42

by Cristian Birsan

[permalink] [raw]
Subject: [PATCH 3/3] ARM: dts: at91: sama5d3_xplained: add pincontrol for USB Host

From: Cristian Birsan <[email protected]>

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.

Fixes: b7c2b61570798 ("ARM: at91: add Atmel's SAMA5D3 Xplained board")
Signed-off-by: Cristian Birsan <[email protected]>
---
arch/arm/boot/dts/at91-sama5d3_xplained.dts | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d3_xplained.dts b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
index cf13632edd44..5179258f9247 100644
--- a/arch/arm/boot/dts/at91-sama5d3_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d3_xplained.dts
@@ -242,6 +242,11 @@ pinctrl_usba_vbus: usba_vbus {
atmel,pins =
<AT91_PIOE 9 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH>; /* PE9, conflicts with A9 */
};
+ pinctrl_usb_default: usb_default {
+ atmel,pins =
+ <AT91_PIOE 3 AT91_PERIPH_GPIO AT91_PINCTRL_NONE
+ AT91_PIOE 4 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>;
+ };
};
};
};
@@ -259,6 +264,8 @@ usb1: ohci@600000 {
&pioE 3 GPIO_ACTIVE_LOW
&pioE 4 GPIO_ACTIVE_LOW
>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usb_default>;
status = "okay";
};

--
2.25.1

2020-11-18 15:06:19

by Ludovic Desroches

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

Hi Cristi,

Adding the gpio list.

On Wed, Nov 18, 2020 at 02:00:16PM +0200, [email protected] wrote:
> From: Cristian Birsan <[email protected]>
>
> 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 <[email protected]>

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 <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Tested-by: Vidya Sagar <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>

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
>

2020-11-18 15:30:16

by Alexandre Belloni

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

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

2020-11-18 21:15:27

by Ludovic Desroches

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

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

2020-11-24 21:29:07

by Alexandre Belloni

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

On Wed, 18 Nov 2020 14:00:16 +0200, [email protected] 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 <[email protected]>