Re: [PATCHv1 1/3] ARM: dts: meson8b: odroidc1: Add usb phy power node

[Martin Blumenstingl]

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.

[...]
> > > +               /*
> > > +                * 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

Did you remove the "enable-active-high;" in your "active low" test?
GPIO polarity for regulators is managed with that flag, not just with
GPIO_ACTIVE_{HIGH,LOW}

[...]
> > >  &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