lkml.org 
[lkml]   [2023]   [Feb]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] net: mdio: thunder: Do not unregister buses that have not been registered
From
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
>>
>>
>

\
 
 \ /
  Last update: 2023-03-27 00:25    [W:0.611 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site