Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 7 Jul 2020 16:54:40 +0200
From:   Maxime Ripard <maxime@cerno.tech>
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        Antoine =?utf-8?Q?T=C3=A9nart?= <antoine.tenart@bootlin.com>
Subject: Re: PHY reset handling during DT parsing
Message-ID: <20200707145440.teimwt6kmsnyi5dz@gilmour.lan>
References: <20200706181331.x2tn5cl5jn5kqmhx@gilmour.lan>
 <20200707141918.GA928075@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20200707141918.GA928075@lunn.ch>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.
> >=20
> > 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.
> >=20
> > 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
> >=20
> > 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].
> >=20
> > get_phy_device() will then call get_phy_id() [4], that will try to
> > access the PHY through the MDIO bus [5].
> >=20
> > 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.
>=20
> Hi Maxime
>=20
> If you look at the history of this code,
>=20
> commit bafbdd527d569c8200521f2f7579f65a044271be
> Author: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Date:   Mon Dec 4 13:35:05 2017 +0100
>=20
>     phylib: Add device reset GPIO support
>=20
> 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.
>=20
> 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