[RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

From: Vladimir Oltean
Date: Sun Sep 12 2021 - 15:28:23 EST


This reverts commit 3ac8eed62596387214869319379c1fcba264d8c6.

I am not actually sure I follow the patch author's logic, because the
change does more than it says on the box, but this patch breaks
suspend/resume on NXP LS1028A and probably on any other systems which
have PHY devices with no driver bound, because the patch has removed the
"!phydev->drv" check without actually explaining why that is fine.

Examples why that is not fine:

- There is an MDIO bus with a PHY device that doesn't have a specific
PHY driver loaded, because mdiobus_register() automatically creates a
PHY device for it but there is no specific PHY driver in the system.
Normally under those circumstances, the generic PHY driver will be
bound lazily to it (at phy_attach_direct time), but since no one has
attempted to connect to that PHY device (yet), it will not have a
driver.

- There is an internal MDIO bus with PCS devices on it, for serial links
such as SGMII. If the driver does not set bus->phy_mask = ~0 to
prevent auto-scanning, PHY devices will get created for those PCSes
too, and although those PHY devices are not used, they do not bother
anybody either. PCS devices are usually managed in Linux as raw MDIO
devices. Nonetheless, they do not have a PHY driver, nor does anybody
attempt to connect to them (because they are not a PHY), and therefore
this patch breaks that.

The stack trace is:

Unable to handle kernel NULL pointer dereference at virtual address 00000000000000e8
pc : mdio_bus_phy_suspend+0xd8/0xec
lr : dpm_run_callback+0x38/0x90
Call trace:
mdio_bus_phy_suspend+0xd8/0xec
dpm_run_callback+0x38/0x90
__device_suspend+0x108/0x3cc
dpm_suspend+0x140/0x210
dpm_suspend_start+0x7c/0xa0
suspend_devices_and_enter+0x13c/0x540
pm_suspend+0x2a4/0x330

Fixes: 3ac8eed62596 ("net: phy: Uniform PHY driver access")
Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
---
drivers/net/phy/phy_device.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9e2891d8e8dd..ba5ad86ec826 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -233,9 +233,11 @@ static DEFINE_MUTEX(phy_fixup_lock);

static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
{
+ struct device_driver *drv = phydev->mdio.dev.driver;
+ struct phy_driver *phydrv = to_phy_driver(drv);
struct net_device *netdev = phydev->attached_dev;

- if (!phydev->drv->suspend)
+ if (!drv || !phydrv->suspend)
return false;

/* PHY not attached? May suspend if the PHY has not already been
--
2.25.1