LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
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 22:55:15 +0530	[thread overview]
Message-ID: <CANAwSgR5nNN3D+NZjqNNjyp=rVYE=RG2icnJAc=5F23xSh9Bvg@mail.gmail.com> (raw)
In-Reply-To: <CANAwSgTf4sjoufrZuK-EjE=+yA8zSoVbqhEyNhJJFBLUCOVZmQ@mail.gmail.com>

Hi Martin.

On Wed, 14 Jul 2021 at 17:29, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Martin,
>
> On Wed, 14 Jul 2021 at 02:05, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > 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.
> >
> 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;
>         };
>

Can you give these small changes a try,
$ git diff
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index 748f4c6a050a..066523f14074 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -47,8 +47,9 @@ 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_OPEN_DRAIN>;
                enable-active-high;
+               regulator-always-on;
        };

[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-usb-pwr-en) out lo

Thanks
-Anand

  reply	other threads:[~2021-07-14 17:25 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
2021-07-14 11:59         ` Anand Moon
2021-07-14 17:25           ` Anand Moon [this message]
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='CANAwSgR5nNN3D+NZjqNNjyp=rVYE=RG2icnJAc=5F23xSh9Bvg@mail.gmail.com' \
    --to=linux.amoon@gmail.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=martin.blumenstingl@googlemail.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).