| Message ID | 20190123151209.2080-3-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Odroid c1+ usb fixs | expand |
Hi Anand, On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote: > > This patch enables the USB Host controller (USB0) and the relative USB0 PHY. > From the shematics GPIOAO.BIT5 gpio input for the PWREN signal of the > USB_OTG controller (usb0) which is also linked to USB_HOST controller (usb1). > Add missing phy-supply link to both USB0 and USB1 phy controller > This changes fixed the power issue on usb ports. Changes help fix usb reset warning. I prefer to change the way patches 2 and 3 are split: - this one should only add the regulators and link it with the USB controller - patch 3 should enable usb0 and usb0_phy the reason behind this is the "Fixes" tag below. it's good to have it, so please keep it in this patch! however, enabling usb0 is not a fix for a commit which enables the "USB host controller on Odroid-C1/C1+" (which is why enabling usb0 and usb0_phy should be part of patch 3) > [ 821.991470] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > [ 825.243385] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > [ 828.151310] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > [ 830.991241] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > Fixes: 2eb79a4d15ff ("ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board") > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > Cc: Jerome Brunet <jbrunet@baylibre.com> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Kevin Hilman <khilman@baylibre.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > Changes from previous patch. > > Fix the subject and commit message as per Martin's request > --Add the signal name in the comment > --Replace vbus-supply with phy-supply linking the power supply to phy node as pointed by Marine > which a PWREN signal in the USB_HOST controller (usb1) > > USB_VBUS 4 2 0 unknown 5000mV 0mA 5000mV 5000mV > phy-c1108820.phy.1 2 0mA 0mV 0mV > phy-c1108800.phy.0 2 0mA 0mV 0mV > --- > arch/arm/boot/dts/meson8b-odroidc1.dts | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts > index 58669abda259..bfa472a679d9 100644 > --- a/arch/arm/boot/dts/meson8b-odroidc1.dts > +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts > @@ -83,6 +83,22 @@ > regulator-max-microvolt = <5000000>; > }; > > + usb_vbus: regulator-usb-vbus { > + compatible = "regulator-fixed"; > + > + regulator-name = "USB_VBUS"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + > + vin-supply = <&p5v0>; > + > + /* > + * signal name from schematics: PWREN > + */ > + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + }; > + > tflash_vdd: regulator-tflash_vdd { > /* > * signal name from schematics: TFLASH_VDD_EN > @@ -295,8 +311,18 @@ > pinctrl-names = "default"; > }; > > +&usb0_phy { > + status = "okay"; > + phy-supply = <&usb_vbus>; I'm not sure whether phy-supply is correct here with the PHY framework and the dwc2 controller regulators can be set as: - phy-supply (inside the PHY node) - vbus-supply (inside the dwc2 controller node) phy-supply is always enabled when the PHY is enabled. vbus-supply is smarter: it's enabled whenever the controller is in host mode (or OTG detects hosts mode), but disabled in device/peripheral mode as far as I understand we don't want to send power to the "Micro USB" port if we're in device/peripheral mode. Regards Martin
Hi Martin, Thanks for your comments. Please find my comments below. On Mon, 4 Feb 2019 at 18:59, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > > This patch enables the USB Host controller (USB0) and the relative USB0 PHY. > > From the shematics GPIOAO.BIT5 gpio input for the PWREN signal of the > > USB_OTG controller (usb0) which is also linked to USB_HOST controller (usb1). > > Add missing phy-supply link to both USB0 and USB1 phy controller > > This changes fixed the power issue on usb ports. Changes help fix usb reset warning. > I prefer to change the way patches 2 and 3 are split: > - this one should only add the regulators and link it with the USB controller > - patch 3 should enable usb0 and usb0_phy > > the reason behind this is the "Fixes" tag below. it's good to have it, > so please keep it in this patch! > however, enabling usb0 is not a fix for a commit which enables the > "USB host controller on Odroid-C1/C1+" (which is why enabling usb0 and > usb0_phy should be part of patch 3) > Ok I will split this changes as per your suggestion. > > [ 821.991470] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > [ 825.243385] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > [ 828.151310] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > [ 830.991241] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > Fixes: 2eb79a4d15ff ("ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board") > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > Cc: Jerome Brunet <jbrunet@baylibre.com> > > Cc: Neil Armstrong <narmstrong@baylibre.com> > > Cc: Kevin Hilman <khilman@baylibre.com> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > Changes from previous patch. > > > > Fix the subject and commit message as per Martin's request > > --Add the signal name in the comment > > --Replace vbus-supply with phy-supply linking the power supply to phy node as pointed by Marine > > which a PWREN signal in the USB_HOST controller (usb1) > > > > USB_VBUS 4 2 0 unknown 5000mV 0mA 5000mV 5000mV > > phy-c1108820.phy.1 2 0mA 0mV 0mV > > phy-c1108800.phy.0 2 0mA 0mV 0mV > > --- > > arch/arm/boot/dts/meson8b-odroidc1.dts | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts > > index 58669abda259..bfa472a679d9 100644 > > --- a/arch/arm/boot/dts/meson8b-odroidc1.dts > > +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts > > @@ -83,6 +83,22 @@ > > regulator-max-microvolt = <5000000>; > > }; > > > > + usb_vbus: regulator-usb-vbus { > > + compatible = "regulator-fixed"; > > + > > + regulator-name = "USB_VBUS"; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + > > + vin-supply = <&p5v0>; > > + > > + /* > > + * signal name from schematics: PWREN > > + */ > > + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > tflash_vdd: regulator-tflash_vdd { > > /* > > * signal name from schematics: TFLASH_VDD_EN > > @@ -295,8 +311,18 @@ > > pinctrl-names = "default"; > > }; > > > > +&usb0_phy { > > + status = "okay"; > > + phy-supply = <&usb_vbus>; > I'm not sure whether phy-supply is correct here > with the PHY framework and the dwc2 controller regulators can be set as: > - phy-supply (inside the PHY node) > - vbus-supply (inside the dwc2 controller node) > > phy-supply is always enabled when the PHY is enabled. > vbus-supply is smarter: it's enabled whenever the controller is in > host mode (or OTG detects hosts mode), but disabled in > device/peripheral mode > phy-supply enables the power per port power setting which get on when device is connected. vbus-supply controls the D+ D- signal during high seed data transfer. I have check this setting dr_mode="peripheral" it did not work out for me. Initial it supper the warning but it was not the correct solution. > as far as I understand we don't want to send power to the "Micro USB" > port if we're in device/peripheral mode. > > > Regards > Martin Best Regards -Anand
Hi Anand, On Mon, Feb 4, 2019 at 9:03 PM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Martin, > > Thanks for your comments. > Please find my comments below. > > On Mon, 4 Feb 2019 at 18:59, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > > > Hi Anand, > > > > On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > This patch enables the USB Host controller (USB0) and the relative USB0 PHY. > > > From the shematics GPIOAO.BIT5 gpio input for the PWREN signal of the > > > USB_OTG controller (usb0) which is also linked to USB_HOST controller (usb1). > > > Add missing phy-supply link to both USB0 and USB1 phy controller > > > This changes fixed the power issue on usb ports. Changes help fix usb reset warning. > > I prefer to change the way patches 2 and 3 are split: > > - this one should only add the regulators and link it with the USB controller > > - patch 3 should enable usb0 and usb0_phy > > > > the reason behind this is the "Fixes" tag below. it's good to have it, > > so please keep it in this patch! > > however, enabling usb0 is not a fix for a commit which enables the > > "USB host controller on Odroid-C1/C1+" (which is why enabling usb0 and > > usb0_phy should be part of patch 3) > > > > Ok I will split this changes as per your suggestion. thank you! > > > [ 821.991470] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > [ 825.243385] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > [ 828.151310] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > [ 830.991241] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > > > Fixes: 2eb79a4d15ff ("ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board") > > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > > Cc: Jerome Brunet <jbrunet@baylibre.com> > > > Cc: Neil Armstrong <narmstrong@baylibre.com> > > > Cc: Kevin Hilman <khilman@baylibre.com> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > Changes from previous patch. > > > > > > Fix the subject and commit message as per Martin's request > > > --Add the signal name in the comment > > > --Replace vbus-supply with phy-supply linking the power supply to phy node as pointed by Marine > > > which a PWREN signal in the USB_HOST controller (usb1) > > > > > > USB_VBUS 4 2 0 unknown 5000mV 0mA 5000mV 5000mV > > > phy-c1108820.phy.1 2 0mA 0mV 0mV > > > phy-c1108800.phy.0 2 0mA 0mV 0mV > > > --- > > > arch/arm/boot/dts/meson8b-odroidc1.dts | 26 ++++++++++++++++++++++++++ > > > 1 file changed, 26 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts > > > index 58669abda259..bfa472a679d9 100644 > > > --- a/arch/arm/boot/dts/meson8b-odroidc1.dts > > > +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts > > > @@ -83,6 +83,22 @@ > > > regulator-max-microvolt = <5000000>; > > > }; > > > > > > + usb_vbus: regulator-usb-vbus { > > > + compatible = "regulator-fixed"; > > > + > > > + regulator-name = "USB_VBUS"; > > > + regulator-min-microvolt = <5000000>; > > > + regulator-max-microvolt = <5000000>; > > > + > > > + vin-supply = <&p5v0>; > > > + > > > + /* > > > + * signal name from schematics: PWREN > > > + */ > > > + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; > > > + enable-active-high; > > > + }; > > > + > > > tflash_vdd: regulator-tflash_vdd { > > > /* > > > * signal name from schematics: TFLASH_VDD_EN > > > @@ -295,8 +311,18 @@ > > > pinctrl-names = "default"; > > > }; > > > > > > +&usb0_phy { > > > + status = "okay"; > > > + phy-supply = <&usb_vbus>; > > I'm not sure whether phy-supply is correct here > > with the PHY framework and the dwc2 controller regulators can be set as: > > - phy-supply (inside the PHY node) > > - vbus-supply (inside the dwc2 controller node) > > > > phy-supply is always enabled when the PHY is enabled. > > vbus-supply is smarter: it's enabled whenever the controller is in > > host mode (or OTG detects hosts mode), but disabled in > > device/peripheral mode > > > phy-supply enables the power per port power setting which get on when > device is connected. the actual dt-bindings description is "a regulator that provides power to the PHY", see [0] (note that it states that it powers the PHY, not the bus or any connected device) > vbus-supply controls the D+ D- signal during high seed data transfer. I believe this is not the case. the vbus-supply seems undocumented at the moment however, the dwc2 driver has separate supplies for vusb_a (analog) and vusb_d (digital), see [1] (these are used here: [2]) VBUS (in my own words) is the +5V power signal which is provided by the USB host to power the USB device. my interpretationfor the Amlogic SoCs is: - vusb_a and vusb_d are generated internally, not sure if there's an external fixed regulator - there is no phy-supply (the PHY itself is powered by clock-gating using the CLKID_USB clock) - usb0 VBUS is controlled by GPIOAO_5 Regards Martin [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/drivers/usb/dwc2/core.h#L72 [2] https://elixir.bootlin.com/linux/v5.0-rc5/source/drivers/usb/dwc2/platform.c#L293
Hi Martin, On Tue, 5 Feb 2019 at 02:05, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Mon, Feb 4, 2019 at 9:03 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Martin, > > > > Thanks for your comments. > > Please find my comments below. > > > > On Mon, 4 Feb 2019 at 18:59, Martin Blumenstingl > > <martin.blumenstingl@googlemail.com> wrote: > > > > > > Hi Anand, > > > > > > On Wed, Jan 23, 2019 at 4:12 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > > > > > > This patch enables the USB Host controller (USB0) and the relative USB0 PHY. > > > > From the shematics GPIOAO.BIT5 gpio input for the PWREN signal of the > > > > USB_OTG controller (usb0) which is also linked to USB_HOST controller (usb1). > > > > Add missing phy-supply link to both USB0 and USB1 phy controller > > > > This changes fixed the power issue on usb ports. Changes help fix usb reset warning. > > > I prefer to change the way patches 2 and 3 are split: > > > - this one should only add the regulators and link it with the USB controller > > > - patch 3 should enable usb0 and usb0_phy > > > > > > the reason behind this is the "Fixes" tag below. it's good to have it, > > > so please keep it in this patch! > > > however, enabling usb0 is not a fix for a commit which enables the > > > "USB host controller on Odroid-C1/C1+" (which is why enabling usb0 and > > > usb0_phy should be part of patch 3) > > > > > > > Ok I will split this changes as per your suggestion. > thank you! > > > > > [ 821.991470] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > [ 825.243385] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > [ 828.151310] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > [ 830.991241] usb 1-1.2: reset high-speed USB device number 3 using dwc2 > > > > > > > > Fixes: 2eb79a4d15ff ("ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board") > > > > Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > > > Cc: Jerome Brunet <jbrunet@baylibre.com> > > > > Cc: Neil Armstrong <narmstrong@baylibre.com> > > > > Cc: Kevin Hilman <khilman@baylibre.com> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > Changes from previous patch. > > > > > > > > Fix the subject and commit message as per Martin's request > > > > --Add the signal name in the comment > > > > --Replace vbus-supply with phy-supply linking the power supply to phy node as pointed by Marine > > > > which a PWREN signal in the USB_HOST controller (usb1) > > > > > > > > USB_VBUS 4 2 0 unknown 5000mV 0mA 5000mV 5000mV > > > > phy-c1108820.phy.1 2 0mA 0mV 0mV > > > > phy-c1108800.phy.0 2 0mA 0mV 0mV > > > > --- > > > > arch/arm/boot/dts/meson8b-odroidc1.dts | 26 ++++++++++++++++++++++++++ > > > > 1 file changed, 26 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts > > > > index 58669abda259..bfa472a679d9 100644 > > > > --- a/arch/arm/boot/dts/meson8b-odroidc1.dts > > > > +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts > > > > @@ -83,6 +83,22 @@ > > > > regulator-max-microvolt = <5000000>; > > > > }; > > > > > > > > + usb_vbus: regulator-usb-vbus { > > > > + compatible = "regulator-fixed"; > > > > + > > > > + regulator-name = "USB_VBUS"; > > > > + regulator-min-microvolt = <5000000>; > > > > + regulator-max-microvolt = <5000000>; > > > > + > > > > + vin-supply = <&p5v0>; > > > > + > > > > + /* > > > > + * signal name from schematics: PWREN > > > > + */ > > > > + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; > > > > + enable-active-high; > > > > + }; > > > > + > > > > tflash_vdd: regulator-tflash_vdd { > > > > /* > > > > * signal name from schematics: TFLASH_VDD_EN > > > > @@ -295,8 +311,18 @@ > > > > pinctrl-names = "default"; > > > > }; > > > > > > > > +&usb0_phy { > > > > + status = "okay"; > > > > + phy-supply = <&usb_vbus>; > > > I'm not sure whether phy-supply is correct here > > > with the PHY framework and the dwc2 controller regulators can be set as: > > > - phy-supply (inside the PHY node) > > > - vbus-supply (inside the dwc2 controller node) > > > > > > phy-supply is always enabled when the PHY is enabled. > > > vbus-supply is smarter: it's enabled whenever the controller is in > > > host mode (or OTG detects hosts mode), but disabled in > > > device/peripheral mode > > > > > phy-supply enables the power per port power setting which get on when > > device is connected. > the actual dt-bindings description is "a regulator that provides power > to the PHY", see [0] > (note that it states that it powers the PHY, not the bus or any > connected device) > > > vbus-supply controls the D+ D- signal during high seed data transfer. > I believe this is not the case. the vbus-supply seems undocumented at the moment > > however, the dwc2 driver has separate supplies for vusb_a (analog) and > vusb_d (digital), see [1] (these are used here: [2]) > > VBUS (in my own words) is the +5V power signal which is provided by > the USB host to power the USB device. > > my interpretationfor the Amlogic SoCs is: > - vusb_a and vusb_d are generated internally, not sure if there's an > external fixed regulator > - there is no phy-supply (the PHY itself is powered by clock-gating > using the CLKID_USB clock) > - usb0 VBUS is controlled by GPIOAO_5 > But as per the device tree binding [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt its uses phy-supply to enable power to usb phy which seem to be correct option. > > Regards > Martin > > > [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt > [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/drivers/usb/dwc2/core.h#L72 > [2] https://elixir.bootlin.com/linux/v5.0-rc5/source/drivers/usb/dwc2/platform.c#L293 Best Regards -Anand
Hi Anand, On Tue, Feb 5, 2019 at 8:53 PM Anand Moon <linux.amoon@gmail.com> wrote: [...] > > my interpretationfor the Amlogic SoCs is: > > - vusb_a and vusb_d are generated internally, not sure if there's an > > external fixed regulator > > - there is no phy-supply (the PHY itself is powered by clock-gating > > using the CLKID_USB clock) > > - usb0 VBUS is controlled by GPIOAO_5 > > > > But as per the device tree binding > > [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt > [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt > > its uses phy-supply to enable power to usb phy which seem to be correct option. phy-bindings.txt states that "phy-supply" is a "Phandle to a regulator that provides power to the PHY". I have written the example in meson8b-usb2-phy.txt and I think it's wrong (back when I wrote it I didn't know about the "vbus-supply" property on the controller and I didn't know about the VBUS constraints). I believe that the phy-supply property is intended for PHYs which need an external power supply (for example because they are not embedded into a SoC or if the SoC has separate voltage inputs for the PHY). I'm not aware of any PHY supply voltage on the Amlogic SoCs (I believe this is done internally within the SoC). instead they use clock gating to power down the PHY. as far as I understand the VBUS signal it depends on the USB mode: - host provides VBUS - peripheral devices are powered using this voltage - with OTG VBUS needs to be turned on or off depending on the current mode (host or peripheral) with the "phy-supply" property there's no way to manage the regulator depending on the USB mode (host or peripheral), it will always be "on". Regards Martin
Hi Martin, On Wed, 6 Feb 2019 at 16:58, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Tue, Feb 5, 2019 at 8:53 PM Anand Moon <linux.amoon@gmail.com> wrote: > [...] > > > my interpretationfor the Amlogic SoCs is: > > > - vusb_a and vusb_d are generated internally, not sure if there's an > > > external fixed regulator > > > - there is no phy-supply (the PHY itself is powered by clock-gating > > > using the CLKID_USB clock) > > > - usb0 VBUS is controlled by GPIOAO_5 > > > > > > > But as per the device tree binding > > > > [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt > > [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt > > > > its uses phy-supply to enable power to usb phy which seem to be correct option. > phy-bindings.txt states that "phy-supply" is a "Phandle to a regulator > that provides power to the PHY". > I have written the example in meson8b-usb2-phy.txt and I think it's > wrong (back when I wrote it I didn't know about the "vbus-supply" > property on the controller and I didn't know about the VBUS > constraints). > > I believe that the phy-supply property is intended for PHYs which need > an external power supply (for example because they are not embedded > into a SoC or if the SoC has separate voltage inputs for the PHY). > I'm not aware of any PHY supply voltage on the Amlogic SoCs (I believe > this is done internally within the SoC). instead they use clock gating > to power down the PHY. > > as far as I understand the VBUS signal it depends on the USB mode: > - host provides VBUS > - peripheral devices are powered using this voltage > - with OTG VBUS needs to be turned on or off depending on the current > mode (host or peripheral) > with the "phy-supply" property there's no way to manage the regulator > depending on the USB mode (host or peripheral), it will always be > "on". > > > Regards > Martin Thanks for the clarification. But this dose not work setting the usb1_phy to use vbus-supply. &usb1_phy { status = "okay"; + vbus-supply = <&usb_vbus>; }; I am attaching a small patch for testing. [0] usbvbus.patch Here is the summary of the above patch. 1. hot-plugins of usb device is not working. 2. only cold/warm boot let the device come up. 3. not power is supplied to the usb ports. 4. no power module is registered with the regulator summary. # cat /sys/kernel/debug/regulator/regulator_summary regulator use open bypass opmode voltage current min max --------------------------------------------------------------------------------------- regulator-dummy 4 3 0 unknown 0mV 0mA 0mV 0mV c90c0000.usb 1 0mA 0mV 0mV c90c0000.usb 1 0mA 0mV 0mV VCCK 1 1 0 unknown 860mV 0mA 860mV 1140mV cpu0 0 0mA 860mV 860mV P5V0 2 4 0 unknown 5000mV 0mA 5000mV 5000mV VCC1V8 1 1 0 unknown 1800mV 0mA 1800mV 1800mV c1108680.adc 1 0mA 0mV 0mV VCC3V3 1 2 0 unknown 3300mV 0mA 3300mV 3300mV VDD_RTC 0 0 0 unknown 900mV 0mA 900mV 900mV TFLASH_VDD 1 1 0 unknown 3300mV 0mA 3300mV 3300mV c1108c20.mmc:slot@1 1 0mA 3300mV 3400mV DDR_VDDC 0 0 0 unknown 1500mV 0mA 1500mV 1500mV USB_VBUS 0 0 0 unknown 5000mV 0mA 5000mV 5000mV TF_IO 0 1 0 unknown 3300mV 0mA 1800mV 3300mV c1108c20.mmc:slot@1 0 0mA 0mV 0mV *So from my side phy-supply is the correct option.* Best Regards -Anand
Hi Anand, On Thu, Feb 7, 2019 at 7:33 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Martin, > > On Wed, 6 Feb 2019 at 16:58, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > > > Hi Anand, > > > > On Tue, Feb 5, 2019 at 8:53 PM Anand Moon <linux.amoon@gmail.com> wrote: > > [...] > > > > my interpretationfor the Amlogic SoCs is: > > > > - vusb_a and vusb_d are generated internally, not sure if there's an > > > > external fixed regulator > > > > - there is no phy-supply (the PHY itself is powered by clock-gating > > > > using the CLKID_USB clock) > > > > - usb0 VBUS is controlled by GPIOAO_5 > > > > > > > > > > But as per the device tree binding > > > > > > [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt > > > [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt > > > > > > its uses phy-supply to enable power to usb phy which seem to be correct option. > > phy-bindings.txt states that "phy-supply" is a "Phandle to a regulator > > that provides power to the PHY". > > I have written the example in meson8b-usb2-phy.txt and I think it's > > wrong (back when I wrote it I didn't know about the "vbus-supply" > > property on the controller and I didn't know about the VBUS > > constraints). > > > > I believe that the phy-supply property is intended for PHYs which need > > an external power supply (for example because they are not embedded > > into a SoC or if the SoC has separate voltage inputs for the PHY). > > I'm not aware of any PHY supply voltage on the Amlogic SoCs (I believe > > this is done internally within the SoC). instead they use clock gating > > to power down the PHY. > > > > as far as I understand the VBUS signal it depends on the USB mode: > > - host provides VBUS > > - peripheral devices are powered using this voltage > > - with OTG VBUS needs to be turned on or off depending on the current > > mode (host or peripheral) > > with the "phy-supply" property there's no way to manage the regulator > > depending on the USB mode (host or peripheral), it will always be > > "on". > > > > > > Regards > > Martin > > Thanks for the clarification. > But this dose not work setting the usb1_phy to use vbus-supply. > > &usb1_phy { > status = "okay"; > + vbus-supply = <&usb_vbus>; > }; > > I am attaching a small patch for testing. > [0] usbvbus.patch indeed, this is not working for me either. I checked my old notes at [0] -> it works for me when setting "vbus-supply" at the usb controller (not the PHY). with the following snippet: &usb1 { status = "okay"; vbus-supply = <&usb_vbus>; }; I get: # cat /sys/kernel/debug/regulator/regulator_summary | grep -i usb c90c0000.usb 1 0mA 0mV 0mV c90c0000.usb 1 0mA 0mV 0mV USB_VBUS 1 1 0 unknown 5000mV 0mA 5000mV 5000mV c90c0000.usb 1 0mA 0mV 0mV can you please try this on your board as well? > Here is the summary of the above patch. > > 1. hot-plugins of usb device is not working. > 2. only cold/warm boot let the device come up. > 3. not power is supplied to the usb ports. > 4. no power module is registered with the regulator summary. with "vbus-supply" moved to the usb1 node I get the following result: 1. same: hot-plugins of usb device is not working 2. different: a reboot doesn't make devices come up 3. same: no power is supplied to the USB ports (in my case this causes #2, but it's not clear why there's no power...) 4. different: the regulator is registered with the USB controller in debugfs Regards Martin [0] https://lkml.org/lkml/2019/1/18/960
Hi Martin, On Sat, 9 Feb 2019 at 06:06, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Thu, Feb 7, 2019 at 7:33 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Martin, > > > > On Wed, 6 Feb 2019 at 16:58, Martin Blumenstingl > > <martin.blumenstingl@googlemail.com> wrote: > > > > > > Hi Anand, > > > > > > On Tue, Feb 5, 2019 at 8:53 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > [...] > > > > > my interpretationfor the Amlogic SoCs is: > > > > > - vusb_a and vusb_d are generated internally, not sure if there's an > > > > > external fixed regulator > > > > > - there is no phy-supply (the PHY itself is powered by clock-gating > > > > > using the CLKID_USB clock) > > > > > - usb0 VBUS is controlled by GPIOAO_5 > > > > > > > > > > > > > But as per the device tree binding > > > > > > > > [0] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/meson8b-usb2-phy.txt > > > > [1] https://elixir.bootlin.com/linux/v5.0-rc5/source/Documentation/devicetree/bindings/phy/phy-bindings.txt > > > > > > > > its uses phy-supply to enable power to usb phy which seem to be correct option. > > > phy-bindings.txt states that "phy-supply" is a "Phandle to a regulator > > > that provides power to the PHY". > > > I have written the example in meson8b-usb2-phy.txt and I think it's > > > wrong (back when I wrote it I didn't know about the "vbus-supply" > > > property on the controller and I didn't know about the VBUS > > > constraints). > > > > > > I believe that the phy-supply property is intended for PHYs which need > > > an external power supply (for example because they are not embedded > > > into a SoC or if the SoC has separate voltage inputs for the PHY). > > > I'm not aware of any PHY supply voltage on the Amlogic SoCs (I believe > > > this is done internally within the SoC). instead they use clock gating > > > to power down the PHY. > > > > > > as far as I understand the VBUS signal it depends on the USB mode: > > > - host provides VBUS > > > - peripheral devices are powered using this voltage > > > - with OTG VBUS needs to be turned on or off depending on the current > > > mode (host or peripheral) > > > with the "phy-supply" property there's no way to manage the regulator > > > depending on the USB mode (host or peripheral), it will always be > > > "on". > > > > > > > > > Regards > > > Martin > > > > Thanks for the clarification. > > But this dose not work setting the usb1_phy to use vbus-supply. > > > > &usb1_phy { > > status = "okay"; > > + vbus-supply = <&usb_vbus>; > > }; > > > > I am attaching a small patch for testing. > > [0] usbvbus.patch > indeed, this is not working for me either. > I checked my old notes at [0] -> it works for me when setting > "vbus-supply" at the usb controller (not the PHY). > > with the following snippet: > &usb1 { > status = "okay"; > vbus-supply = <&usb_vbus>; > }; > I get: > # cat /sys/kernel/debug/regulator/regulator_summary | grep -i usb > c90c0000.usb 1 0mA > 0mV 0mV > c90c0000.usb 1 0mA > 0mV 0mV > USB_VBUS 1 1 0 unknown 5000mV 0mA > 5000mV 5000mV > c90c0000.usb 1 0mA > 0mV 0mV > > can you please try this on your board as well? > > > Here is the summary of the above patch. > > > > 1. hot-plugins of usb device is not working. > > 2. only cold/warm boot let the device come up. > > 3. not power is supplied to the usb ports. > > 4. no power module is registered with the regulator summary. > with "vbus-supply" moved to the usb1 node I get the following result: > 1. same: hot-plugins of usb device is not working > 2. different: a reboot doesn't make devices come up > 3. same: no power is supplied to the USB ports (in my case this causes > #2, but it's not clear why there's no power...) > 4. different: the regulator is registered with the USB controller in debugfs > Here the the logs after I applied the above changes [1] https://pastebin.com/rVa8gxNG with below message. [ 14.793044] usb 1-1.3: reset high-speed USB device number 3 using dwc2 [ 18.253011] usb 1-1.3: reset high-speed USB device number 3 using dwc2 [ 21.643017] usb 1-1.3: reset high-speed USB device number 3 using dwc2 [ 25.033012] usb 1-1.3: reset high-speed USB device number 3 using dwc2 [ 28.412995] usb 1-1.3: reset high-speed USB device number 3 using dwc2 So this is not correct solution to the problem. > > Regards > Martin > > > [0] https://lkml.org/lkml/2019/1/18/960 Best Regards -Anand
Hi Anand, On Sat, Feb 9, 2019 at 6:55 PM Anand Moon <linux.amoon@gmail.com> wrote: [...] > > > But this dose not work setting the usb1_phy to use vbus-supply. > > > > > > &usb1_phy { > > > status = "okay"; > > > + vbus-supply = <&usb_vbus>; > > > }; > > > > > > I am attaching a small patch for testing. > > > [0] usbvbus.patch > > indeed, this is not working for me either. > > I checked my old notes at [0] -> it works for me when setting > > "vbus-supply" at the usb controller (not the PHY). > > > > with the following snippet: > > &usb1 { > > status = "okay"; > > vbus-supply = <&usb_vbus>; > > }; > > I get: > > # cat /sys/kernel/debug/regulator/regulator_summary | grep -i usb > > c90c0000.usb 1 0mA > > 0mV 0mV > > c90c0000.usb 1 0mA > > 0mV 0mV > > USB_VBUS 1 1 0 unknown 5000mV 0mA > > 5000mV 5000mV > > c90c0000.usb 1 0mA > > 0mV 0mV > > > > can you please try this on your board as well? > > > > > Here is the summary of the above patch. > > > > > > 1. hot-plugins of usb device is not working. > > > 2. only cold/warm boot let the device come up. > > > 3. not power is supplied to the usb ports. > > > 4. no power module is registered with the regulator summary. > > with "vbus-supply" moved to the usb1 node I get the following result: > > 1. same: hot-plugins of usb device is not working > > 2. different: a reboot doesn't make devices come up > > 3. same: no power is supplied to the USB ports (in my case this causes > > #2, but it's not clear why there's no power...) > > 4. different: the regulator is registered with the USB controller in debugfs > > > Here the the logs after I applied the above changes > > [1] https://pastebin.com/rVa8gxNG can you please provide the .dts patch for this log? I can see the following message in your log: [ 1.977367] USB_VBUS: disabling I have tried it with the attached patch, then I get: # dmesg | grep VBUS # # cat /sys/kernel/debug/regulator/regulator_summary regulator use open bypass opmode voltage current min max --------------------------------------------------------------------------------------- regulator-dummy 4 3 0 unknown 0mV 0mA 0mV 0mV c90c0000.usb 1 0mA 0mV 0mV c90c0000.usb 1 0mA 0mV 0mV VCCK 1 1 0 unknown 860mV 0mA 860mV 1140mV cpu0 0 0mA 860mV 860mV P5V0 3 4 0 unknown 5000mV 0mA 5000mV 5000mV VCC1V8 2 2 0 unknown 1800mV 0mA 1800mV 1800mV c1108680.adc 1 0mA 0mV 0mV c1108e00.mmc 1 0mA 1800mV 1950mV VCC3V3 2 3 0 unknown 3300mV 0mA 3300mV 3300mV c1108e00.mmc 1 0mA 3300mV 3400mV VDD_RTC 0 0 0 unknown 900mV 0mA 900mV 900mV TFLASH_VDD 1 1 0 unknown 3300mV 0mA 3300mV 3300mV c1108c20.mmc:slot@1 1 0mA 3300mV 3400mV DDR_VDDC 0 0 0 unknown 1500mV 0mA 1500mV 1500mV USB_VBUS 1 1 0 unknown 5000mV 0mA 5000mV 5000mV c90c0000.usb 1 0mA 0mV 0mV TF_IO 0 1 0 unknown 3300mV 0mA 1800mV 3300mV c1108c20.mmc:slot@1 0 0mA 0mV 0mV (my kernel build includes two more changes on top of Kevin's v5.1/dt branch which add two new regulators: it enables the RTC and the SDHC MMC controller, but these are not related to USB at all) USB ist still not working for me, but I believe my problem is different to yours. Regards Martin
Hi Martin, On Mon, 11 Feb 2019 at 03:51, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > Hi Anand, > > On Sat, Feb 9, 2019 at 6:55 PM Anand Moon <linux.amoon@gmail.com> wrote: > [...] > > > > But this dose not work setting the usb1_phy to use vbus-supply. > > > > > > > > &usb1_phy { > > > > status = "okay"; > > > > + vbus-supply = <&usb_vbus>; > > > > }; > > > > > > > > I am attaching a small patch for testing. > > > > [0] usbvbus.patch > > > indeed, this is not working for me either. > > > I checked my old notes at [0] -> it works for me when setting > > > "vbus-supply" at the usb controller (not the PHY). > > > > > > with the following snippet: > > > &usb1 { > > > status = "okay"; > > > vbus-supply = <&usb_vbus>; > > > }; > > > I get: > > > # cat /sys/kernel/debug/regulator/regulator_summary | grep -i usb > > > c90c0000.usb 1 0mA > > > 0mV 0mV > > > c90c0000.usb 1 0mA > > > 0mV 0mV > > > USB_VBUS 1 1 0 unknown 5000mV 0mA > > > 5000mV 5000mV > > > c90c0000.usb 1 0mA > > > 0mV 0mV > > > > > > can you please try this on your board as well? > > > > > > > Here is the summary of the above patch. > > > > > > > > 1. hot-plugins of usb device is not working. > > > > 2. only cold/warm boot let the device come up. > > > > 3. not power is supplied to the usb ports. > > > > 4. no power module is registered with the regulator summary. > > > with "vbus-supply" moved to the usb1 node I get the following result: > > > 1. same: hot-plugins of usb device is not working > > > 2. different: a reboot doesn't make devices come up > > > 3. same: no power is supplied to the USB ports (in my case this causes > > > #2, but it's not clear why there's no power...) > > > 4. different: the regulator is registered with the USB controller in debugfs > > > > > Here the the logs after I applied the above changes > > > > [1] https://pastebin.com/rVa8gxNG > can you please provide the .dts patch for this log? > I can see the following message in your log: > [ 1.977367] USB_VBUS: disabling > Sorry for the confusion on the logs. Above logs are generated with below setting. &usb1 { status = "okay"; vbus-supply = <&usb_vbus>; }; > I have tried it with the attached patch, then I get: > # dmesg | grep VBUS > # > # cat /sys/kernel/debug/regulator/regulator_summary > regulator use open bypass opmode voltage current > min max > --------------------------------------------------------------------------------------- > regulator-dummy 4 3 0 unknown 0mV 0mA > 0mV 0mV > c90c0000.usb 1 0mA > 0mV 0mV > c90c0000.usb 1 0mA > 0mV 0mV > VCCK 1 1 0 unknown 860mV 0mA > 860mV 1140mV > cpu0 0 0mA > 860mV 860mV > P5V0 3 4 0 unknown 5000mV 0mA > 5000mV 5000mV > VCC1V8 2 2 0 unknown 1800mV 0mA > 1800mV 1800mV > c1108680.adc 1 0mA > 0mV 0mV > c1108e00.mmc 1 0mA > 1800mV 1950mV > VCC3V3 2 3 0 unknown 3300mV 0mA > 3300mV 3300mV > c1108e00.mmc 1 0mA > 3300mV 3400mV > VDD_RTC 0 0 0 unknown 900mV 0mA > 900mV 900mV > TFLASH_VDD 1 1 0 unknown 3300mV 0mA > 3300mV 3300mV > c1108c20.mmc:slot@1 1 0mA > 3300mV 3400mV > DDR_VDDC 0 0 0 unknown 1500mV 0mA > 1500mV 1500mV > USB_VBUS 1 1 0 unknown 5000mV 0mA > 5000mV 5000mV > c90c0000.usb 1 0mA > 0mV 0mV > TF_IO 0 1 0 unknown 3300mV 0mA > 1800mV 3300mV > c1108c20.mmc:slot@1 0 0mA > 0mV 0mV > > (my kernel build includes two more changes on top of Kevin's v5.1/dt > branch which add two new regulators: it enables the RTC and the SDHC > MMC controller, but these are not related to USB at all) > > USB ist still not working for me, but I believe my problem is > different to yours. > > > Regards > > Martin Can we have other people who can test both the approach for *vbus-supply* vs *phy-supply* setting. Then we can conclude on the correct approach. Honestly I am using power supply from dc jack rather than power via usb otg. Best Regards -Anand
Hi Anand, On Mon, Feb 11, 2019 at 4:09 PM Anand Moon <linux.amoon@gmail.com> wrote: [...] > Can we have other people who can test both the approach for > *vbus-supply* vs *phy-supply* setting. > Then we can conclude on the correct approach. this is a good idea. do we focus on the 4x USB host ports for this test or also on the OTG port? my test-setup so far was: - Odroid-C1+ board revision 0.4 20150615 - the board is powered using the micro USB port - connect an USB thumb drive to one of the USB host ports test-cases: - an "unpatched" (in terms of USB power regulators) kernel - using Anand's phy-supply = <&usb_vbus>; inside the usb1_phy node - using vbus-supply = <&usb_vbus>; inside the usb1 node in all thee scenarios the LED of my thumb drive did not light up and there was nothing in the kernel log. also lsusb -vv didn't show anything. Anand, do you have any other test-case in mind that we need to check? I CC'ed Emiliano because he initially added USB support to meson8b-odroidc1.dts (that was before I got my own Odroid-C1+). He can probably help us test on his board(s) as well. > Honestly I am using power supply from dc jack rather than power via usb otg. I don't have a plug which matches the little DC jack so I'm powering my board using the micro USB connector. Regards Martin
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts b/arch/arm/boot/dts/meson8b-odroidc1.dts index 58669abda259..bfa472a679d9 100644 --- a/arch/arm/boot/dts/meson8b-odroidc1.dts +++ b/arch/arm/boot/dts/meson8b-odroidc1.dts @@ -83,6 +83,22 @@ regulator-max-microvolt = <5000000>; }; + usb_vbus: regulator-usb-vbus { + compatible = "regulator-fixed"; + + regulator-name = "USB_VBUS"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + + vin-supply = <&p5v0>; + + /* + * signal name from schematics: PWREN + */ + gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + tflash_vdd: regulator-tflash_vdd { /* * signal name from schematics: TFLASH_VDD_EN @@ -295,8 +311,18 @@ pinctrl-names = "default"; }; +&usb0_phy { + status = "okay"; + phy-supply = <&usb_vbus>; +}; + &usb1_phy { status = "okay"; + phy-supply = <&usb_vbus>; +}; + +&usb0 { + status = "okay"; }; &usb1 {
This patch enables the USB Host controller (USB0) and the relative USB0 PHY. From the shematics GPIOAO.BIT5 gpio input for the PWREN signal of the USB_OTG controller (usb0) which is also linked to USB_HOST controller (usb1). Add missing phy-supply link to both USB0 and USB1 phy controller This changes fixed the power issue on usb ports. Changes help fix usb reset warning. [ 821.991470] usb 1-1.2: reset high-speed USB device number 3 using dwc2 [ 825.243385] usb 1-1.2: reset high-speed USB device number 3 using dwc2 [ 828.151310] usb 1-1.2: reset high-speed USB device number 3 using dwc2 [ 830.991241] usb 1-1.2: reset high-speed USB device number 3 using dwc2 Fixes: 2eb79a4d15ff ("ARM: dts: meson: enabling the USB Host controller on Odroid-C1/C1+ board") Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Cc: Jerome Brunet <jbrunet@baylibre.com> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Kevin Hilman <khilman@baylibre.com> Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- Changes from previous patch. Fix the subject and commit message as per Martin's request --Add the signal name in the comment --Replace vbus-supply with phy-supply linking the power supply to phy node as pointed by Marine which a PWREN signal in the USB_HOST controller (usb1) USB_VBUS 4 2 0 unknown 5000mV 0mA 5000mV 5000mV phy-c1108820.phy.1 2 0mA 0mV 0mV phy-c1108800.phy.0 2 0mA 0mV 0mV --- arch/arm/boot/dts/meson8b-odroidc1.dts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)