| Message ID | 1637583298-20321-2-git-send-email-zhuyinbo@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1,1/2] modpost: file2alias: fixup mdio alias garbled code in modules.alias | expand |
On Mon, Nov 22, 2021 at 08:14:58PM +0800, Yinbo Zhu wrote: > the phy_id is only phy identifier, that phy module auto-load function > should according the phy_id event rather than other information, this > patch is remove other unnecessary information and add phy_id event in > mdio_uevent function and ethernet phy module auto-load function will > work well. > > Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> > --- > drivers/net/phy/mdio_bus.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 6865d93..999f0d4 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -962,12 +962,12 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv) > > static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env) > { > - int rc; > + struct phy_device *pdev; > > - /* Some devices have extra OF data and an OF-style MODALIAS */ > - rc = of_device_uevent_modalias(dev, env); > - if (rc != -ENODEV) > - return rc; > + pdev = to_phy_device(dev); > + > + if (add_uevent_var(env, "MODALIAS=mdio:p%08X", pdev->phy_id)) > + return -ENOMEM; The MDIO bus contains more than just PHYs. This completely breaks anything that isn't a PHY device - likely by performing an out-of-bounds access. This change also _totally_ breaks any MDIO devices that rely on matching via the "of:" mechanism using the compatible specified in DT. An example of that is the B53 DSA switch. Sorry, but we've already learnt this lesson from a similar case with SPI. Once one particular way of dealing with MODALIAS has been established for auto-loading modules for a subsystem, it is very difficult to change it without causing regressions. We need a very clear description of the problem that these patches are attempting to address, and then we need to see that effort has been put in to verify that changing the auto-loading mechanism is safe to do - such as auditing every single driver that use the MDIO subsystem. > > return 0; > } > @@ -991,7 +991,7 @@ static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env) > }; > > struct bus_type mdio_bus_type = { > - .name = "mdio_bus", > + .name = "mdio", This looks like an unrelated user-interface breaking change. This changes the path of all MDIO devices and drivers in /sys/bus/mdio_bus/* Thanks.
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 6865d93..999f0d4 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -962,12 +962,12 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv) static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env) { - int rc; + struct phy_device *pdev; - /* Some devices have extra OF data and an OF-style MODALIAS */ - rc = of_device_uevent_modalias(dev, env); - if (rc != -ENODEV) - return rc; + pdev = to_phy_device(dev); + + if (add_uevent_var(env, "MODALIAS=mdio:p%08X", pdev->phy_id)) + return -ENOMEM; return 0; } @@ -991,7 +991,7 @@ static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env) }; struct bus_type mdio_bus_type = { - .name = "mdio_bus", + .name = "mdio", .dev_groups = mdio_bus_dev_groups, .match = mdio_bus_match, .uevent = mdio_uevent,
the phy_id is only phy identifier, that phy module auto-load function should according the phy_id event rather than other information, this patch is remove other unnecessary information and add phy_id event in mdio_uevent function and ethernet phy module auto-load function will work well. Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn> --- drivers/net/phy/mdio_bus.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)