[PATCH v2 net-next] net: phy: check return code when requesting PHY driver module

[PATCH v2 net-next] net: phy: check return code when requesting PHY driver module

[Heiner Kallweit]

When requesting the PHY driver module fails we'll bind the genphy
driver later. This isn't obvious to the user and may cause, depending
on the PHY, different types of issues. Therefore check the return code
of request_module(). Note that we only check for failures in loading
the module, not whether a module exists for the respective PHY ID.

v2:
- add comment explaining what is checked and what is not
- return error from phy_device_create() if loading module fails

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a2423cbb2..32772f422 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -548,12 +548,31 @@ static const struct device_type mdio_bus_phy_type = {
 	.pm = MDIO_BUS_PHY_PM_OPS,
 };
 
+static int phy_request_driver_module(struct phy_device *dev, int phy_id)
+{
+	int ret;
+
+	ret = request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT,
+			     MDIO_ID_ARGS(phy_id));
+	/* we only check for failures in executing the usermode binary,
+	 * not whether a PHY driver module exists for the PHY ID
+	 */
+	if (IS_ENABLED(CONFIG_MODULES) && ret < 0) {
+		phydev_err(dev, "error %d loading PHY driver module for ID 0x%08x\n",
+			   ret, phy_id);
+		return ret;
+	}
+
+	return 0;
+}
+
 struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 				     bool is_c45,
 				     struct phy_c45_device_ids *c45_ids)
 {
 	struct phy_device *dev;
 	struct mdio_device *mdiodev;
+	int ret = 0;
 
 	/* We allocate the device, and initialize the default values */
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
@@ -610,15 +629,21 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 			if (!(c45_ids->devices_in_package & (1 << i)))
 				continue;
 
-			request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT,
-				       MDIO_ID_ARGS(c45_ids->device_ids[i]));
+			ret = phy_request_driver_module(dev,
+						c45_ids->device_ids[i]);
+			if (ret)
+				break;
 		}
 	} else {
-		request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT,
-			       MDIO_ID_ARGS(phy_id));
+		ret = phy_request_driver_module(dev, phy_id);
 	}
 
-	device_initialize(&mdiodev->dev);
+	if (!ret) {
+		device_initialize(&mdiodev->dev);
+	} else {
+		kfree(dev);
+		dev = ERR_PTR(ret);
+	}
 
 	return dev;
 }
-- 
2.20.1

Re: [PATCH v2 net-next] net: phy: check return code when requesting PHY driver module

[Andrew Lunn]

On Wed, Jan 16, 2019 at 08:07:38AM +0100, Heiner Kallweit wrote:
> When requesting the PHY driver module fails we'll bind the genphy
> driver later. This isn't obvious to the user and may cause, depending
> on the PHY, different types of issues. Therefore check the return code
> of request_module(). Note that we only check for failures in loading
> the module, not whether a module exists for the respective PHY ID.
> 
> v2:
> - add comment explaining what is checked and what is not
> - return error from phy_device_create() if loading module fails
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v2 net-next] net: phy: check return code when requesting PHY driver module

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 16 Jan 2019 08:07:38 +0100

> When requesting the PHY driver module fails we'll bind the genphy
> driver later. This isn't obvious to the user and may cause, depending
> on the PHY, different types of issues. Therefore check the return code
> of request_module(). Note that we only check for failures in loading
> the module, not whether a module exists for the respective PHY ID.
> 
> v2:
> - add comment explaining what is checked and what is not
> - return error from phy_device_create() if loading module fails
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied, thanks Heiner.