| Message ID | 20210920214209.1733768-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix mdiobus users with devres | expand |
On Mon, Sep 20, 2021 at 11:42 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > 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. > > Vladimir Oltean (2): > net: dsa: don't allocate the slave_mii_bus using devres > net: dsa: realtek: register the MDIO bus under devres > > drivers/net/dsa/realtek-smi-core.c | 2 +- > net/dsa/dsa2.c | 12 +++++++++--- > 2 files changed, 10 insertions(+), 4 deletions(-) > > -- > 2.25.1 > Hi Vladimir, Thanks for the detailed description and sorry for the trouble this caused. I will revisit this and go through the drivers using those functions again and possibly come up with some improvement. Acked-by: Bartosz Golaszewski <brgl@bgdev.pl> Bartosz
Hello: This series was applied to netdev/net.git (refs/heads/master): On Tue, 21 Sep 2021 00:42:07 +0300 you wrote: > 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: > > [...] Here is the summary with links: - [net,1/2] net: dsa: don't allocate the slave_mii_bus using devres https://git.kernel.org/netdev/net/c/5135e96a3dd2 - [net,2/2] net: dsa: realtek: register the MDIO bus under devres https://git.kernel.org/netdev/net/c/74b6d7d13307 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
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. Vladimir Oltean (2): net: dsa: don't allocate the slave_mii_bus using devres net: dsa: realtek: register the MDIO bus under devres drivers/net/dsa/realtek-smi-core.c | 2 +- net/dsa/dsa2.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-)