diff options
author | David S. Miller <[email protected]> | 2021-09-21 13:52:16 +0100 |
---|---|---|
committer | David S. Miller <[email protected]> | 2021-09-21 13:52:16 +0100 |
commit | b3f98404bd629a243c0a15a3ade32b1cf9fbe0da (patch) | |
tree | 47d513bd1a6faead4589b3f42467ff81e0148fb5 | |
parent | 3e95cfa24e24fbd7fb7675ab972a5aa507c7a89c (diff) | |
parent | 74b6d7d13307b016f4b5bba8198297824c0ee6df (diff) |
Merge branch 'dsa-devres'
Vladimir Oltean says:
====================
Fix mdiobus users with devres
Commit ac3a68d56651 ("net: phy: don't abuse devres in
devm_mdiobus_register()") by Bartosz Golaszewski has introduced two
classes of potential bugs by making the devres callback of
devm_mdiobus_alloc stop calling mdiobus_unregister.
The exact buggy circumstances are presented in the individual commit
messages. I have searched the tree for other occurrences, but at the
moment:
- for issue (a) I have no concrete proof that other buses except SPI and
I2C suffer from it, and the only SPI or I2C device drivers that call
of_mdiobus_alloc are the DSA drivers that leave a NULL
ds->slave_mii_bus and a non-NULL ds->ops->phy_read, aka ksz9477,
ksz8795, lan9303_i2c, vsc73xx-spi.
- for issue (b), all drivers which call of_mdiobus_alloc either use
of_mdiobus_register too, or call mdiobus_unregister sometime within
the ->remove path.
Although at this point I've seen enough strangeness caused by this
"device_del during ->shutdown" that I'm just going to copy the SPI and
I2C subsystem maintainers to this patch series, to get their feedback
whether they've had reports about things like this before. I don't think
other buses behave in this way, it forces SPI and I2C devices to have to
protect themselves from a really strange set of issues.
====================
Signed-off-by: David S. Miller <[email protected]>
-rw-r--r-- | drivers/net/dsa/realtek-smi-core.c | 2 | ||||
-rw-r--r-- | net/dsa/dsa2.c | 12 |
2 files changed, 10 insertions, 4 deletions
diff --git a/drivers/net/dsa/realtek-smi-core.c b/drivers/net/dsa/realtek-smi-core.c index dd2f0d6208b3..2fcfd917b876 100644 --- a/drivers/net/dsa/realtek-smi-core.c +++ b/drivers/net/dsa/realtek-smi-core.c @@ -368,7 +368,7 @@ int realtek_smi_setup_mdio(struct realtek_smi *smi) smi->slave_mii_bus->parent = smi->dev; smi->ds->slave_mii_bus = smi->slave_mii_bus; - ret = of_mdiobus_register(smi->slave_mii_bus, mdio_np); + ret = devm_of_mdiobus_register(smi->dev, smi->slave_mii_bus, mdio_np); if (ret) { dev_err(smi->dev, "unable to register MDIO bus %s\n", smi->slave_mii_bus->id); diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index f54639a3a822..b29262eee00b 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c @@ -880,7 +880,7 @@ static int dsa_switch_setup(struct dsa_switch *ds) devlink_params_publish(ds->devlink); if (!ds->slave_mii_bus && ds->ops->phy_read) { - ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev); + ds->slave_mii_bus = mdiobus_alloc(); if (!ds->slave_mii_bus) { err = -ENOMEM; goto teardown; @@ -890,13 +890,16 @@ static int dsa_switch_setup(struct dsa_switch *ds) err = mdiobus_register(ds->slave_mii_bus); if (err < 0) - goto teardown; + goto free_slave_mii_bus; } ds->setup = true; return 0; +free_slave_mii_bus: + if (ds->slave_mii_bus && ds->ops->phy_read) + mdiobus_free(ds->slave_mii_bus); teardown: if (ds->ops->teardown) ds->ops->teardown(ds); @@ -921,8 +924,11 @@ static void dsa_switch_teardown(struct dsa_switch *ds) if (!ds->setup) return; - if (ds->slave_mii_bus && ds->ops->phy_read) + if (ds->slave_mii_bus && ds->ops->phy_read) { mdiobus_unregister(ds->slave_mii_bus); + mdiobus_free(ds->slave_mii_bus); + ds->slave_mii_bus = NULL; + } dsa_switch_unregister_notifier(ds); |