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

From: Anand Moon <linux.amoon@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-phy@lists.infradead.org,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-amlogic@lists.infradead.org,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Matt Corallo <oc2udbzfd@mattcorallo.com>,
	Rob Herring <robh+dt@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	Emiliano Ingrassia <ingrassia@epigenesys.com>,
	devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCHv1 1/3] ARM: dts: meson8b: odroidc1: Add usb phy power node
Date: Wed, 14 Jul 2021 00:14:53 +0530	[thread overview]
Message-ID: <CANAwSgTh02qg=cMT8tpHcEfAs5yW2vHQ7tPOkvuKXos_fOmh8A@mail.gmail.com> (raw)
In-Reply-To: <CAFBinCD3ahpNQhP_jWZ3-4nXx8OqZOAANCpTN3BrsULRbYZHVw@mail.gmail.com>

Hi Martin,

Thanks for reviewing the changes,

On Tue, 13 Jul 2021 at 20:35, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Tue, Jul 13, 2021 at 7:53 AM Anand Moon <linux.amoon@gmail.com> 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

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

> [...]
> > +               gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_HIGH>;
> > +               enable-active-high;
> I booted my Odroid-C1+ with this and USB was working fine.
> Then I replaced these two lines with:
>   gpio = <&gpio_ao GPIOAO_5 GPIO_ACTIVE_LOW>;
> and I found that USB is still working.
>
Yep it should be GPIOAO_5 GPIO_ACTIVE_LOW, my mistake

> 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

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

[    1.250044] phy phy-c1108820.phy.0: Looking up phy-supply from device tree
[    1.250060] phy phy-c1108820.phy.0: Looking up phy-supply property
in node /soc/cbus@c1100000/phy@8820 failed
[    7.222566] libphy: stmmac: probed

vbus-supply power is needed for dwc2 node see below.

[    1.257714] dwc2 c90c0000.usb: Looking up vbus-supply from device tree
[    1.257725] dwc2 c90c0000.usb: Looking up vbus-supply property in
node /soc/usb@c90c0000 failed

>
> Best regards,
> Martin

Thanks
-Anand