linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
@ 2020-11-19 20:34 Grygorii Strashko
  2020-11-19 21:11 ` Heiner Kallweit
  2020-11-21  3:12 ` Jakub Kicinski
  0 siblings, 2 replies; 10+ messages in thread
From: Grygorii Strashko @ 2020-11-19 20:34 UTC (permalink / raw)
  To: David S. Miller, netdev, Jakub Kicinski, Andrew Lunn, Heiner Kallweit
  Cc: linux-kernel, 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 which 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 for devm_gpiod_get_optional() returning
-EPROBE_DEFER case by using dev_err_probe().

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/phy/mdio_bus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 757e950fb745..83cd61c3dd01 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -546,10 +546,10 @@ 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 = dev_err_probe(&bus->dev, PTR_ERR(gpiod),
+				    "mii_bus %s couldn't get reset GPIO\n", bus->id);
 		device_del(&bus->dev);
-		return PTR_ERR(gpiod);
+		return err;
 	} else	if (gpiod) {
 		bus->reset_gpiod = gpiod;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 20:34 [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER Grygorii Strashko
@ 2020-11-19 21:11 ` Heiner Kallweit
  2020-11-19 21:17   ` Grygorii Strashko
  2020-11-21  3:12 ` Jakub Kicinski
  1 sibling, 1 reply; 10+ messages in thread
From: Heiner Kallweit @ 2020-11-19 21:11 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev, Jakub Kicinski, Andrew Lunn
  Cc: linux-kernel

Am 19.11.2020 um 21:34 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 which 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 for devm_gpiod_get_optional() returning
> -EPROBE_DEFER case by using dev_err_probe().
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/net/phy/mdio_bus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 757e950fb745..83cd61c3dd01 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -546,10 +546,10 @@ 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 = dev_err_probe(&bus->dev, PTR_ERR(gpiod),
> +				    "mii_bus %s couldn't get reset GPIO\n", bus->id);

Doesn't checkpatch complain about line length > 80 here?

>  		device_del(&bus->dev);
> -		return PTR_ERR(gpiod);
> +		return err;
>  	} else	if (gpiod) {
>  		bus->reset_gpiod = gpiod;
>  
> 

Last but not least the net or net-next patch annotation is missing.
I'd be fine with treating the change as an improvement (net-next).

Apart from that change looks good to me.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 21:11 ` Heiner Kallweit
@ 2020-11-19 21:17   ` Grygorii Strashko
  2020-11-19 21:23     ` Heiner Kallweit
  0 siblings, 1 reply; 10+ messages in thread
From: Grygorii Strashko @ 2020-11-19 21:17 UTC (permalink / raw)
  To: Heiner Kallweit, David S. Miller, netdev, Jakub Kicinski, Andrew Lunn
  Cc: linux-kernel



On 19/11/2020 23:11, Heiner Kallweit wrote:
> Am 19.11.2020 um 21:34 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 which 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 for devm_gpiod_get_optional() returning
>> -EPROBE_DEFER case by using dev_err_probe().
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   drivers/net/phy/mdio_bus.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 757e950fb745..83cd61c3dd01 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -546,10 +546,10 @@ 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 = dev_err_probe(&bus->dev, PTR_ERR(gpiod),
>> +				    "mii_bus %s couldn't get reset GPIO\n", bus->id);
> 
> Doesn't checkpatch complain about line length > 80 here?

:)

commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
Author: Joe Perches <joe@perches.com>
Date:   Fri May 29 16:12:21 2020 -0700

     checkpatch/coding-style: deprecate 80-column warning

> 
>>   		device_del(&bus->dev);
>> -		return PTR_ERR(gpiod);
>> +		return err;
>>   	} else	if (gpiod) {
>>   		bus->reset_gpiod = gpiod;
>>   
>>
> 
> Last but not least the net or net-next patch annotation is missing.
> I'd be fine with treating the change as an improvement (net-next).
> 
> Apart from that change looks good to me.
> 

-- 
Best regards,
grygorii

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 21:17   ` Grygorii Strashko
@ 2020-11-19 21:23     ` Heiner Kallweit
  2020-11-19 21:41       ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: Heiner Kallweit @ 2020-11-19 21:23 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, netdev, Jakub Kicinski, Andrew Lunn
  Cc: linux-kernel

Am 19.11.2020 um 22:17 schrieb Grygorii Strashko:
> 
> 
> On 19/11/2020 23:11, Heiner Kallweit wrote:
>> Am 19.11.2020 um 21:34 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 which 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 for devm_gpiod_get_optional() returning
>>> -EPROBE_DEFER case by using dev_err_probe().
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   drivers/net/phy/mdio_bus.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>> index 757e950fb745..83cd61c3dd01 100644
>>> --- a/drivers/net/phy/mdio_bus.c
>>> +++ b/drivers/net/phy/mdio_bus.c
>>> @@ -546,10 +546,10 @@ 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 = dev_err_probe(&bus->dev, PTR_ERR(gpiod),
>>> +                    "mii_bus %s couldn't get reset GPIO\n", bus->id);
>>
>> Doesn't checkpatch complain about line length > 80 here?
> 
> :)
> 
> commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> Author: Joe Perches <joe@perches.com>
> Date:   Fri May 29 16:12:21 2020 -0700
> 
>     checkpatch/coding-style: deprecate 80-column warning
> 

Ah, again something learnt. Thanks for the reference.

>>
>>>           device_del(&bus->dev);
>>> -        return PTR_ERR(gpiod);
>>> +        return err;
>>>       } else    if (gpiod) {
>>>           bus->reset_gpiod = gpiod;
>>>  
>>
>> Last but not least the net or net-next patch annotation is missing.
>> I'd be fine with treating the change as an improvement (net-next).
>>
>> Apart from that change looks good to me.
>>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 21:23     ` Heiner Kallweit
@ 2020-11-19 21:41       ` Andrew Lunn
  2020-11-19 22:09         ` Heiner Kallweit
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2020-11-19 21:41 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Grygorii Strashko, David S. Miller, netdev, Jakub Kicinski, linux-kernel

> >> Doesn't checkpatch complain about line length > 80 here?
> > 
> > :)
> > 
> > commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> > Author: Joe Perches <joe@perches.com>
> > Date:   Fri May 29 16:12:21 2020 -0700
> > 
> >     checkpatch/coding-style: deprecate 80-column warning
> > 
> 
> Ah, again something learnt. Thanks for the reference.

But it then got revoked for netdev. Or at least it was planned to
re-impose 80 for netdev. I don't know if checkpatch got patched yet.

	  Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 21:41       ` Andrew Lunn
@ 2020-11-19 22:09         ` Heiner Kallweit
  2020-11-20  5:21           ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Heiner Kallweit @ 2020-11-19 22:09 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Grygorii Strashko, David S. Miller, netdev, Jakub Kicinski, linux-kernel

Am 19.11.2020 um 22:41 schrieb Andrew Lunn:
>>>> Doesn't checkpatch complain about line length > 80 here?
>>>
>>> :)
>>>
>>> commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
>>> Author: Joe Perches <joe@perches.com>
>>> Date:   Fri May 29 16:12:21 2020 -0700
>>>
>>>     checkpatch/coding-style: deprecate 80-column warning
>>>
>>
>> Ah, again something learnt. Thanks for the reference.
> 
> But it then got revoked for netdev. Or at least it was planned to
> re-impose 80 for netdev. I don't know if checkpatch got patched yet.
> 
> 	  Andrew
> 
At a first glance it sounds strange that subsystems may define own
rules for such basic things. But supposedly there has been a longer
emotional disucssion about this already ..

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 22:09         ` Heiner Kallweit
@ 2020-11-20  5:21           ` Jakub Kicinski
  2020-11-20  7:06             ` Joe Perches
  0 siblings, 1 reply; 10+ messages in thread
From: Jakub Kicinski @ 2020-11-20  5:21 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Andrew Lunn, Grygorii Strashko, David S. Miller, netdev, linux-kernel

On Thu, 19 Nov 2020 23:09:52 +0100 Heiner Kallweit wrote:
> Am 19.11.2020 um 22:41 schrieb Andrew Lunn:
> >>>> Doesn't checkpatch complain about line length > 80 here?  
> >>>
> >>> :)
> >>>
> >>> commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> >>> Author: Joe Perches <joe@perches.com>
> >>> Date:   Fri May 29 16:12:21 2020 -0700
> >>>
> >>>     checkpatch/coding-style: deprecate 80-column warning
> >>>  
> >>
> >> Ah, again something learnt. Thanks for the reference.  
> > 
> > But it then got revoked for netdev. Or at least it was planned to
> > re-impose 80 for netdev. I don't know if checkpatch got patched yet.

FWIW I had a patch for it but before I sent it Dave suggested I ask
around and Alexei was opposed.

And I don't have the strength to argue :)

I'll just tell people case by case when they have 4+ indentation levels
in their code or use 40+ character variables/defines, in my copious
spare time. 

> At a first glance it sounds strange that subsystems may define own
> rules for such basic things. But supposedly there has been a longer
> emotional disucssion about this already ..

We do have our own comment style rule in networking since the beginning
of time, and reverse xmas tree, so it's not completely crazy.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-20  5:21           ` Jakub Kicinski
@ 2020-11-20  7:06             ` Joe Perches
  2020-11-20 14:50               ` Andrew Lunn
  0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2020-11-20  7:06 UTC (permalink / raw)
  To: Jakub Kicinski, Heiner Kallweit
  Cc: Andrew Lunn, Grygorii Strashko, David S. Miller, netdev, linux-kernel

On Thu, 2020-11-19 at 21:21 -0800, Jakub Kicinski wrote:
> We do have our own comment style rule in networking since the beginning
> of time, and reverse xmas tree, so it's not completely crazy.

reverse xmas tree is completely crazy.

But I posted a patch to checkpatch to suggest it for net/
and drivers/net/ once

https://lkml.org/lkml/2016/11/4/54



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-20  7:06             ` Joe Perches
@ 2020-11-20 14:50               ` Andrew Lunn
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2020-11-20 14:50 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jakub Kicinski, Heiner Kallweit, Grygorii Strashko,
	David S. Miller, netdev, linux-kernel

Hi Joe

> reverse xmas tree is completely crazy.
> 
> But I posted a patch to checkpatch to suggest it for net/
> and drivers/net/ once
> 
> https://lkml.org/lkml/2016/11/4/54
 

> From Joe Perches <> 
>
...

> and the reverse xmas tree helpfulness of looking up the
> type of bar is neither obvious nor easy.
>
> My preference would be for a bar that serves coffee and alcohol.

Ah, those were the days.

Anyway, can this patch be brought back to life, with the problem
pointed out fixed? It is still something we do in netdev, and a
machine can spot these problems better than a human maintainer or
developer.

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
  2020-11-19 20:34 [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER Grygorii Strashko
  2020-11-19 21:11 ` Heiner Kallweit
@ 2020-11-21  3:12 ` Jakub Kicinski
  1 sibling, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2020-11-21  3:12 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Andrew Lunn, Heiner Kallweit, linux-kernel

On Thu, 19 Nov 2020 22:34:46 +0200 Grygorii Strashko wrote:
> 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 which 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 for devm_gpiod_get_optional() returning
> -EPROBE_DEFER case by using dev_err_probe().
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Applied (with the line wrapped), thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-21  3:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19 20:34 [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER Grygorii Strashko
2020-11-19 21:11 ` Heiner Kallweit
2020-11-19 21:17   ` Grygorii Strashko
2020-11-19 21:23     ` Heiner Kallweit
2020-11-19 21:41       ` Andrew Lunn
2020-11-19 22:09         ` Heiner Kallweit
2020-11-20  5:21           ` Jakub Kicinski
2020-11-20  7:06             ` Joe Perches
2020-11-20 14:50               ` Andrew Lunn
2020-11-21  3:12 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).