Le 12/24/18 à 3:21 AM, Heiner Kallweit a écrit :
> phy_device_create() uses request_module() to load the PHY driver module
> based on the PHY ID of the device. There is some timing issue which
> sometimes prevents the PHY driver to bind to the device. In such cases
> the genphy driver is used what can cause problems if genphy isn't
> compatible with the respective PHY.
> It turned out that the first fix can fix the issue in some but not all
> cases. Moving the call to device_initialize() before the call to
> request_module() was reported to fix the issue.
> I can't explain where the root cause of the issue is and why this fix
> works. AFAICS device_initialize() just initializes the device struct
> w/o doing anything that could interfere with e.g. bus_add_driver().
> This patch removes the first preliminary fix attempt.

Humm but phy_device is comprised of a mdio_device on which the actual
matching is done, so you do have to call device_initialize() first in
order for the phy_device instance to have its companion mdio_device's
kobject to be properly initialized.

Out of curiosity, do any of the people who tested that change have the
ability to run a kernel with list/kobject debugging enabled so we can
learn a bit more about the problematic code path?
> 
> Reference:
> https://bugzilla.redhat.com/show_bug.cgi?id=1650984
> 
> Fixes: c85ddecae6e5 ("net: phy: add workaround for issue where PHY driver 
> doesn't bind to the device")
> Tested-by: Norbert Jurkeit <norbert.jurk...@web.de>
> Tested-by: Frank Crawford <fr...@crawford.emu.id.au>
> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com>
> ---
>  drivers/net/phy/phy_device.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 26c41ede5..ac0a83c7d 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -579,6 +579,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
> int addr, int phy_id,
>               dev->c45_ids = *c45_ids;
>       dev->irq = bus->irq[addr];
>       dev_set_name(&mdiodev->dev, PHY_ID_FMT, bus->id, addr);
> +     device_initialize(&mdiodev->dev);
>  
>       dev->state = PHY_DOWN;
>  
> @@ -598,8 +599,6 @@ struct phy_device *phy_device_create(struct mii_bus *bus, 
> int addr, int phy_id,
>        */
>       request_module(MDIO_MODULE_PREFIX MDIO_ID_FMT, MDIO_ID_ARGS(phy_id));
>  
> -     device_initialize(&mdiodev->dev);
> -
>       return dev;
>  }
>  EXPORT_SYMBOL(phy_device_create);
> @@ -2191,14 +2190,6 @@ int phy_driver_register(struct phy_driver *new_driver, 
> struct module *owner)
>       new_driver->mdiodrv.driver.remove = phy_remove;
>       new_driver->mdiodrv.driver.owner = owner;
>  
> -     /* The following works around an issue where the PHY driver doesn't bind
> -      * to the device, resulting in the genphy driver being used instead of
> -      * the dedicated driver. The root cause of the issue isn't known yet
> -      * and seems to be in the base driver core. Once this is fixed we may
> -      * remove this workaround.
> -      */
> -     new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
> -
>       retval = driver_register(&new_driver->mdiodrv.driver);
>       if (retval) {
>               pr_err("%s: Error %d in registering driver\n",
> 


-- 
Florian

Reply via email to