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

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

[cristian.birsan]

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

-- 
2.25.1

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

[cristian.birsan]

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.

Fixes: 1e5f532c2737 ("ARM: dts: at91: sam9x60: add device tree for soc and board")
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 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

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

[cristian.birsan]

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.

Fixes: 38153a017896f ("ARM: at91/dt: sama5d4: add dts for sama5d4 xplained board")
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 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

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

[cristian.birsan]

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.

Fixes: b7c2b61570798 ("ARM: at91: add Atmel's SAMA5D3 Xplained board")
Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 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

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

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

[Alexandre Belloni]

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,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>