diff mbox series

mdio_bus: suppress err message for reset gpio EPROBE_DEFER

Message ID 20201118142426.25369-1-grygorii.strashko@ti.com
State Superseded
Headers show
Series mdio_bus: suppress err message for reset gpio EPROBE_DEFER | expand

Commit Message

Grygorii Strashko Nov. 18, 2020, 2:24 p.m. UTC 
The mdio_bus may have dependencies from GPIO controller and so got
deferred. Now it will print error message every time -EPROBE_DEFER is
returned from:
__mdiobus_register()
 |-devm_gpiod_get_optional()
without actually identifying error code.

"mdio_bus 4a101000.mdio: mii_bus 4a101000.mdio couldn't get reset GPIO"

Hence, suppress error message when devm_gpiod_get_optional() returning
-EPROBE_DEFER case.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

---
 drivers/net/phy/mdio_bus.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
2.17.1

Comments

Heiner Kallweit Nov. 19, 2020, 12:30 p.m. UTC | #1 
Am 18.11.2020 um 15:24 schrieb Grygorii Strashko:
> The mdio_bus may have dependencies from GPIO controller and so got

> deferred. Now it will print error message every time -EPROBE_DEFER is

> returned from:

> __mdiobus_register()

>  |-devm_gpiod_get_optional()

> without actually identifying error code.

> 

> "mdio_bus 4a101000.mdio: mii_bus 4a101000.mdio couldn't get reset GPIO"

> 

> Hence, suppress error message when devm_gpiod_get_optional() returning

> -EPROBE_DEFER case.

> 

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

> ---

>  drivers/net/phy/mdio_bus.c | 7 ++++---

>  1 file changed, 4 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c

> index 757e950fb745..54fc13043656 100644

> --- a/drivers/net/phy/mdio_bus.c

> +++ b/drivers/net/phy/mdio_bus.c

> @@ -546,10 +546,11 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)

>  	/* de-assert bus level PHY GPIO reset */

>  	gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);

>  	if (IS_ERR(gpiod)) {

> -		dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",

> -			bus->id);

> +		err = PTR_ERR(gpiod);

> +		if (err != -EPROBE_DEFER)

> +			dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO %d\n", bus->id, err);

>  		device_del(&bus->dev);

> -		return PTR_ERR(gpiod);

> +		return err;

>  	} else	if (gpiod) {

>  		bus->reset_gpiod = gpiod;

>  

> 


Using dev_err_probe() here would simplify the code.
Grygorii Strashko Nov. 19, 2020, 1:38 p.m. UTC | #2 
On 19/11/2020 14:30, Heiner Kallweit wrote:
> Am 18.11.2020 um 15:24 schrieb Grygorii Strashko:

>> The mdio_bus may have dependencies from GPIO controller and so got

>> deferred. Now it will print error message every time -EPROBE_DEFER is

>> returned from:

>> __mdiobus_register()

>>   |-devm_gpiod_get_optional()

>> without actually identifying error code.

>>

>> "mdio_bus 4a101000.mdio: mii_bus 4a101000.mdio couldn't get reset GPIO"

>>

>> Hence, suppress error message when devm_gpiod_get_optional() returning

>> -EPROBE_DEFER case.

>>

>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

>> ---

>>   drivers/net/phy/mdio_bus.c | 7 ++++---

>>   1 file changed, 4 insertions(+), 3 deletions(-)

>>

>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c

>> index 757e950fb745..54fc13043656 100644

>> --- a/drivers/net/phy/mdio_bus.c

>> +++ b/drivers/net/phy/mdio_bus.c

>> @@ -546,10 +546,11 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)

>>   	/* de-assert bus level PHY GPIO reset */

>>   	gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);

>>   	if (IS_ERR(gpiod)) {

>> -		dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",

>> -			bus->id);

>> +		err = PTR_ERR(gpiod);

>> +		if (err != -EPROBE_DEFER)

>> +			dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO %d\n", bus->id, err);

>>   		device_del(&bus->dev);

>> -		return PTR_ERR(gpiod);

>> +		return err;

>>   	} else	if (gpiod) {

>>   		bus->reset_gpiod = gpiod;

>>   

>>

> 

> Using dev_err_probe() here would simplify the code.

> 


this was my first though, but was not sure if it's correct as dev_err_probe() will use dev
to store defer reason, but the same 'dev' is deleted on the next line.

I also thought about using bus->parent, but it's not always provided.

So, if you think dev_err_probe(0) can be used - I can change and re-send.

-- 
Best regards,
grygorii
Heiner Kallweit Nov. 19, 2020, 4:58 p.m. UTC | #3 
Am 19.11.2020 um 14:38 schrieb Grygorii Strashko:
> 

> 

> On 19/11/2020 14:30, Heiner Kallweit wrote:

>> Am 18.11.2020 um 15:24 schrieb Grygorii Strashko:

>>> The mdio_bus may have dependencies from GPIO controller and so got

>>> deferred. Now it will print error message every time -EPROBE_DEFER is

>>> returned from:

>>> __mdiobus_register()

>>>   |-devm_gpiod_get_optional()

>>> without actually identifying error code.

>>>

>>> "mdio_bus 4a101000.mdio: mii_bus 4a101000.mdio couldn't get reset GPIO"

>>>

>>> Hence, suppress error message when devm_gpiod_get_optional() returning

>>> -EPROBE_DEFER case.

>>>

>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

>>> ---

>>>   drivers/net/phy/mdio_bus.c | 7 ++++---

>>>   1 file changed, 4 insertions(+), 3 deletions(-)

>>>

>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c

>>> index 757e950fb745..54fc13043656 100644

>>> --- a/drivers/net/phy/mdio_bus.c

>>> +++ b/drivers/net/phy/mdio_bus.c

>>> @@ -546,10 +546,11 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)

>>>       /* de-assert bus level PHY GPIO reset */

>>>       gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);

>>>       if (IS_ERR(gpiod)) {

>>> -        dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",

>>> -            bus->id);

>>> +        err = PTR_ERR(gpiod);

>>> +        if (err != -EPROBE_DEFER)

>>> +            dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO %d\n", bus->id, err);

>>>           device_del(&bus->dev);

>>> -        return PTR_ERR(gpiod);

>>> +        return err;

>>>       } else    if (gpiod) {

>>>           bus->reset_gpiod = gpiod;

>>>  

>>

>> Using dev_err_probe() here would simplify the code.

>>

> 

> this was my first though, but was not sure if it's correct as dev_err_probe() will use dev

> to store defer reason, but the same 'dev' is deleted on the next line.

> 

If you look at device_del() you see that it calls driver_deferred_probe_del()
which frees the reason string. Means you're right in a way that storing
the deferral reason doesn't provide any benefit here, but it also doesn't
hurt.
Good thing about using dev_err_probe() is also that it prints a debug info
in case of deferral what may help people chasing an issue involving this
deferral.

> I also thought about using bus->parent, but it's not always provided.

> 

> So, if you think dev_err_probe(0) can be used - I can change and re-send.

>
diff mbox series

Patch

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 757e950fb745..54fc13043656 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -546,10 +546,11 @@  int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 	/* de-assert bus level PHY GPIO reset */
 	gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(gpiod)) {
-		dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",
-			bus->id);
+		err = PTR_ERR(gpiod);
+		if (err != -EPROBE_DEFER)
+			dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO %d\n", bus->id, err);
 		device_del(&bus->dev);
-		return PTR_ERR(gpiod);
+		return err;
 	} else	if (gpiod) {
 		bus->reset_gpiod = gpiod;