Messages in this thread | | | Date | Thu, 16 Feb 2023 07:45:33 +0100 | Subject | Re: [PATCH] net: mdio: thunder: Do not unregister buses that have not been registered | From | Christophe JAILLET <> |
| |
Le 13/05/2021 à 14:16, Russell King - ARM Linux admin a écrit : > On Thu, May 13, 2021 at 01:51:40PM +0200, Christophe JAILLET wrote: >> In the probe, if 'of_mdiobus_register()' fails, 'nexus->buses[i]' will >> still have a non-NULL value. >> So in the remove function, we will try to unregister a bus that has not >> been registered. >> >> In order to avoid that NULLify 'nexus->buses[i]'. >> 'oct_mdio_writeq(0,...)' must also be called here. >> >> Suggested-by: Russell King - ARM Linux admin <linux@armlinux.org.uk> >> Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO buses.") >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> Calling 'devm_mdiobus_free()' would also be cleaner, IMHO. >> I've not added it because: >> - it should be fine, even without it >> - I'm not sure how to use it > > devm_mdiobus_free() is a static function not intended to be used by > drivers. There is no devm.*free() function available for this, so > this memory will only ever be freed when either probe fails or the > driver is unbound from its device. > > That should be fine, but it would be nice to give that memory back > to the system. Without having a function for drivers to use though, > that's not possible. Such a function should take a struct device > pointer and the struct mii_bus pointer returned by the devm > allocation function. > > So, unless Andrew things we really need to free that, what you're > doing below should be fine as far as setting the pointer to NULL. > > I think I'd want comments from Cavium on setting the register to > zero - as we don't know how this hardware behaves, and whether that > would have implications we aren't aware of. So, I'm copying in > David Daney (the original driver author) for comment, if his email > address still works!
Hi,
drivers/net/mdio/mdio-thunder.c has been touched recently, so i take the opportunity to ping on this old patch.
It does not cleanly apply anymore, but still look valid to me.
CJ
> >> --- >> drivers/net/mdio/mdio-thunder.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/mdio/mdio-thunder.c b/drivers/net/mdio/mdio-thunder.c >> index 822d2cdd2f35..140c405d4a41 100644 >> --- a/drivers/net/mdio/mdio-thunder.c >> +++ b/drivers/net/mdio/mdio-thunder.c >> @@ -97,8 +97,14 @@ static int thunder_mdiobus_pci_probe(struct pci_dev *pdev, >> bus->mii_bus->write = cavium_mdiobus_write; >> >> err = of_mdiobus_register(bus->mii_bus, node); >> - if (err) >> + if (err) { >> dev_err(&pdev->dev, "of_mdiobus_register failed\n"); >> + /* non-registered buses must not be unregistered in >> + * the .remove function >> + */ >> + oct_mdio_writeq(0, bus->register_base + SMI_EN); >> + nexus->buses[i] = NULL; >> + } >> >> dev_info(&pdev->dev, "Added bus at %llx\n", r.start); >> if (i >= ARRAY_SIZE(nexus->buses)) >> -- >> 2.30.2 >> >> >
| |