ARM: dts: sun8i-r40: remove unused GPIO regulator

[PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Hermann.Lauer@uni-heidelberg.de 1 year ago

Subject: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Banana Pi M2 Ultra V1.1 board resets immediately when the usb core tries
to setup PH23 GPIO. It turned out that the V1.0 board USB-A ports are
always power supplied and according to the board scheme PH23 is simply
not connected.

So remove the PH23 setup: Doesn't harm V1.0 (with R40) and let V1.1
(with A40i) start.

Signed-off-by: Hermann Lauer <Hermann.Lauer@uni-heidelberg.de>
---
V2: shorten subject, rm dangerous PH23 regulator completely

diff --git a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
--- a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
@@ -91,15 +91,6 @@
 		};
 	};
 
-	reg_vcc5v0: vcc5v0 {
-		compatible = "regulator-fixed";
-		regulator-name = "vcc5v0";
-		regulator-min-microvolt = <5000000>;
-		regulator-max-microvolt = <5000000>;
-		gpio = <&pio 7 23 GPIO_ACTIVE_HIGH>; /* PH23 */
-		enable-active-high;
-	};
-
 	wifi_pwrseq: pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 WIFI_EN */
@@ -347,7 +338,5 @@
 };
 
 &usbphy {
-	usb1_vbus-supply = <&reg_vcc5v0>;
-	usb2_vbus-supply = <&reg_vcc5v0>;
 	status = "okay";
 };

Re: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Andre Przywara 1 year ago

On Fri, 13 Dec 2024 20:54:33 +0100
Hermann.Lauer@uni-heidelberg.de wrote:

Hi,

CC:ing Icenowy, who added the regulator originally, with commit
0ca12c1ee43c ("ARM: sun8i: r40: add 5V regulator for Banana Pi M2
Ultra").

Icenowy: can you clarify what "newer" version this was referring to in
that commit message? That commit in itself doesn't seem to do anything,
as the regulator isn't referenced, and it's not always-on. It's only
later when the USB nodes were added that it got used?
So was PH23 really necessary? As Hermann reports, setting PH23 on a v1.1
makes it reboot.

> Subject: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Hermann, this looks like an extra subject line here?

> Banana Pi M2 Ultra V1.1 board resets immediately when the usb core tries
> to setup PH23 GPIO. It turned out that the V1.0 board USB-A ports are
> always power supplied and according to the board scheme PH23 is simply
> not connected.
> 
> So remove the PH23 setup: Doesn't harm V1.0 (with R40) and let V1.1
> (with A40i) start.
> 
> Signed-off-by: Hermann Lauer <Hermann.Lauer@uni-heidelberg.de>

The patch itself looks good to me, but it would be good to clarify the
situation with the "older newer" version.

Just in case:
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre
 
> ---
> V2: shorten subject, rm dangerous PH23 regulator completely
> 
> diff --git a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
> --- a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
> +++ b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
> @@ -91,15 +91,6 @@
>  		};
>  	};
>  
> -	reg_vcc5v0: vcc5v0 {
> -		compatible = "regulator-fixed";
> -		regulator-name = "vcc5v0";
> -		regulator-min-microvolt = <5000000>;
> -		regulator-max-microvolt = <5000000>;
> -		gpio = <&pio 7 23 GPIO_ACTIVE_HIGH>; /* PH23 */
> -		enable-active-high;
> -	};
> -
>  	wifi_pwrseq: pwrseq {
>  		compatible = "mmc-pwrseq-simple";
>  		reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 WIFI_EN */
> @@ -347,7 +338,5 @@
>  };
>  
>  &usbphy {
> -	usb1_vbus-supply = <&reg_vcc5v0>;
> -	usb2_vbus-supply = <&reg_vcc5v0>;
>  	status = "okay";
>  };
>

Re: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Icenowy Zheng 1 year ago

在 2024-12-14星期六的 01:16 +0000，Andre Przywara写道：
> On Fri, 13 Dec 2024 20:54:33 +0100
> Hermann.Lauer@uni-heidelberg.de wrote:
> 
> Hi,
> 
> CC:ing Icenowy, who added the regulator originally, with commit
> 0ca12c1ee43c ("ARM: sun8i: r40: add 5V regulator for Banana Pi M2
> Ultra").
> 
> Icenowy: can you clarify what "newer" version this was referring to
> in
> that commit message? That commit in itself doesn't seem to do
> anything,
> as the regulator isn't referenced, and it's not always-on. It's only
> later when the USB nodes were added that it got used?
> So was PH23 really necessary? As Hermann reports, setting PH23 on a
> v1.1
> makes it reboot.

I am not sure, the schematics I have here (which says V1.0) have PH23
as NC... Well, the M2 Berry schematics have PH23 as 5V EN, maybe I
messed up M2U and M2B when developing?

Sorry but I think the data retention situation of my memory looks bad
here, maybe it needs a shorter refresh interval ;-)

> 
> > Subject: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO
> > regulator
> 
> Hermann, this looks like an extra subject line here?
> 
> > Banana Pi M2 Ultra V1.1 board resets immediately when the usb core
> > tries
> > to setup PH23 GPIO. It turned out that the V1.0 board USB-A ports
> > are
> > always power supplied and according to the board scheme PH23 is
> > simply
> > not connected.
> > 
> > So remove the PH23 setup: Doesn't harm V1.0 (with R40) and let V1.1
> > (with A40i) start.
> > 
> > Signed-off-by: Hermann Lauer <Hermann.Lauer@uni-heidelberg.de>
> 
> The patch itself looks good to me, but it would be good to clarify
> the
> situation with the "older newer" version.
> 
> Just in case:
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre
>  
> > ---
> > V2: shorten subject, rm dangerous PH23 regulator completely
> > 
> > diff --git a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-
> > ultra.dts b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-
> > ultra.dts
> > --- a/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
> > +++ b/arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts
> > @@ -91,15 +91,6 @@
> >                 };
> >         };
> >  
> > -       reg_vcc5v0: vcc5v0 {
> > -               compatible = "regulator-fixed";
> > -               regulator-name = "vcc5v0";
> > -               regulator-min-microvolt = <5000000>;
> > -               regulator-max-microvolt = <5000000>;
> > -               gpio = <&pio 7 23 GPIO_ACTIVE_HIGH>; /* PH23 */
> > -               enable-active-high;
> > -       };
> > -
> >         wifi_pwrseq: pwrseq {
> >                 compatible = "mmc-pwrseq-simple";
> >                 reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10
> > WIFI_EN */
> > @@ -347,7 +338,5 @@
> >  };
> >  
> >  &usbphy {
> > -       usb1_vbus-supply = <&reg_vcc5v0>;
> > -       usb2_vbus-supply = <&reg_vcc5v0>;
> >         status = "okay";
> >  };
> > 
> 
>

Re: Re: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Hermann Lauer 12 months ago

Hi,

On Sat, Dec 14, 2024 at 03:13:23PM +0800, Icenowy Zheng wrote:
> > CC:ing Icenowy, who added the regulator originally, with commit
> > 0ca12c1ee43c ("ARM: sun8i: r40: add 5V regulator for Banana Pi M2
> > Ultra").
...
> > Icenowy: can you clarify what "newer" version this was referring to
> > in
> > that commit message? That commit in itself doesn't seem to do
> > anything,
> > as the regulator isn't referenced, and it's not always-on. It's only
> > later when the USB nodes were added that it got used?
> > So was PH23 really necessary? As Hermann reports, setting PH23 on a
> > v1.1
> > makes it reboot.

diagnosed that futher now: PH23 is indeed powering the USB-Ports. Whats
happens ist that I powered the board through the micro USB port which turned
out to be limited to 900mA in axp221s. So the setting of reg 0x30 is
the real culprit: Setting the two lowest bits in this register allows
unlimited power over micro usb.

In U-Boot:
 i2c dev 0
 i2c mw 34 30 63

Or power the board through the barrel connector.

In all cases the kernel turns on USB-A power and boots.

> I am not sure, the schematics I have here (which says V1.0) have PH23
> as NC... Well, the M2 Berry schematics have PH23 as 5V EN, maybe I
> messed up M2U and M2B when developing?

While V1.0 didn't need the PH23 setup due to nc, V1.1 needs it. Maybe V1.1
was already on the horizon...

Thanks for the insights and your support, guys.

With seasons greetings
  Hermann

Re: Re: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Chen-Yu Tsai 11 months, 4 weeks ago

On Sat, Dec 21, 2024 at 1:21 AM Hermann Lauer
<Hermann.Lauer@iwr.uni-heidelberg.de> wrote:
>
> Hi,
>
> On Sat, Dec 14, 2024 at 03:13:23PM +0800, Icenowy Zheng wrote:
> > > CC:ing Icenowy, who added the regulator originally, with commit
> > > 0ca12c1ee43c ("ARM: sun8i: r40: add 5V regulator for Banana Pi M2
> > > Ultra").
> ...
> > > Icenowy: can you clarify what "newer" version this was referring to
> > > in
> > > that commit message? That commit in itself doesn't seem to do
> > > anything,
> > > as the regulator isn't referenced, and it's not always-on. It's only
> > > later when the USB nodes were added that it got used?
> > > So was PH23 really necessary? As Hermann reports, setting PH23 on a
> > > v1.1
> > > makes it reboot.
>
> diagnosed that futher now: PH23 is indeed powering the USB-Ports. Whats
> happens ist that I powered the board through the micro USB port which turned
> out to be limited to 900mA in axp221s. So the setting of reg 0x30 is
> the real culprit: Setting the two lowest bits in this register allows
> unlimited power over micro usb.
>
> In U-Boot:
>  i2c dev 0
>  i2c mw 34 30 63
>
> Or power the board through the barrel connector.
>
> In all cases the kernel turns on USB-A power and boots.
>
> > I am not sure, the schematics I have here (which says V1.0) have PH23
> > as NC... Well, the M2 Berry schematics have PH23 as 5V EN, maybe I
> > messed up M2U and M2B when developing?
>
> While V1.0 didn't need the PH23 setup due to nc, V1.1 needs it. Maybe V1.1
> was already on the horizon...

It seems this patch isn't needed then?


ChenYu

> Thanks for the insights and your support, guys.
>
> With seasons greetings
>   Hermann

Re: Re: Re: [PATCH v2] ARM: dts: sun8i-r40: remove unused GPIO regulator

Posted by Hermann.Lauer@uni-heidelberg.de 10 months ago

On Mon, Dec 23, 2024 at 01:18:18AM +0800, Chen-Yu Tsai wrote:
> > > > CC:ing Icenowy, who added the regulator originally, with commit
> > > > 0ca12c1ee43c ("ARM: sun8i: r40: add 5V regulator for Banana Pi M2
> > > > Ultra").
> > ...
> > > > Icenowy: can you clarify what "newer" version this was referring to
> > > > in
> > > > that commit message? That commit in itself doesn't seem to do
> > > > anything,
> > > > as the regulator isn't referenced, and it's not always-on. It's only
> > > > later when the USB nodes were added that it got used?
> > > > So was PH23 really necessary?
...
> > diagnosed that futher now: PH23 is indeed powering the USB-Ports. Whats
> > happens ist that I powered the board through the micro USB port which turned
> > out to be limited to 900mA in axp221s. So the setting of reg 0x30 is
> > the real culprit: Setting the two lowest bits in this register allows
> > unlimited power over micro usb.
...
> > > I am not sure, the schematics I have here (which says V1.0) have PH23
> > > as NC... Well, the M2 Berry schematics have PH23 as 5V EN, maybe I
> > > messed up M2U and M2B when developing?
> >
> > While V1.0 didn't need the PH23 setup due to nc, V1.1 needs it. Maybe V1.1
> > was already on the horizon...
> 
> It seems this patch isn't needed then?

It's plain wrong: The culprit is the power issue as discussed above. Strange
is the increase of power needed on V1.1, as V1.0 is working with the limit
in place.

As there are issues with the stability of the mmc driver even when disabling
mmc2 and mmc3, maybe something yet to be found changed in the mmc wirering
on V1.1 bords which increases power consumption compared to V1.0 with the
actual driver.

Sorry for the late reply - my time to look at this is scarse.
Thanks (especially to Andre for his help) and greetings
 Hermann