LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
To: Anand Moon <linux.amoon@gmail.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: Tue, 13 Jul 2021 22:35:17 +0200	[thread overview]
Message-ID: <CAFBinCC-kD-MW+bwqCZH5AjYDhxWa_pN2WEnHuiZpx=RhUdROQ@mail.gmail.com> (raw)
In-Reply-To: <CANAwSgTh02qg=cMT8tpHcEfAs5yW2vHQ7tPOkvuKXos_fOmh8A@mail.gmail.com>

Hi Anand,

On Tue, Jul 13, 2021 at 8:45 PM Anand Moon <linux.amoon@gmail.com> wrote:
>
> 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
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

  reply	other threads:[~2021-07-13 20:35 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13  5:52 [PATCHv1 0/3] Meson-8b and Meson-gxbb Fix some missing code Anand Moon
2021-07-13  5:52 ` [PATCHv1 1/3] ARM: dts: meson8b: odroidc1: Add usb phy power node Anand Moon
2021-07-13 15:05   ` Martin Blumenstingl
2021-07-13 18:44     ` Anand Moon
2021-07-13 20:35       ` Martin Blumenstingl [this message]
2021-07-14 11:59         ` Anand Moon
2021-07-14 17:25           ` Anand Moon
2021-07-14 23:30             ` Martin Blumenstingl
2021-07-15 10:24               ` Anand Moon
2021-07-13  5:52 ` [PATCHv1 2/3] phy: amlogic: meson8b-usb2: Power off the PHY by putting it into reset mode Anand Moon
2021-07-13 15:10   ` Martin Blumenstingl
2021-07-13  5:52 ` [PATCHv1 3/3] phy: amlogic: meson8b-usb2: don't log an error on -EPROBE_DEFER Anand Moon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFBinCC-kD-MW+bwqCZH5AjYDhxWa_pN2WEnHuiZpx=RhUdROQ@mail.gmail.com' \
    --to=martin.blumenstingl@googlemail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ingrassia@epigenesys.com \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=kishon@ti.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux.amoon@gmail.com \
    --cc=narmstrong@baylibre.com \
    --cc=oc2udbzfd@mattcorallo.com \
    --cc=robh+dt@kernel.org \
    --cc=vkoul@kernel.org \
    --subject='Re: [PATCHv1 1/3] ARM: dts: meson8b: odroidc1: Add usb phy power node' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).