linux-kernel - Re: [RFC next v1 5/5] arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY reset line

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: <CAFBinCDOhBUqNx4y+j1NeE08wWuZ-bhX5DsE8kcG4LUHKEQ+nQ@mail.gmail.com>
Date:   Mon, 10 Jun 2019 00:28:50 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     netdev <netdev@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "open list:ARM/Amlogic Meson..." <linux-amlogic@...ts.infradead.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>,
        Alexandre TORGUE <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kevin Hilman <khilman@...libre.com>,
        Neil Armstrong <narmstrong@...libre.com>
Subject: Re: [RFC next v1 5/5] arm64: dts: meson: g12a: x96-max: fix the
 Ethernet PHY reset line

Hi Linus,

On Mon, Jun 10, 2019 at 12:06 AM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> On Sun, Jun 9, 2019 at 11:36 PM Martin Blumenstingl
> <martin.blumenstingl@...glemail.com> wrote:
>
> > > If "snps,reset-active-low" was set it results in the sequence 1, 0, 1
> > > if it is not set it results in the sequence 0, 1, 0.
> >
> > I'm changing this logic with earlier patches of this series.
> > can you please look at these as well because GPIO_OPEN_SOURCE doesn't
> > work with the old version of stmmac_mdio_reset() that you are showing.
>
> OK but the logic is the same, just that the polarity handling is moved
> into gpiolib.
>
> > > The high (reset) is asserted by switching the pin into high-z open drain
> > > mode, which happens by switching the line into input mode in some
> > > cases.
> > >
> > > I think the real reason it works now is that reset is actually active high.
> >
> > let me write down what I definitely know so far
> >
> > the RTL8211F PHY wants the reset line to be LOW for a few milliseconds
> > to put it into reset mode.
> > driving the reset line HIGH again takes it out of reset.
> >
> > Odroid-N2's schematics [0] (page 30) shows that there's a pull-up for
> > the PHYRSTB pin, which is also connected to the NRST signal which is
> > GPIOZ_15
>
> Looks correct, R143 is indeed a pull up indicating that the line is
> open drain, active low.
good so far

> > > It makes a lot of sense, since if it resets the device when set as input
> > > (open drain) it holds all devices on that line in reset, which is likely
> > > what you want as most GPIOs come up as inputs (open drain).
> > > A pull-up resistor will ascertain that the devices are in reset.
> >
> > my understanding is that the pull-up resistor holds it out of reset
> > driving GPIOZ_15's (open drain) output LOW pulls the signal to ground
> > and asserts the reset
>
> Yep that seems correct.
>
> Oh I guess it is this:
>
>         amlogic,tx-delay-ns = <2>;
> -       snps,reset-gpio = <&gpio GPIOZ_14 0>;
> +       snps,reset-gpio = <&gpio GPIOZ_15 GPIO_OPEN_SOURCE>;
>         snps,reset-delays-us = <0 10000 1000000>;
> -       snps,reset-active-low;
>
> Can you try:
> snps,reset-gpio = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
> ?
I tried it and it works!

> Open source is nominally (and rarely) used for lines that are active high.
> For lines that are active low, we want to use open drain combined
> with active low.
thank you for the explanation - I'll take your version for v2 :)


Martin