Re: PHY reset handling during DT parsing

[Maxime Ripard]

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@xxxxxxxxxxxxxxxxxx>
> 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