Meson-8b and Meson-gxbb Fix some missing code

PATCH Superseded Anand Moon 2 participants 15 messages Jul 16, 2021 – Aug 13, 2021

Subsystems (3)

Device tree bindings Arm/Amlogic Meson SoC support Generic PHY framework

lore

Share & tools

lore

Anand Moon

author

2021-07-16 10:37 UTC #0 lore raw

v2 cover Meson-8b and Meson-gxbb Fix some missing code

On Odroid C1+ and Odroid C2 USB feature is broken

It's being observed the after initiation of USB phy
the USB port goes in to suspend state, If we pass usbcore.autosuspend=-1 via command line USB hotplug seen to be working.

Another issue I observed is increase of USB interrupts event even if there is not much activity on USB ports.

$ cat /proc/interrupts | grep usb
 35:   26462800          0          0          0     GIC-0  63 Level
			c90c0000.usb, dwc2_hsotg:usb1

Changes added power node to usb phy and small code cleanup in usb phy.

Previous version RFC.
[0] https://patchwork.kernel.org/project/linux-amlogic/cover/[email protected]/
Dopped the reoder of code changes as of now.

V1 > changes Fixed the GPIO input signal on Odroid C1+/C2
New patch added to fix Odroid C2.

Thanks
-Anand

Anand Moon (4):
ARM: dts: meson8b: odroidc1: Add usb phy power node
arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb
power
phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset
mode
phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER

 arch/arm/boot/dts/meson8b-odroidc1.dts        | 19 +++++++++++++++++++
 .../boot/dts/amlogic/meson-gxbb-odroidc2.dts  |  7 +++----
 drivers/phy/amlogic/phy-meson8b-usb2.c        |  8 ++++++--
 3 files changed, 28 insertions(+), 6 deletions(-)

Signature · 1 line

2.32.0

Anand Moon

author

2021-07-16 10:37 UTC #1 lore raw

PATCH v2 1/4 ARM: dts: meson8b: odroidc1: Add usb phy power node

Add missing usb phy power node for phy mode fix below warning. P5V0 regulator suppy input voltage range to USB host controller. As descriped in the C1+ schematics, GPIO GPIOAO_5 is used to enable input power to USB ports, set it to Active Low.

[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
		in node /soc/cbus@c1100000/phy@8820 failed

Fixes 2eb79a4d15ff (ARM: dts: meson: enabling the USB Host

controller on Odroid-C1/C1+ board)

Cc Martin Blumenstingl <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
V1 > Fix the Input GPIO polarity from HIGH to LOW.

previous version
[0] https://patchwork.kernel.org/project/linux-amlogic/patch/[email protected]

changes fix the vbus-suppy to phy-supply, drop enable usb0

    USB_PWR                       2    1      0 unknown  5000mV     0mA  5000mV  5000mV
       phy-c1108820.phy.0-phy     2                                 0mA     0mV     0mV
---
 arch/arm/boot/dts/meson8b-odroidc1.dts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

arch/arm/boot/dts/meson8b-odroidc1.dts +19 -0

index c440ef94e082..26e8f91e0950 100644
@@ -32,6 +32,24 @@ emmc_pwrseq: emmc-pwrseq {
 		reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
 	};
 
+	usb_pwr_en: regulator-usb-pwr-en {
+		compatible = "regulator-fixed";
+
+		regulator-name = "USB_PWR";
+
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		/*
+		 * signal name from schematics: USB_POWER - P5V0
+		 */
+		vin-supply = <&p5v0>;
+
+		/*
+		 * signal name from schematics: PWREN - GPIOAO.BIT5
+		 */
+		gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
+	};
+
 	leds {
 		compatible = "gpio-leds";
 		blue {

@@ -378,6 +396,7 @@ &uart_AO {
 
 &usb1_phy {
 	status = "okay";
+	phy-supply = <&usb_pwr_en>;
 };
 
 &usb1 {

Signature · 1 line

2.32.0

Martin Blumenstingl

reviewer

2021-07-17 19:46 UTC #2 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Anand,

Quoted 3 lines · Anand Moon

Add missing usb phy power node for phy mode fix below warning. P5V0 regulator suppy input voltage range to USB host controller.

low prio - typo: suppy -> supply

Quoted 1 line

As descriped in the C1+ schematics, GPIO GPIOAO_5 is used to

low prio - typo: descriped -> described

Quoted 5 lines

enable input power to USB ports, set it to Active Low.

[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
                in node /soc/cbus@c1100000/phy@8820 failed

high prio:
Can you please describe how I can test this patch?
My concern is that previously I have tested your patch with ACTIVE_LOW and ACTIVE_HIGH polarity.
In both cases USB is working and I cannot observe any change (apart from this debug message being gone).

In the Odroid-C1 schematics (page 1) GPIOAO.BIT5 is connected to USB_OTG *only*. I cannot give my Acked-/Reviewed-/Tested-by without a description of how I can actually test the GPIO potion of this patch.

[...]

arch/arm/boot/dts/meson8b-odroidc1.dts emmc_pwrseq: emmc-pwrseq { · line 50

Quoted 4 lines

arch/arm/boot/dts/meson8b-odroidc1.dts +4 -0

+               /*
+                * signal name from schematics: PWREN - GPIOAO.BIT5
+                */
+               gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;

low prio:
Even though it's strictly not necessary I think this is confusing to read. Since there's no "enable-active-high" property the GPIO will be considered as "active low".
My suggestion is to change GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW

Also if you have any extra information since you last pinged me on IRC then it would be great if you could document it.
I am referring to these IRC message, where you are stating that the correct polarity should be "active high":
<armoon> xdarklight I have a question on USB GPIO Polarity on Odroid C1+ <armoon> As per the
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
<armoon> MP62551DGT-LF IC controls the power for the USB PORTS <armoon> https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf is MP62551DGT datasheet
<armoon> As per the data sheet in section ORDERING INFORMATION Active enable signal is set below MP62551DGT Active High

[...]

Quoted 3 lines

 &usb1_phy {
        status = "okay";
+ phy-supply = <&usb_pwr_en>;

medium priority:
I have raised the following concern in one of my previous emails on this topic:

Quoted 4 lines

The mistake I made before is considering USB VBUS as PHY power supply. I believe the USB PHY is actually powered by the AVDD18_USB_ADC and USB33_VDDIOH signals. See the S905 datasheet [0], page 25 These are 1.8V and 3.3V signals while you are adding a 5V regulator.

you replied with:

Quoted 1 line

OK, thanks.

so I don't understand what "OK, thanks" means.
If it means "Martin, you are wrong" then please provide a description so I can also learn something!

Best regards,
Martin

Signature · 3 lines

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Anand Moon

author

2021-07-18 03:38 UTC #3 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Martin,

Thanks for the review comments.

Quoted 12 lines · Martin Blumenstingl

Hi Anand,
Quoted 3 lines · Anand Moon
Add missing usb phy power node for phy mode fix below warning. P5V0 regulator suppy input voltage range to USB host controller.
low prio - typo: suppy -> supply
Quoted 1 line
As descriped in the C1+ schematics, GPIO GPIOAO_5 is used to
low prio - typo: descriped -> described

I definitely run checkpatch with the spellcheck option but I need to improve on this aspect.

Quoted 21 lines

Quoted 5 lines
enable input power to USB ports, set it to Active Low.
[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
                in node /soc/cbus@c1100000/phy@8820 failed
high prio:
Can you please describe how I can test this patch?
My concern is that previously I have tested your patch with ACTIVE_LOW and ACTIVE_HIGH polarity.
In both cases USB is working and I cannot observe any change (apart from this debug message being gone).
In the Odroid-C1 schematics (page 1) GPIOAO.BIT5 is connected to USB_OTG *only*. I cannot give my Acked-/Reviewed-/Tested-by without a description of how I can actually test the GPIO potion of this patch.
[...]
arch/arm/boot/dts/meson8b-odroidc1.dts emmc_pwrseq: emmc-pwrseq { · line 50
Quoted 4 lines
arch/arm/boot/dts/meson8b-odroidc1.dts +4 -0
+               /*
+                * signal name from schematics: PWREN - GPIOAO.BIT5
+                */
+               gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;

Quoted 6 lines

low prio:
Even though it's strictly not necessary I think this is confusing to read. Since there's no "enable-active-high" property the GPIO will be considered as "active low".
My suggestion is to change GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW

My apologies, I might have sent the wrong set of patches its GPIO_ACTIVE_LOW I meant
I have formatted with and in the course of testing and verifying It might have got SKIPPED.
I didn't do this intentionally it happen with a mistake with my two repositories.
I don't intend to repeat my mistake, again and again, *sorry for the trouble*.

Quoted 4 lines

Also if you have any extra information since you last pinged me on IRC then it would be great if you could document it.
I am referring to these IRC message, where you are stating that the correct polarity should be "active high":

Quoted 9 lines

<armoon> xdarklight I have a question on USB GPIO Polarity on Odroid C1+ <armoon> As per the
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
<armoon> MP62551DGT-LF IC controls the power for the USB PORTS <armoon> https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf is MP62551DGT datasheet
<armoon> As per the data sheet in section ORDERING INFORMATION Active enable signal is set below MP62551DGT Active High

[1] https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf

I have read the datasheets MP62551DGT EN* pin its Enable input. Active Low:

       *EN I Enable input. Active Low: (MP62550), Active High: (MP62551).*

I have tested with ACTIVE_LOW and followed all the steps invalidating
this with debugfs output.

Last login: Tue Jul 13 00:02:49 2021 from 10.0.0.14 [alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out lo ACTIVE LOW

Quoted 10 lines

[...]
Quoted 3 lines
 &usb1_phy {
        status = "okay";
+ phy-supply = <&usb_pwr_en>;
medium priority:
I have raised the following concern in one of my previous emails on this topic:
Quoted 4 lines
The mistake I made before is considering USB VBUS as PHY power supply. I believe the USB PHY is actually powered by the AVDD18_USB_ADC and USB33_VDDIOH signals. See the S905 datasheet [0], page 25 These are 1.8V and 3.3V signals while you are adding a 5V regulator.

Quoted 5 lines

you replied with:
Quoted 1 line
OK, thanks.
so I don't understand what "OK, thanks" means.
If it means "Martin, you are wrong" then please provide a description so I can also learn something!

Yes, I understood your inputs. But from the logs below is seen to looking for phy-supply
This is the reason I went ahead with phy-supply as the core phy node needs this property.

Please check below commit
e841ec956e53 ("ARM64: dts: meson-gxbb-odroidc2: fix usb1 power supply")

[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
in node /soc/cbus@c1100000/phy@8820 failed
[    1.255031] pwm-regulator regulator-vcck: Failed to get PWM, deferring probe
[    1.256730] pwm-regulator regulator-vddee: Failed to get PWM, deferring probe
[    1.260300] dwc2 c90c0000.usb: mapped PA c90c0000 to VA (ptrval)
[    1.260446] dwc2 c90c0000.usb: Looking up vusb_d-supply from device tree
[    1.260460] dwc2 c90c0000.usb: Looking up vusb_d-supply property in
node /soc/usb@c90c0000 failed
[    1.260490] dwc2 c90c0000.usb: supply vusb_d not found, using dummy regulator
[    1.260606] dwc2 c90c0000.usb: Looking up vusb_a-supply from device tree
[    1.260620] dwc2 c90c0000.usb: Looking up vusb_a-supply property in
node /soc/usb@c90c0000 failed
[    1.260641] dwc2 c90c0000.usb: supply vusb_a not found, using dummy regulator
[    1.260717] dwc2 c90c0000.usb: registering common handler for irq35
[    1.260772] dwc2 c90c0000.usb: Looking up vbus-supply from device tree
[    1.260784] dwc2 c90c0000.usb: Looking up vbus-supply property in
node /soc/usb@c90c0000 failed
[    1.261949] dwc2 c90c0000.usb: Core Release: 3.10a (snpsid=4f54310a)

Quoted 4 lines

Best regards,
Martin

-Anand

Martin Blumenstingl

reviewer

2021-07-18 11:37 UTC #4 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Anand,

On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <[email protected]> wrote:
[...]

Quoted 12 lines

Quoted 12 lines
Quoted 5 lines
enable input power to USB ports, set it to Active Low.
[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
                in node /soc/cbus@c1100000/phy@8820 failed
high prio:
Can you please describe how I can test this patch?
My concern is that previously I have tested your patch with ACTIVE_LOW and ACTIVE_HIGH polarity.
In both cases USB is working and I cannot observe any change (apart from this debug message being gone).

Quoted 3 lines

Quoted 3 lines
In the Odroid-C1 schematics (page 1) GPIOAO.BIT5 is connected to USB_OTG *only*. I cannot give my Acked-/Reviewed-/Tested-by without a description of how I can actually test the GPIO potion of this patch.

This question is still open.
Even with all your explanations below I am missing a way to verify if GPIOAO_5 is the correct GPIO to use.

Quoted 11 lines

Quoted 11 lines
[...]
arch/arm/boot/dts/meson8b-odroidc1.dts emmc_pwrseq: emmc-pwrseq { · line 50
Quoted 4 lines
arch/arm/boot/dts/meson8b-odroidc1.dts +4 -0
+               /*
+                * signal name from schematics: PWREN - GPIOAO.BIT5
+                */
+               gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
low prio:
Even though it's strictly not necessary I think this is confusing to read. Since there's no "enable-active-high" property the GPIO will be considered as "active low".
My suggestion is to change GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW

Quoted 7 lines

My apologies, I might have sent the wrong set of patches its GPIO_ACTIVE_LOW I meant
I have formatted with and in the course of testing and verifying It might have got SKIPPED.
I didn't do this intentionally it happen with a mistake with my two repositories.
I don't intend to repeat my mistake, again and again, *sorry for the trouble*.

no worries, that's why I mentioned that it's low priority

Quoted 19 lines

Quoted 4 lines
Also if you have any extra information since you last pinged me on IRC then it would be great if you could document it.
I am referring to these IRC message, where you are stating that the correct polarity should be "active high":
Quoted 9 lines
<armoon> xdarklight I have a question on USB GPIO Polarity on Odroid C1+ <armoon> As per the
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
<armoon> MP62551DGT-LF IC controls the power for the USB PORTS <armoon> https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf is MP62551DGT datasheet
<armoon> As per the data sheet in section ORDERING INFORMATION Active enable signal is set below MP62551DGT Active High
[1] https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf
I have read the datasheets MP62551DGT EN* pin its Enable input. Active Low:
       *EN I Enable input. Active Low: (MP62550), Active High: (MP62551).*

Quoted 7 lines

I have tested with ACTIVE_LOW and followed all the steps invalidating
this with debugfs output.
Last login: Tue Jul 13 00:02:49 2021 from 10.0.0.14 [alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out lo ACTIVE LOW

This means that whatever is configured in the .dts is also showing up in debugfs.
It doesn't mean that the correct GPIO or polarity is used -> that is why I want to understand how to actually test this patch. Technically I can write a patch that makes GPIOAO_13 (which is connected to the status LED) show up as being used by regulator-usb-pwr-en in debugfs.

[...]

Quoted 9 lines

Quoted 9 lines
Quoted 3 lines
 &usb1_phy {
        status = "okay";
+ phy-supply = <&usb_pwr_en>;
medium priority:
I have raised the following concern in one of my previous emails on this topic:
Quoted 4 lines
The mistake I made before is considering USB VBUS as PHY power supply. I believe the USB PHY is actually powered by the AVDD18_USB_ADC and USB33_VDDIOH signals. See the S905 datasheet [0], page 25 These are 1.8V and 3.3V signals while you are adding a 5V regulator.

Quoted 8 lines

Quoted 5 lines
you replied with:
Quoted 1 line
OK, thanks.
so I don't understand what "OK, thanks" means.
If it means "Martin, you are wrong" then please provide a description so I can also learn something!
Yes, I understood your inputs. But from the logs below is seen to looking for phy-supply

This sentence is correct

Quoted 2 lines

This is the reason I went ahead with phy-supply as the core phy node needs this property.

This sentence is not correct
From drivers/phy/phy-core.c:

    phy->pwr = regulator_get_optional(&phy->dev, "phy");

As you can see, the "phy-supply" regulator is marked as optional in the PHY subsystem.
This matches with Documentation/devicetree/bindings/phy/phy-bindings.txt where "phy-supply" is marked as an optional property.

Quoted 2 lines

Please check below commit
e841ec956e53 ("ARM64: dts: meson-gxbb-odroidc2: fix usb1 power supply")

That commit is from 2017. You'll also find some commits where I am also using the phy-supply property (I didn't know better back then). However, in 2018 things changed when the dwc2 driver gained a vbus-supply property in commit 531ef5ebea9639 ("usb: dwc2: add support for host mode external vbus supply")
So for new .dts support phy-supply should not be used anymore for VBUS because phy-supply (described as "Phandle to a regulator that provides power to the PHY." in Documentation/devicetree/bindings/phy/phy-bindings.txt) and vbus-supply are two different things.

Best regards,
Martin

Signature · 3 lines

linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy

Anand Moon

author

2021-07-18 13:29 UTC #5 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Martin,

Thanks for your valuable feedback,

Quoted 17 lines · Martin Blumenstingl

Hi Anand,
On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <[email protected]> wrote:
[...]
Quoted 12 lines
Quoted 12 lines
Quoted 5 lines
enable input power to USB ports, set it to Active Low.
[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
                in node /soc/cbus@c1100000/phy@8820 failed
high prio:
Can you please describe how I can test this patch?
My concern is that previously I have tested your patch with ACTIVE_LOW and ACTIVE_HIGH polarity.
In both cases USB is working and I cannot observe any change (apart from this debug message being gone).

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
In the Odroid-C1 schematics (page 1) GPIOAO.BIT5 is connected to USB_OTG *only*. I cannot give my Acked-/Reviewed-/Tested-by without a description of how I can actually test the GPIO potion of this patch.
This question is still open.
Even with all your explanations below I am missing a way to verify if GPIOAO_5 is the correct GPIO to use.

From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf

You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.

Hope I am clean in my thought process now.
.

Quoted 47 lines

Quoted 12 lines
Quoted 6 lines
[...]
arch/arm/boot/dts/meson8b-odroidc1.dts emmc_pwrseq: emmc-pwrseq { · line 50
Quoted 4 lines
arch/arm/boot/dts/meson8b-odroidc1.dts +4 -0
+               /*
+                * signal name from schematics: PWREN - GPIOAO.BIT5
+                */
+               gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
Quoted 6 lines
low prio:
Even though it's strictly not necessary I think this is confusing to read. Since there's no "enable-active-high" property the GPIO will be considered as "active low".
My suggestion is to change GPIO_ACTIVE_HIGH to GPIO_ACTIVE_LOW
Quoted 7 lines
My apologies, I might have sent the wrong set of patches its GPIO_ACTIVE_LOW I meant
I have formatted with and in the course of testing and verifying It might have got SKIPPED.
I didn't do this intentionally it happen with a mistake with my two repositories.
I don't intend to repeat my mistake, again and again, *sorry for the trouble*.
no worries, that's why I mentioned that it's low priority
Quoted 19 lines
Quoted 4 lines
Also if you have any extra information since you last pinged me on IRC then it would be great if you could document it.
I am referring to these IRC message, where you are stating that the correct polarity should be "active high":
Quoted 9 lines
<armoon> xdarklight I have a question on USB GPIO Polarity on Odroid C1+ <armoon> As per the
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
<armoon> MP62551DGT-LF IC controls the power for the USB PORTS <armoon> https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf is MP62551DGT datasheet
<armoon> As per the data sheet in section ORDERING INFORMATION Active enable signal is set below MP62551DGT Active High
[1] https://www.mouser.com/datasheet/2/277/MP62550-1384050.pdf
I have read the datasheets MP62551DGT EN* pin its Enable input. Active Low:
       *EN I Enable input. Active Low: (MP62550), Active High: (MP62551).*
Quoted 7 lines
I have tested with ACTIVE_LOW and followed all the steps invalidating
this with debugfs output.
Last login: Tue Jul 13 00:02:49 2021 from 10.0.0.14 [alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out lo ACTIVE LOW

Quoted 7 lines

This means that whatever is configured in the .dts is also showing up in debugfs.
It doesn't mean that the correct GPIO or polarity is used -> that is why I want to understand how to actually test this patch. Technically I can write a patch that makes GPIOAO_13 (which is connected to the status LED) show up as being used by regulator-usb-pwr-en in debugfs.

Yep, you are correct, If I used GPIOAO_13 wrong pin, it will not enable the USB power. See below.
[alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi

So correct way with gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;

[alarm@archl-c1e linux-amlogic-5.y-devel]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out lo ACTIVE LOW

The reason for USB ports has power without this patch is applied. Please check schematics S805 USB2 SDB
USB_VDD33 is powered with VDDIO_AO3V3 regulator directly along with SD CARD.

Quoted 33 lines

[...]
Quoted 9 lines
Quoted 9 lines
Quoted 3 lines
 &usb1_phy {
        status = "okay";
+ phy-supply = <&usb_pwr_en>;
medium priority:
I have raised the following concern in one of my previous emails on this topic:
Quoted 4 lines
The mistake I made before is considering USB VBUS as PHY power supply. I believe the USB PHY is actually powered by the AVDD18_USB_ADC and USB33_VDDIOH signals. See the S905 datasheet [0], page 25 These are 1.8V and 3.3V signals while you are adding a 5V regulator.
Quoted 8 lines
Quoted 5 lines
you replied with:
Quoted 1 line
OK, thanks.
so I don't understand what "OK, thanks" means.
If it means "Martin, you are wrong" then please provide a description so I can also learn something!
Yes, I understood your inputs. But from the logs below is seen to looking for phy-supply
This sentence is correct
Quoted 2 lines
This is the reason I went ahead with phy-supply as the core phy node needs this property.
This sentence is not correct
From drivers/phy/phy-core.c:
    phy->pwr = regulator_get_optional(&phy->dev, "phy");
As you can see, the "phy-supply" regulator is marked as optional in the PHY subsystem.
This matches with Documentation/devicetree/bindings/phy/phy-bindings.txt where "phy-supply" is marked as an optional property.

Quoted 7 lines

Quoted 2 lines
Please check below commit
e841ec956e53 ("ARM64: dts: meson-gxbb-odroidc2: fix usb1 power supply")
That commit is from 2017. You'll also find some commits where I am also using the phy-supply property (I didn't know better back then). However, in 2018 things changed when the dwc2 driver gained a vbus-supply property in commit 531ef5ebea9639 ("usb: dwc2: add support for host mode external vbus supply")

OK.

Quoted 6 lines

So for new .dts support phy-supply should not be used anymore for VBUS because phy-supply (described as "Phandle to a regulator that provides power to the PHY." in Documentation/devicetree/bindings/phy/phy-bindings.txt) and vbus-supply are two different things.

It just came to my notice your email on this issue sees below.
[0] https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
[1] https://patchwork.kernel.org/patch/10868515/
[2] https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=d8b475212bbf9e5f80c1c923a9701dca5ceb23e2

From the openwrt commit d8b475212bbf9e5f80c1c923a9701dca5ceb23e2 and binding yaml [3]
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/dwc2.yaml#L87

USB DWC2 power is linked to vbus-supply, so it should be moved to usb node. Now I am getting your point correctly.

[    1.260460] dwc2 c90c0000.usb: Looking up vusb_d-supply property in
node /soc/usb@c90c0000 failed
[    1.260490] dwc2 c90c0000.usb: supply vusb_d not found, using dummy regulator
[    1.260606] dwc2 c90c0000.usb: Looking up vusb_a-supply from device tree
[    1.260620] dwc2 c90c0000.usb: Looking up vusb_a-supply property in
node /soc/usb@c90c0000 failed
[    1.260641] dwc2 c90c0000.usb: supply vusb_a not found, using dummy regulator
[    1.260717] dwc2 c90c0000.usb: registering common handler for irq35
[    1.260772] dwc2 c90c0000.usb: Looking up vbus-supply from device tree
[    1.260784] dwc2 c90c0000.usb: Looking up vbus-supply property in

node /soc/usb@c90c0000 failed

Thanks for the input, I will update the vbus-supply in the next version to USB nodes.

Quoted 3 lines

Best regards,
Martin

Thanks
-Anand

Martin Blumenstingl

reviewer

2021-07-18 14:01 UTC #6 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Anand,

Quoted 31 lines · Anand Moon

Hi Martin,
Thanks for your valuable feedback,
Quoted 17 lines · Martin Blumenstingl
Hi Anand,
On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <[email protected]> wrote:
[...]
Quoted 12 lines
Quoted 12 lines
Quoted 5 lines
enable input power to USB ports, set it to Active Low.
[    1.253149] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.253166] phy phy-c1108820.phy.0: Looking up phy-supply property
                in node /soc/cbus@c1100000/phy@8820 failed
high prio:
Can you please describe how I can test this patch?
My concern is that previously I have tested your patch with ACTIVE_LOW and ACTIVE_HIGH polarity.
In both cases USB is working and I cannot observe any change (apart from this debug message being gone).
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
In the Odroid-C1 schematics (page 1) GPIOAO.BIT5 is connected to USB_OTG *only*. I cannot give my Acked-/Reviewed-/Tested-by without a description of how I can actually test the GPIO potion of this patch.
This question is still open.
Even with all your explanations below I am missing a way to verify if GPIOAO_5 is the correct GPIO to use.

Quoted 8 lines

From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.
Hope I am clean in my thought process now.

Can you please point out the page numbers for me?

What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG PWREN signal (that's the green box with the label "USB_OTG.SchDoc" above it).

Below (still on page 1) there's a whole section for USB_HOST_PWR_SW.SchDoc and USB_HUB_GL852G-OHG.SchDoc.
This box also uses a PWREN signal, but from my understanding it's a different one:
- PWREN in this area is an OUTPUT on the USB hub (USB_HUB_GL852G-OHG.SchDoc). This is confirmed by the PWREN signals on page 14 of the GL852 datasheet: [4]
- Then PWREN is an INPUT for the overcurrent switches (USB_HOST_PWR_SW.SchDoc) This section does not mention GPIOAO_5 (or GPIOAO.BIT5) at all.

For better illustration I have attached a screenshot with some annotations from the schematics.
This is how I understand the whole USB setup.
This doesn't mean that I am right though.

[...]

Quoted 8 lines

Quoted 3 lines
Technically I can write a patch that makes GPIOAO_13 (which is connected to the status LED) show up as being used by regulator-usb-pwr-en in debugfs.
Yep, you are correct, If I used GPIOAO_13 wrong pin, it will not enable the USB power. See below.
[alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi

Did you also delete the "leds" node from meson8b-odroidc1.dts?

[...]

Quoted 12 lines

Quoted 6 lines
So for new .dts support phy-supply should not be used anymore for VBUS because phy-supply (described as "Phandle to a regulator that provides power to the PHY." in Documentation/devicetree/bindings/phy/phy-bindings.txt) and vbus-supply are two different things.
It just came to my notice your email on this issue sees below.
[0] https://patchwork.kernel.org/project/linux-usb/patch/[email protected]/
[1] https://patchwork.kernel.org/patch/10868515/
[2] https://git.openwrt.org/?p=openwrt/openwrt.git;a=commitdiff;h=d8b475212bbf9e5f80c1c923a9701dca5ceb23e2

Quoted 6 lines

From the openwrt commit d8b475212bbf9e5f80c1c923a9701dca5ceb23e2 and binding yaml [3]
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/usb/dwc2.yaml#L87
USB DWC2 power is linked to vbus-supply, so it should be moved to usb node. Now I am getting your point correctly.

Yes, I think we're now on the same page on this topic
I totally forgot about some of these patches - great that you found them and added them here for further documentation of this topic!

Best regards,
Martin

[4] https://datasheet.lcsc.com/szlcsc/Genesys-Logic-GL852G-HHG12_C136618.pdf

Martin Blumenstingl

reviewer

2021-07-18 19:09 UTC #7 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Anand,

On Sun, Jul 18, 2021 at 4:01 PM Martin Blumenstingl <[email protected]> wrote:
[...]

Quoted 7 lines

Quoted 7 lines
From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.

Quoted 7 lines

Quoted 1 line
Hope I am clean in my thought process now.
Can you please point out the page numbers for me?
What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG PWREN signal (that's the green box with the label "USB_OTG.SchDoc" above it).

I unfortunately missed your question on IRC on this topic:
<armoon> xdarklight: just want to understand the confusion on PWREN, either is INPUT / OTPUT to usb hub am I correct
My understanding of the INPUT/OUTPUT direction in the Odroid-C1 schematics (which you linked) above is that the arrow direction shows whether it's an input or output.
For example (all on page one):
- HDMI_HPD is an input to the Meson8b SoC, arrow direction is: HDMI => S805
- IR_IN is an input to the Meson8b SoC, arrow direction is: S805 <= GPIOAO.BIT7 IR_IN
- PWM is an output, arrow direction is: S805 => PWM.C VCCK_PWM
- 3.3V/1.8V switch for the SD card is an output, arrow direction is:
S805 => GPIOAO.BIT3 TF_3V3N_1V8_EN

That tells me: the arrow direction shows which connections are inputs or outputs.
Some pins can be input and output at the same time (HDMI_CEC). But let's keep it simple for now any only look at inputs/output.

Based on this information I am looking at the arrow direction for
USB_OTG: GPIOAO.BIT5 PWREN <= S805
So this confirms that GPIOAO_5 is an output to the SoC. Please note though that this part is not linked with any USB host connector, it's only linked to the USB OTG path.

For the next part the S805 SoC is not involved:
USB1 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
USB2 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
This tells me that the PWREN signal for the two USB_HOST_PWR_SW (USB host power switches / current limiters) is driven by the GL852G USB hub.

The GL852G USB hub datasheet [0] mentions on page 14:
pin name: PWREN1#~4
I/O type: O (which stands for: OUTPUT)
description: Active low. Power enable output for DSPORT1~4 PWREN1# is the only power-enable output for GANG mode.
This seems to confirm my understanding of the Odroid-C1 schematics, where the PWREN signal of the current limiters is coming from the USB hub (so the Meson8b SoC is not involved directly).

<armoon> sorry I am not the expert in schematics interpretation, but what I understand is PWREN (GPIOAO.BIT5) is input signal to MP62551DGT-LF-Z which controls the VBUS signal power to USB HUB to pin V5 on GL852G IC
That part is unfortunately hard to say because the author of the Odroid-C1 schematics decided to two different signals called PWREN. Now it's not clear if this refers to the USB_OTG PWREN or USB_HOST PWREN signal.
Based on page 1 I *assume* that it's linked to the USB_HOST PWREN signal.

<armoon> plz check the GL852G power section. it has 5V Power
input. It need be NC if using external regulator
<armoon> 5V and V33 are the regulator for GL852G IC HUB, Looking at page 27 of the Odroid-C1 schematics I can see that PSELF, PGANG, V33, AVDD1, AVDD2, AVDD3 and DVDD are supplied by a 3.3V power supply called HUB_AVDD.
V5 is a 5V signal which is not connected (there's a red cross next to it).

<armoon> I feel setting GPIOAO.BIT5 setting to Active Low just enable the 5V via external regulator MP62551DGT-LF-Z
My understanding is that GPIOAO.BIT5 enables or disables VBUS for the USB_OTG port (&usb1 in our .dts)
See page 28 of the schematics, the RT9715EGB power switch uses a PWREN signal for it's ENABLE pin based on page 1 of the schematics. MP62551DGT-LF-Z would then be getting it's PWREN signal from the GL852G hub.

Having different power switches for USB HOST and USB OTG makes sense because for USB OTG the VBUS signal needs to be disabled when in peripheral/device mode (for example when connecting the Odroid-C1 via micro-USB cable to a computer).

<armoon> I don't know much on internal IC signal apart from this, may be I need to check with the board designer Odroid / Hardkernel for more details
That would be great actually. Unfortunately Brian Kim's email address (which you had Cc'ed in the beginning of this conversation) is not valid anymore.
If you have any other contact at Hardkernel then it would be great if you could add them to the loop so they can clear our questions.

Best regards,
Martin

[0] https://datasheet.lcsc.com/szlcsc/Genesys-Logic-GL852G-HHG12_C136618.pdf

Martin Blumenstingl

reviewer

2021-08-04 21:58 UTC #8 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Quoted 45 lines · Martin Blumenstingl

Hi Anand,
On Sun, Jul 18, 2021 at 4:01 PM Martin Blumenstingl <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 7 lines
From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.
Quoted 7 lines
Quoted 1 line
Hope I am clean in my thought process now.
Can you please point out the page numbers for me?
What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG PWREN signal (that's the green box with the label "USB_OTG.SchDoc" above it).
I unfortunately missed your question on IRC on this topic:
<armoon> xdarklight: just want to understand the confusion on PWREN, either is INPUT / OTPUT to usb hub am I correct
My understanding of the INPUT/OUTPUT direction in the Odroid-C1 schematics (which you linked) above is that the arrow direction shows whether it's an input or output.
For example (all on page one):
- HDMI_HPD is an input to the Meson8b SoC, arrow direction is: HDMI => S805
- IR_IN is an input to the Meson8b SoC, arrow direction is: S805 <= GPIOAO.BIT7 IR_IN
- PWM is an output, arrow direction is: S805 => PWM.C VCCK_PWM
- 3.3V/1.8V switch for the SD card is an output, arrow direction is:
S805 => GPIOAO.BIT3 TF_3V3N_1V8_EN
That tells me: the arrow direction shows which connections are inputs or outputs.
Some pins can be input and output at the same time (HDMI_CEC). But let's keep it simple for now any only look at inputs/output.
Based on this information I am looking at the arrow direction for
USB_OTG: GPIOAO.BIT5 PWREN <= S805
So this confirms that GPIOAO_5 is an output to the SoC. Please note though that this part is not linked with any USB host connector, it's only linked to the USB OTG path.

Quoted 6 lines

For the next part the S805 SoC is not involved:
USB1 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
USB2 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
This tells me that the PWREN signal for the two USB_HOST_PWR_SW (USB host power switches / current limiters) is driven by the GL852G USB hub.

Hardkernel team responded on the Odroid forums to the request from Anand:
"Power-Enable signal to the USB load switch ICs is connected to the USB hub controller.
It is not connected the CPU GPIO."

The regulator from Anand's patch is therefore connected to the micro USB port. This means that the patch needs updating in case Anand wants to have it included.

Best regards,
Martin

[0] https://forum.odroid.com/viewtopic.php?p=332217#p332217

Anand Moon

author

2021-08-05 18:18 UTC #9 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Martin,

Quoted 54 lines · Martin Blumenstingl

Quoted 45 lines · Martin Blumenstingl
Hi Anand,
On Sun, Jul 18, 2021 at 4:01 PM Martin Blumenstingl <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 7 lines
From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.
Quoted 7 lines
Quoted 1 line
Hope I am clean in my thought process now.
Can you please point out the page numbers for me?
What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG PWREN signal (that's the green box with the label "USB_OTG.SchDoc" above it).
I unfortunately missed your question on IRC on this topic:
<armoon> xdarklight: just want to understand the confusion on PWREN, either is INPUT / OTPUT to usb hub am I correct
My understanding of the INPUT/OUTPUT direction in the Odroid-C1 schematics (which you linked) above is that the arrow direction shows whether it's an input or output.
For example (all on page one):
- HDMI_HPD is an input to the Meson8b SoC, arrow direction is: HDMI => S805
- IR_IN is an input to the Meson8b SoC, arrow direction is: S805 <= GPIOAO.BIT7 IR_IN
- PWM is an output, arrow direction is: S805 => PWM.C VCCK_PWM
- 3.3V/1.8V switch for the SD card is an output, arrow direction is:
S805 => GPIOAO.BIT3 TF_3V3N_1V8_EN
That tells me: the arrow direction shows which connections are inputs or outputs.
Some pins can be input and output at the same time (HDMI_CEC). But let's keep it simple for now any only look at inputs/output.
Based on this information I am looking at the arrow direction for
USB_OTG: GPIOAO.BIT5 PWREN <= S805
So this confirms that GPIOAO_5 is an output to the SoC. Please note though that this part is not linked with any USB host connector, it's only linked to the USB OTG path.
Quoted 6 lines
For the next part the S805 SoC is not involved:
USB1 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
USB2 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
This tells me that the PWREN signal for the two USB_HOST_PWR_SW (USB host power switches / current limiters) is driven by the GL852G USB hub.

Quoted 9 lines

Hardkernel team responded on the Odroid forums to the request from Anand:
"Power-Enable signal to the USB load switch ICs is connected to the USB hub controller.
It is not connected the CPU GPIO."
The regulator from Anand's patch is therefore connected to the micro USB port. This means that the patch needs updating in case Anand wants to have it included.

Yes, I will send a corrected version of the patches soon.

Quoted 6 lines

Best regards,
Martin
[0] https://forum.odroid.com/viewtopic.php?p=332217#p332217

Thanks
-Anand

Anand Moon

author

2021-08-13 19:35 UTC #10 lore raw

Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Martin,

Quoted 64 lines · Anand Moon

Hi Martin,
Quoted 59 lines · Martin Blumenstingl
Quoted 45 lines · Martin Blumenstingl
Hi Anand,
On Sun, Jul 18, 2021 at 4:01 PM Martin Blumenstingl <[email protected]> wrote:
[...]
Quoted 7 lines
Quoted 7 lines
From the schematics [1]
https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf
You could find references to PWREN <--- GPIOAO.BIT5 The second reference is USB HOST Power Switch The third reference is USB HOST POWER.
Quoted 7 lines
Quoted 1 line
Hope I am clean in my thought process now.
Can you please point out the page numbers for me?
What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG PWREN signal (that's the green box with the label "USB_OTG.SchDoc" above it).
I unfortunately missed your question on IRC on this topic:
<armoon> xdarklight: just want to understand the confusion on PWREN, either is INPUT / OTPUT to usb hub am I correct
My understanding of the INPUT/OUTPUT direction in the Odroid-C1 schematics (which you linked) above is that the arrow direction shows whether it's an input or output.
For example (all on page one):
- HDMI_HPD is an input to the Meson8b SoC, arrow direction is: HDMI => S805
- IR_IN is an input to the Meson8b SoC, arrow direction is: S805 <= GPIOAO.BIT7 IR_IN
- PWM is an output, arrow direction is: S805 => PWM.C VCCK_PWM
- 3.3V/1.8V switch for the SD card is an output, arrow direction is:
S805 => GPIOAO.BIT3 TF_3V3N_1V8_EN
That tells me: the arrow direction shows which connections are inputs or outputs.
Some pins can be input and output at the same time (HDMI_CEC). But let's keep it simple for now any only look at inputs/output.
Based on this information I am looking at the arrow direction for
USB_OTG: GPIOAO.BIT5 PWREN <= S805
So this confirms that GPIOAO_5 is an output to the SoC. Please note though that this part is not linked with any USB host connector, it's only linked to the USB OTG path.
Quoted 6 lines
For the next part the S805 SoC is not involved:
USB1 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
USB2 USB_HOST_PWR_SW PWREN <= PWREN USB_HUB_GL852G
This tells me that the PWREN signal for the two USB_HOST_PWR_SW (USB host power switches / current limiters) is driven by the GL852G USB hub.
Hardkernel team responded on the Odroid forums to the request from Anand:
"Power-Enable signal to the USB load switch ICs is connected to the USB hub controller.
It is not connected the CPU GPIO."

Quoted 6 lines

Quoted 4 lines
The regulator from Anand's patch is therefore connected to the micro USB port. This means that the patch needs updating in case Anand wants to have it included.
Yes, I will send a corrected version of the patches soon.

I have a small query regarding GPIO polarity

As per the datasheet of *MP62551DGT-LF-Z*
[0] https://www.mouser.in/datasheet/2/277/MP62550-1384050.pdf

USB power enables EN signal need to be Active High.

PIN Fucntion > Enable input. Active Low: (MP62550), Active High: (MP62551).

So the correct option in dts
+ /*
+ * signal name from schematics: PWREN - GPIOAO.BIT5
+ */
+ gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
+ enable-active-high;

which gives me the below output.

$  sudo cat /sys/kernel/debug/gpio | grep USB
 gpio-1953 (USB_HUB_RST_N       |usb-hub-reset       ) out hi
 gpio-1954 (USB_OTG_PWREN       |regulator-usb-pwr-en) out hi

Please clear this small query so that I can send the update patch.

Thanks
-Anand

Quoted 10 lines

Quoted 7 lines
Best regards,
Martin
[0] https://forum.odroid.com/viewtopic.php?p=332217#p332217
Thanks
-Anand

Anand Moon

author

2021-07-16 10:38 UTC #11 lore raw

PATCH v2 2/4 arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power

In reply to Anand Moon

Fix the chip enable signal changing from Active High to Active Low to enable input power to USB power. Also updated signal name as per the shematics.

Fixes 5a0803bd5ae2 ("ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes")

Cc Martin Blumenstingl <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
New patch in this series

---
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts +3 -4

index 201596247fd9..3f4f16a5dedd 100644
@@ -38,12 +38,11 @@ usb_otg_pwr: regulator-usb-pwrs {
 		regulator-max-microvolt = <5000000>;
 
 		/*
-		 * signal name from schematics: PWREN
+		 * signal name from schematics: PWREN - GPIOAO.BIT5
 		 */
-		gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
-		enable-active-high;
+		gpios = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
 		/*
-		 * signal name from schematics: USB_POWER
+		 * signal name from schematics: USB_POWER - P5V0
 		 */
 		vin-supply = <&p5v0>;
 	};

Signature · 1 line

2.32.0

Martin Blumenstingl

reviewer

2021-07-17 19:59 UTC #12 lore raw

Re: [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power

Hi Anand,

Quoted 4 lines · Anand Moon

Fix the chip enable signal changing from Active High to Active Low to enable input power to USB power. Also updated signal name as per the shematics.

typo: shematics -> schematics

Quoted 4 lines

Fixes 5a0803bd5ae2 ("ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes")
Cc Martin Blumenstingl <[email protected]>

I don't have an Odroid-C2 so I will not be able to review this as well as the Odroid-C1 patch.
My suggestion is that we discuss the Odroid-C1 patch first as that's some hardware that we both have.
Once I understand how the Odroid-C1 patch works I can at least give my Acked-by on the Odroid-C2 change.

Best regards,
Martin

Anand Moon

author

2021-07-16 10:38 UTC #13 lore raw

PATCH v2 3/4 phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset mode

In reply to Anand Moon

Power off the PHY by putting it into reset mode.

Cc Martin Blumenstingl <[email protected]>

Acked-by Martin Blumenstingl <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
None: Added Martin's Ack

---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/phy/amlogic/phy-meson8b-usb2.c +4 -0

index 03c061dd5f0d..2aad45c55494 100644
@@ -219,6 +219,10 @@ static int phy_meson8b_usb2_power_off(struct phy *phy)
 	clk_disable_unprepare(priv->clk_usb);
 	clk_disable_unprepare(priv->clk_usb_general);
 
+	/* power off the PHY by putting it into reset mode */
+	regmap_update_bits(priv->regmap, REG_CTRL, REG_CTRL_POWER_ON_RESET,
+			   REG_CTRL_POWER_ON_RESET);
+
 	return 0;
 }

Signature · 1 line

2.32.0

Anand Moon

author

2021-07-16 10:38 UTC #14 lore raw

PATCH v2 4/4 phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER

In reply to Anand Moon

devm_phy_create can return -EPROBE_DEFER if the phy-supply is not ready yet. Silence this warning as the driver framework will re-attempt registering the PHY. Use dev_err_probe() for phy resources to indicate the deferral reason when waiting for the resource to come up.

Cc Martin Blumenstingl <[email protected]>

Reviewed-by Martin Blumenstingl <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
None:

---
 drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/phy/amlogic/phy-meson8b-usb2.c +2 -2

index 2aad45c55494..cf10bed40528 100644
@@ -277,8 +277,8 @@ static int phy_meson8b_usb2_probe(struct platform_device *pdev)
 
 	phy = devm_phy_create(&pdev->dev, NULL, &phy_meson8b_usb2_ops);
 	if (IS_ERR(phy)) {
-		dev_err(&pdev->dev, "failed to create PHY\n");
-		return PTR_ERR(phy);
+		return dev_err_probe(&pdev->dev, PTR_ERR(phy),
+				     "failed to create PHY\n");
 	}
 
 	phy_set_drvdata(phy, priv);

Signature · 1 line

2.32.0