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

Date:   Mon, 6 Jul 2020 20:13:31 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
Cc:     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: PHY reset handling during DT parsing

Hi,

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.

I'm not entirely sure how to fix that though. I tried to fix it by
splitting away the gpio / reset code away from mdiobus_device_register
into a new function, and calling it before the first call to get_phy_id
so that we can put our phy out of reset, but it looks like the device
registration makes it more complicated than that. Any ideas?

Thanks!
Maxime

1: https://elixir.bootlin.com/linux/latest/source/drivers/of/of_mdio.c#L274
2: https://elixir.bootlin.com/linux/latest/source/drivers/of/of_mdio.c#L82
3: https://elixir.bootlin.com/linux/latest/source/drivers/of/of_mdio.c#L119
4: https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy_device.c#L830
5: https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy_device.c#L791

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)