Re: [PATCHv1 1/3] ARM: dts: meson8b: odroidc1: Add usb phy power node
From: Anand Moon
Date: Wed Jul 14 2021 - 08:00:00 EST
Hi Martin,
On Wed, 14 Jul 2021 at 02:05, Martin Blumenstingl
<martin.blumenstingl@xxxxxxxxxxxxxx> wrote:
>
> Hi Anand,
>
> On Tue, Jul 13, 2021 at 8:45 PM Anand Moon <linux.amoon@xxxxxxxxx> wrote:
> >
> > Hi Martin,
> >
> > Thanks for reviewing the changes,
> >
> > On Tue, 13 Jul 2021 at 20:35, Martin Blumenstingl
> > <martin.blumenstingl@xxxxxxxxxxxxxx> wrote:
> > >
> > > Hi Anand,
> > >
> > > On Tue, Jul 13, 2021 at 7:53 AM Anand Moon <linux.amoon@xxxxxxxxx> wrote:
> > > >
> > > > Add missing usb phy power node for phy mode fix below warning.
> > > >
> > > > [ 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
> > > I did some testing on my own Odroid-C1+ and this patch is not doing
> > > anything for me.
> > > more information below.
> > Some device node for USB will have
> 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.
>
OK, thanks.
> [...]
> > > > + /*
> > > > + * signal name from schematics: USB_POWER
> > > > + */
> > > Just a few lines below you're saying that the name from the schematics is PWREN
> > > If this patch is getting another round then please clarify the actual
> > > signal name, or name both signals if the schematics is actually using
> > > both names.
> > >
> > As per the schematics.
> > PWREN ---> GPIOAO.BIT5 gpio pin control
> > USB_POWER ---> P5V0 power source regulator.
> ah, thanks for clarifying this
> my suggestion is to put that exact paragraph into the comment to avoid confusion
>
> [...]
> > > Can you please give this a try on your Odroid-C1 as well?
> > > The conclusion from my own testing is that GPIOAO_5 doesn't seem to be
> > > related to USB1 (host-only) because if it was then inverting the
> > > polarity (from active high to active low) should result in a change.
> > >
> >
> > Ok I have modified as per above but not changes in gpio polarity
> > from active high to active low. see below.
> >
> > # Odroid C1
> > [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-usbp_pwr_e) out hi
> >
> > # Odroid C2
> > [alarm@archl-c2lm ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
> > gpio-501 (USB HUB nRESET |usb-hub-reset ) out hi
> > gpio-502 (USB OTG Power En |regulator-usb-pwrs ) out hi
> that's strange, my result is different
>
> gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
> enable-active-high;
> gives me:
> # grep USB_OTG_PWREN /sys/kernel/debug/gpio
> gpio-418 (USB_OTG_PWREN |regulator-usb-pwr-en) out hi
>
> gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
> gives me:
> # grep USB_OTG_PWREN /sys/kernel/debug/gpio
> gpio-418 (USB_OTG_PWREN |regulator-usb-pwr-en) out lo ACTIVE LOW
This gpio pin number dose not match the gpio pin on Odroid c1+, see below.
>
> Did you remove the "enable-active-high;" in your "active low" test?
No
> GPIO polarity for regulators is managed with that flag, not just with
> GPIO_ACTIVE_{HIGH,LOW}
It's just with changes the following, below
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -47,7 +47,7 @@ usb_pwr_en: regulator-usb-pwr-en {
/*
* signal name from schematics: PWREN
*/
- gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
+ gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
enable-active-high;
};
[alarm@archl-c1e ~]$ sudo grep USB_OTG_PWREN /sys/kernel/debug/gpio
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out hi
[alarm@archl-c1e ~]$ sudo cat /sys/kernel/debug/gpio
gpiochip1: GPIOs 1949-1964, parent: platform/c8100084.pinctrl, aobus-banks:
gpio-1949 (UART TX )
gpio-1950 (UART RX )
gpio-1951 ( )
gpio-1952 (TF_3V3N_1V8_EN |TF_IO ) out lo
gpio-1953 (USB_HUB_RST_N |usb-hub-reset ) out hi
gpio-1954 (USB_OTG_PWREN |regulator-usb-pwr-en) out hi
gpio-1955 (J7 Header Pin 2 )
gpio-1956 (IR_IN )
gpio-1957 (J7 Header Pin 4 )
gpio-1958 (J7 Header Pin 6 )
gpio-1959 (J7 Header Pin 5 )
gpio-1960 (J7 Header Pin 7 )
gpio-1961 (HDMI_CEC )
gpio-1962 (SYS_LED |c1:blue:alive ) out hi ACTIVE LOW
gpio-1963 ( )
gpio-1964 ( )
gpiochip0: GPIOs 1965-2047, parent: platform/c1109880.pinctrl, cbus-banks:
gpio-1965 (J2 Header Pin 35 )
gpio-1966 (J2 Header Pin 36 )
gpio-1967 (J2 Header Pin 32 )
gpio-1968 (J2 Header Pin 31 )
gpio-1969 (J2 Header Pin 29 )
gpio-1970 (J2 Header Pin 18 )
gpio-1971 (J2 Header Pin 22 )
gpio-1972 (J2 Header Pin 16 )
gpio-1973 (J2 Header Pin 23 )
gpio-1974 (J2 Header Pin 21 )
gpio-1975 (J2 Header Pin 19 )
gpio-1976 (J2 Header Pin 33 )
gpio-1977 (J2 Header Pin 8 )
gpio-1978 (J2 Header Pin 10 )
gpio-1979 (J2 Header Pin 15 )
gpio-1980 (J2 Header Pin 13 )
gpio-1981 (J2 Header Pin 24 )
gpio-1982 (J2 Header Pin 26 )
gpio-1983 (Revision (upper) )
gpio-1984 (Revision (lower) )
gpio-1985 (J2 Header Pin 7 )
gpio-1986 ( )
gpio-1987 (J2 Header Pin 12 )
gpio-1988 (J2 Header Pin 11 )
gpio-1989 ( )
gpio-1990 ( )
gpio-1991 ( )
gpio-1992 (TFLASH_VDD_EN |regulator-tflash_vdd) out lo
gpio-1993 ( )
gpio-1994 ( )
gpio-1995 (VCCK_PWM (PWM_C) )
gpio-1996 (I2CA_SDA )
gpio-1997 (I2CA_SCL )
gpio-1998 (I2CB_SDA )
gpio-1999 (I2CB_SCL )
gpio-2000 (VDDEE_PWM (PWM_D) )
gpio-2001 ( )
gpio-2002 (HDMI_HPD )
gpio-2003 (HDMI_I2C_SDA )
gpio-2004 (HDMI_I2C_SCL )
gpio-2005 (ETH_PHY_INTR )
gpio-2006 (ETH_PHY_NRST |PHY reset ) out hi ACTIVE LOW
gpio-2007 (ETH_TXD1 )
gpio-2008 (ETH_TXD0 )
gpio-2009 (ETH_TXD3 )
gpio-2010 (ETH_TXD2 )
gpio-2011 (ETH_RGMII_TX_CLK )
gpio-2012 (SD_DATA1 (SDB_D1) )
gpio-2013 (SD_DATA0 (SDB_D0) )
gpio-2014 (SD_CLK )
gpio-2015 (SD_CMD )
gpio-2016 (SD_DATA3 (SDB_D3) )
gpio-2017 (SD_DATA2 (SDB_D2) )
gpio-2018 (SD_CDN (SD_DET_N) |cd ) in hi ACTIVE LOW
gpio-2019 (SDC_D0 (EMMC) )
gpio-2020 (SDC_D1 (EMMC) )
gpio-2021 (SDC_D2 (EMMC) )
gpio-2022 (SDC_D3 (EMMC) )
gpio-2023 (SDC_D4 (EMMC) )
gpio-2024 (SDC_D5 (EMMC) )
gpio-2025 (SDC_D6 (EMMC) )
gpio-2026 (SDC_D7 (EMMC) )
gpio-2027 (SDC_CLK (EMMC) )
gpio-2028 (SDC_RSTn (EMMC) |reset ) out hi ACTIVE LOW
gpio-2029 (SDC_CMD (EMMC) )
gpio-2030 (BOOT_SEL )
gpio-2031 ( )
gpio-2032 ( )
gpio-2033 ( )
gpio-2034 ( )
gpio-2035 ( )
gpio-2036 ( )
gpio-2037 ( )
gpio-2038 (ETH_RXD1 )
gpio-2039 (ETH_RXD0 )
gpio-2040 (ETH_RX_DV )
gpio-2041 (RGMII_RX_CLK )
gpio-2042 (ETH_RXD3 )
gpio-2043 (ETH_RXD2 )
gpio-2044 (ETH_TXEN )
gpio-2045 (ETH_PHY_REF_CLK_25MO)
gpio-2046 (ETH_MDC )
gpio-2047 (ETH_MDIO )
>
> [...]
> > > > &usb1_phy {
> > > > status = "okay";
> > > > + phy-supply = <&usb_pwr_en>;
> > > From the schematics it seems that this is not the PHY supply (which I
> > > admittedly have used incorrectly for VBUS before).
> > > In the schematics that I have (odroid-c1+_rev0.4_20150615.pdf) it
> > > seems to be enabling VBUS.
> > > So in that case a vbus-supply property should be used inside &usb1 instead.
> > >
> > As per the debug log I have added this since core phy looking for this property
> Let's discuss the results with different polarities first, then we can
> also discuss the rest.
>
>
> Best regards,
> Martin
Thanks
Anand