linux-kernel - Re: PHY reset handling during DT parsing

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]

Date:   Tue, 7 Jul 2020 16:54:40 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Antoine Ténart <antoine.tenart@...tlin.com>
Subject: Re: PHY reset handling during DT parsing

Hi Andrew,

On Tue, Jul 07, 2020 at 04:19:18PM +0200, Andrew Lunn wrote:
> On Mon, Jul 06, 2020 at 08:13:31PM +0200, Maxime Ripard wrote:
> > I came across an issue today on an Allwinner board, but I believe it's a
> > core issue.
> > 
> > That board is using the stmac driver together with a phy that happens to
> > have a reset GPIO, except that that GPIO will never be claimed, and the
> > PHY will thus never work.
> > 
> > You can find an example of such a board here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun6i-a31-hummingbird.dts#n195
> > 
> > It looks like when of_mdiobus_register() will parse the DT, it will then
> > call of_mdiobus_register_phy() for each PHY it encounters [1].
> > of_mdiobus_register_phy() will then if the phy doesn't have an
> > ethernet-phy-id* compatible call get_phy_device() [2], and will later on
> > call phy_register_device [3].
> > 
> > get_phy_device() will then call get_phy_id() [4], that will try to
> > access the PHY through the MDIO bus [5].
> > 
> > The code that deals with the PHY reset line / GPIO is however only done
> > in mdiobus_device_register, called through phy_device_register. Since
> > this is happening way after the call to get_phy_device, our PHY might
> > still very well be in reset if the bootloader hasn't put it out of reset
> > and left it there.
> 
> Hi Maxime
> 
> If you look at the history of this code,
> 
> commit bafbdd527d569c8200521f2f7579f65a044271be
> Author: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
> Date:   Mon Dec 4 13:35:05 2017 +0100
> 
>     phylib: Add device reset GPIO support
> 
> you will see there is an assumption the PHY can be detected while in
> reset. The reset was originally handled inside the at803x PHY driver
> probe function, before it got moved into the core.
> 
> What you are asking for it reasonable, but you have some history to
> deal with, changing some assumptions as to what the reset is all
> about.

Thanks for the pointer.

It looks to me from the commit log that the assumption was that a
bootloader could leave the PHY into reset though?

It starts with:

> The PHY devices sometimes do have their reset signal (maybe even power
> supply?) tied to some GPIO and sometimes it also does happen that a
> boot loader does not leave it deasserted.

This is exactly the case I was discussing. The bootloader hasn't used
the PHY, and thus the PHY reset signal is still asserted?

Maxime