netdev - Re: fec: micrel: Ethernet PHY type ID auto-detection issue

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] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 10:51:19 +0200
From:   Bruno Thomsen <bruno.thomsen@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Bruno Thomsen <bth@...strup.com>,
        Fabio Estevam <festevam@...il.com>,
        netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Lars Alex Pedersen <laa@...strup.com>
Subject: Re: fec: micrel: Ethernet PHY type ID auto-detection issue

Hi Andrew,

Den ons. 22. jul. 2020 kl. 15.26 skrev Andrew Lunn <andrew@...n.ch>:
> Is it held in reset, and the reset is released, or is the reset line
> toggled active and then inactive?

When the kernel boots PHY reset is not active (gpio level low).
The device needs a reset pulse.

> > if (d)
> >       if (d > 20000)
> >               msleep(d / 1000);
> >       else
> >               usleep_range(d, d + max_t(unsigned int, d / 10, 100));
>
> Patch welcome.

I will put together a patch with fsleep as suggested by Heiner.

> > So my current conclusion is that using generic mdio phy handling does
> > not work with Micrel PHYs unless 3 issues has been resolved.
> > - Reset PHY before auto type detection.
>
> This has been raised recently. Look back in the mail archive about a
> month. For GPIOs this is easier to solve. But regulators pose a
> problem.
>
> Part of the problem is the history of this code. It originated from a
> PHY which needed to be reset after probe and configuration to make its
> clock stable, if i remember correctly. So the PHY would already probe,
> without the reset. Something similar was needed for another PHY so the
> code got pulled out of the driver and into the PHY core. But the
> assumption remained, the PHY will probe, the reset is used after the
> probe. This code now needs to be made more generic.
>
> There is one other option, depending on your board design. The PHY
> core supports two different resets. There is a per PHY reset, which is
> what you are using. And then there is a reset for all devices on the
> bus. That is used when multiple PHYs are connected to one reset GPIO.
> You might be able to use that reset instead. But you might need to fix
> up the sleep code in that case as well.

Resetting all PHYs does not work out-of-the box for my case as there
is no delay after reset release. I have a small patch that fixes my case
by reusing the mdio reset-delay-us device tree property as both the
reset assert delay and reset deassert delay. That way we don't need
to rename or add more dt properties. Just update existing help text.
Otherwise I think the current reset-delay-us should be rename and
follow phy naming with reset-{assert,deassert}-us.

/Bruno