Merge branch 'more-dsa-fixes-for-devres-mdiobus_-alloc-register'
authorJakub Kicinski <kuba@kernel.org>
Wed, 9 Feb 2022 04:30:38 +0000 (20:30 -0800)
committerJakub Kicinski <kuba@kernel.org>
Wed, 9 Feb 2022 04:30:39 +0000 (20:30 -0800)
commit1335648f0b6f9e2f643ae0c1501e995752b5c79e
treeedb2ef6be23852af1ccd20ca0ec12e30fa931e70
parent23de0d7b6f0e3f9a6283a882594c479949da1120
parent0d120dfb5d67edc5bcd1804e167dba2b30809afd
Merge branch 'more-dsa-fixes-for-devres-mdiobus_-alloc-register'

Vladimir Oltean says:

====================
More DSA fixes for devres + mdiobus_{alloc,register}

The initial patch series "[net,0/2] Fix mdiobus users with devres"
https://patchwork.kernel.org/project/netdevbpf/cover/20210920214209.1733768-1-vladimir.oltean@nxp.com/
fixed some instances where DSA drivers on slow buses (SPI, I2C) trigger
a panic (changed since then to a warn) in mdiobus_free. That was due to
devres calling mdiobus_free() with no prior mdiobus_unregister(), which
again was due to commit ac3a68d56651 ("net: phy: don't abuse devres in
devm_mdiobus_register()") by Bartosz Golaszewski.

Rafael Richter and Daniel Klauer report yet another variation on that
theme, but this time it applies to any DSA switch driver, not just those
on buses which have a "->shutdown() calls ->remove() which unregisters
children" sequence.

Their setup is that of an LX2160A DPAA2 SoC driving a Marvell DSA switch
(MDIO). DPAA2 Ethernet drivers probe on the "fsl-mc" bus
(drivers/bus/fsl-mc/fsl-mc-bus.c). This bus is meant to be the
kernel-side representation of the networking objects kept by the
Management Complex (MC) firmware.

The fsl-mc bus driver has this pattern:

static void fsl_mc_bus_shutdown(struct platform_device *pdev)
{
fsl_mc_bus_remove(pdev);
}

which proceeds to remove the children on the bus. Among those children,
the dpaa2-eth network driver.

When dpaa2-eth is a DSA master, this removal of the master on shutdown
trips up the device link created by dsa_master_setup(), and as such, the
Marvell switch is also removed.

From this point on, readers can revisit the description of commits
74b6d7d13307 ("net: dsa: realtek: register the MDIO bus under devres")
5135e96a3dd2 ("net: dsa: don't allocate the slave_mii_bus using devres")

since the prerequisites for the BUG_ON in mdiobus_free() have been
accomplished if there is a devres mismatch between mdiobus_alloc() and
mdiobus_register().

Most DSA drivers have this kind of mismatch, and upon my initial
assessment I had not realized the possibility described above, so I
didn't fix it. This patch series walks through all drivers and makes
them use either fully devres, or no devres.

I am aware that there are DSA drivers that are only known to be tested
with a single DSA master, so some patches are probably overkill for
them. But code is copy-pasted from so many sources without fully
understanding the differences, that I think it's better to not leave an
in-tree source of inspiration that may lead to subtle breakage if not
adapted properly.
====================

Link: https://lore.kernel.org/r/20220207161553.579933-1-vladimir.oltean@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>