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