* [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node
2021-07-16 10:36 [PATCHv2 0/4] Meson-8b and Meson-gxbb Fix some missing code Anand Moon
@ 2021-07-16 10:36 ` Anand Moon
2021-07-17 19:46 ` Martin Blumenstingl
2021-07-16 10:36 ` [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power Anand Moon
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Anand Moon @ 2021-07-16 10:36 UTC (permalink / raw)
To: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Anand Moon, Matt Corallo, Martin Blumenstingl, Rob Herring,
Neil Armstrong, Kevin Hilman, Jerome Brunet,
Kishon Vijay Abraham I, Vinod Koul, Emiliano Ingrassia, Brian Kim,
devicetree
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 <martin.blumenstingl@googlemail.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
V1 > Fix the Input GPIO polarity from HIGH to LOW.
previous version
[0] https://patchwork.kernel.org/project/linux-amlogic/patch/20190113181808.5768-1-linux.amoon@gmail.com
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(+)
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts
index c440ef94e082..26e8f91e0950 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -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 {
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node
2021-07-16 10:36 ` [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node Anand Moon
@ 2021-07-17 19:46 ` Martin Blumenstingl
2021-07-18 3:38 ` Anand Moon
0 siblings, 1 reply; 14+ messages in thread
From: Martin Blumenstingl @ 2021-07-17 19:46 UTC (permalink / raw)
To: Anand Moon
Cc: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel,
Matt Corallo, Rob Herring, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Kishon Vijay Abraham I, Vinod Koul,
Emiliano Ingrassia, Brian Kim, devicetree
Hi Anand,
On Fri, Jul 16, 2021 at 12:37 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> 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
> As descriped in the C1+ schematics, GPIO GPIOAO_5 is used to
low prio - typo: descriped -> described
> 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.
[...]
> + /*
> + * 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
[...]
> &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:
> 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:
> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node
2021-07-17 19:46 ` Martin Blumenstingl
@ 2021-07-18 3:38 ` Anand Moon
2021-07-18 11:37 ` Martin Blumenstingl
0 siblings, 1 reply; 14+ messages in thread
From: Anand Moon @ 2021-07-18 3:38 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-phy, linux-arm-kernel, linux-amlogic, Linux Kernel,
Matt Corallo, Rob Herring, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Kishon Vijay Abraham I, Vinod Koul,
Emiliano Ingrassia, Brian Kim, devicetree
Hi Martin,
Thanks for the review comments.
On Sun, 18 Jul 2021 at 01:16, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Fri, Jul 16, 2021 at 12:37 PM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > 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
>
> > 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.
> > 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.
>
> [...]
> > + /*
> > + * 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
>
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*.
> 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
>
[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
> [...]
> > &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:
> > 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:
> > 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)
>
>
> Best regards,
> Martin
-Anand
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node
2021-07-18 3:38 ` Anand Moon
@ 2021-07-18 11:37 ` Martin Blumenstingl
2021-07-18 13:29 ` Anand Moon
0 siblings, 1 reply; 14+ messages in thread
From: Martin Blumenstingl @ 2021-07-18 11:37 UTC (permalink / raw)
To: Anand Moon
Cc: linux-phy, linux-arm-kernel, linux-amlogic, Linux Kernel,
Matt Corallo, Rob Herring, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Kishon Vijay Abraham I, Vinod Koul,
Emiliano Ingrassia, Brian Kim, devicetree
Hi Anand,
On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <linux.amoon@gmail.com> wrote:
[...]
> > > 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.
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.
> > [...]
> > > + /*
> > > + * 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
> >
> 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
> > 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
> >
>
> [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
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.
[...]
> > > &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:
> > > 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:
> > > 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
> 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.
> 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node
2021-07-18 11:37 ` Martin Blumenstingl
@ 2021-07-18 13:29 ` Anand Moon
[not found] ` <CAFBinCCGA0b=LVjbQood+iN+DHLodY0RJ-GrSOcgdzrDFJ25pQ@mail.gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Anand Moon @ 2021-07-18 13:29 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-phy, linux-arm-kernel, linux-amlogic, Linux Kernel,
Matt Corallo, Rob Herring, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Kishon Vijay Abraham I, Vinod Koul,
Emiliano Ingrassia, devicetree
Hi Martin,
Thanks for your valuable feedback,
On Sun, 18 Jul 2021 at 17:07, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <linux.amoon@gmail.com> wrote:
> [...]
> > > > 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.
> 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.
.
>
> > > [...]
> > > > + /*
> > > > + * 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
> > >
> > 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
>
> > > 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
> > >
> >
> > [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
> 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.
>
> [...]
> > > > &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:
> > > > 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:
> > > > 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
>
> > 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.
>
> > 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.
> 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/20190306212431.5779-1-martin.blumenstingl@googlemail.com/
[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.
>
> Best regards,
> Martin
Thanks
-Anand
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power
2021-07-16 10:36 [PATCHv2 0/4] Meson-8b and Meson-gxbb Fix some missing code Anand Moon
2021-07-16 10:36 ` [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node Anand Moon
@ 2021-07-16 10:36 ` Anand Moon
2021-07-17 19:59 ` Martin Blumenstingl
2021-07-16 10:36 ` [PATCHv2 3/4] phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset mode Anand Moon
2021-07-16 10:36 ` [PATCHv2 4/4] phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER Anand Moon
3 siblings, 1 reply; 14+ messages in thread
From: Anand Moon @ 2021-07-16 10:36 UTC (permalink / raw)
To: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Anand Moon, Matt Corallo, Martin Blumenstingl, Rob Herring,
Neil Armstrong, Kevin Hilman, Jerome Brunet,
Kishon Vijay Abraham I, Vinod Koul, Emiliano Ingrassia, Brian Kim,
devicetree
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 <martin.blumenstingl@googlemail.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
New patch in this series
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 201596247fd9..3f4f16a5dedd 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -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>;
};
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power
2021-07-16 10:36 ` [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power Anand Moon
@ 2021-07-17 19:59 ` Martin Blumenstingl
0 siblings, 0 replies; 14+ messages in thread
From: Martin Blumenstingl @ 2021-07-17 19:59 UTC (permalink / raw)
To: Anand Moon
Cc: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel,
Matt Corallo, Rob Herring, Neil Armstrong, Kevin Hilman,
Jerome Brunet, Kishon Vijay Abraham I, Vinod Koul,
Emiliano Ingrassia, Brian Kim, devicetree
Hi Anand,
On Fri, Jul 16, 2021 at 12:38 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> 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
>
> Fixes: 5a0803bd5ae2 ("ARM64: dts: meson-gxbb-odroidc2: Enable USB Nodes")
>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv2 3/4] phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset mode
2021-07-16 10:36 [PATCHv2 0/4] Meson-8b and Meson-gxbb Fix some missing code Anand Moon
2021-07-16 10:36 ` [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node Anand Moon
2021-07-16 10:36 ` [PATCHv2 2/4] arm64: dts: amlogic: odroidc2: Fix the chip enable signal for usb power Anand Moon
@ 2021-07-16 10:36 ` Anand Moon
2021-07-16 10:36 ` [PATCHv2 4/4] phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER Anand Moon
3 siblings, 0 replies; 14+ messages in thread
From: Anand Moon @ 2021-07-16 10:36 UTC (permalink / raw)
To: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Anand Moon, Matt Corallo, Martin Blumenstingl, Rob Herring,
Neil Armstrong, Kevin Hilman, Jerome Brunet,
Kishon Vijay Abraham I, Vinod Koul, Emiliano Ingrassia, Brian Kim,
devicetree
Power off the PHY by putting it into reset mode.
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
None: Added Martin's Ack
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 03c061dd5f0d..2aad45c55494 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -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;
}
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCHv2 4/4] phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER
2021-07-16 10:36 [PATCHv2 0/4] Meson-8b and Meson-gxbb Fix some missing code Anand Moon
` (2 preceding siblings ...)
2021-07-16 10:36 ` [PATCHv2 3/4] phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset mode Anand Moon
@ 2021-07-16 10:36 ` Anand Moon
3 siblings, 0 replies; 14+ messages in thread
From: Anand Moon @ 2021-07-16 10:36 UTC (permalink / raw)
To: linux-phy, linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Anand Moon, Matt Corallo, Martin Blumenstingl, Rob Herring,
Neil Armstrong, Kevin Hilman, Jerome Brunet,
Kishon Vijay Abraham I, Vinod Koul, Emiliano Ingrassia, Brian Kim,
devicetree
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 <martin.blumenstingl@googlemail.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
None:
---
drivers/phy/amlogic/phy-meson8b-usb2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/amlogic/phy-meson8b-usb2.c b/drivers/phy/amlogic/phy-meson8b-usb2.c
index 2aad45c55494..cf10bed40528 100644
--- a/drivers/phy/amlogic/phy-meson8b-usb2.c
+++ b/drivers/phy/amlogic/phy-meson8b-usb2.c
@@ -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);
--
2.32.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread