linux-kernel - Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFBinCCGA0b=LVjbQood+iN+DHLodY0RJ-GrSOcgdzrDFJ25pQ@mail.gmail.com>
Date:   Sun, 18 Jul 2021 16:01:34 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Anand Moon <linux.amoon@...il.com>
Cc:     linux-phy@...ts.infradead.org,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-amlogic@...ts.infradead.org,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        Matt Corallo <oc2udbzfd@...tcorallo.com>,
        Rob Herring <robh+dt@...nel.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Emiliano Ingrassia <ingrassia@...genesys.com>,
        devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCHv2 1/4] ARM: dts: meson8b: odroidc1: Add usb phy power node

Hi Anand,

On Sun, Jul 18, 2021 at 3:29 PM Anand Moon <linux.amoon@...il.com> wrote:
>
> Hi Martin,
>
> Thanks for your valuable feedback,
>
> On Sun, 18 Jul 2021 at 17:07, Martin Blumenstingl
> <martin.blumenstingl@...glemail.com> wrote:
> >
> > Hi Anand,
> >
> > On Sun, Jul 18, 2021 at 5:38 AM Anand Moon <linux.amoon@...il.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@...00000/phy@...0 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.
Can you please point out the page numbers for me?

What I am seeing on page 1 is:
GPIOAO_5 (called GPIOAO.BIT5) is connected as an INPUT to the USB_OTG
PWREN signal (that's the green box with the label "USB_OTG.SchDoc"
above it).

Below (still on page 1) there's a whole section for
USB_HOST_PWR_SW.SchDoc and USB_HUB_GL852G-OHG.SchDoc.
This box also uses a PWREN signal, but from my understanding it's a
different one:
- PWREN in this area is an OUTPUT on the USB hub
(USB_HUB_GL852G-OHG.SchDoc). This is confirmed by the PWREN signals on
page 14 of the GL852 datasheet: [4]
- Then PWREN is an INPUT for the overcurrent switches (USB_HOST_PWR_SW.SchDoc)
This section does not mention GPIOAO_5 (or GPIOAO.BIT5) at all.

For better illustration I have attached a screenshot with some
annotations from the schematics.
This is how I understand the whole USB setup.
This doesn't mean that I am right though.

[...]
> > 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@...hl-c1e ~]$ sudo cat /sys/kernel/debug/gpio | grep usb
>  gpio-1953 (USB_HUB_RST_N       |usb-hub-reset       ) out hi
Did you also delete the "leds" node from meson8b-odroidc1.dts?

[...]
> > 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.
Yes, I think we're now on the same page on this topic
I totally forgot about some of these patches - great that you found
them and added them here for further documentation of this topic!


Best regards,
Martin


[4] https://datasheet.lcsc.com/szlcsc/Genesys-Logic-GL852G-HHG12_C136618.pdf

Download attachment "odroidc1-usb-pwren.png" of type "image/png" (123552 bytes)