On 1/15/19 12:33 PM, 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() and inform the user in case of failure.
Would you see any value in checking but not propagating the
request_module() return value back to the caller? Surely, something will
fail later on which may, or may not be properly handled.
>
> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
> ---
> drivers/net/phy/phy_device.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index a2423cbb2..1527ed0f2 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -548,6 +548,17 @@ static const struct device_type mdio_bus_phy_type = {
> .pm = MDIO_BUS_PHY_PM_OPS,
> };
>
> +static void 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));
> + if (IS_ENABLED(CONFIG_MODULES) && ret < 0)
> + phydev_err(dev, "error %d loading PHY driver module for ID
> 0x%08x\n",
> + ret, phy_id);
> +}
> +
> 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)
> @@ -610,12 +621,10 @@ 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]));
> + phy_request_driver_module(dev, c45_ids->device_ids[i]);
> }
> } else {
> - request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT,
> - MDIO_ID_ARGS(phy_id));
> + phy_request_driver_module(dev, phy_id);
> }
>
> device_initialize(&mdiodev->dev);
>
--
Florian
