Re: [PATCH] [RFC] net: phy: Fix reboot crash if CONFIG_IP_PNP is not set

On 1/4/21 10:43 AM, Ioana Ciornei wrote:

On Mon, Jan 04, 2021 at 06:31:05PM +0100, Andrew Lunn wrote:

quoted

quoted

The basic rules here should be, if the MDIO bus is registered, it is
usable. There are things like PHY statistics, HWMON temperature
sensors, etc, DSA switches, all which have a life cycle separate to
the interface being up.

[Goes and looks at the code]

Yes, this is runtime PM which is broken.

sh_mdio_init() needs to wrap the mdp->mii_bus->read and
mdp->mii_bus->write calls with calls to

pm_runtime_get_sync(&mdp->pdev->dev);

and

pm_runtime_put_sync(&mdp->pdev->dev);

Agree. Thanks for actually looking into it.. I'm not really well versed
in runtime PM.

quoted

The KSZ8041RNLI supports statistics, which ethtool --phy-stats can
read, and these will also going to cause problems.

Not really, this driver connects to the PHY on .ndo_open(), thus any
try to actually dump the PHY statistics before an ifconfig up would get
an -EOPNOTSUPP since the dev->phydev is not yet populated.

This is exactly why I do not understand why some drivers insist on
calling of_phy_connect() and its variants on .ndo_open() and not while
probing the device - you can access the debug stats only if the
interface was started.

Doing the connect in ndo_open() allows you to keep the PHY in whatever
the state it was prior to the kernel managing it, which if everything is
correctly designed means in a low power state.

Your Ethernet driver's probe function may be called on boot and you may
never use the network device at all, so it is a waste of energy to power
on the PHY, have it potentially link with its link partner while you
still have no chance of doing any configuration to it because you have
not brought up the network interface.
-- 
Florian