| Message ID | 20201118142426.25369-1-grygorii.strashko@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mdio_bus: suppress err message for reset gpio EPROBE_DEFER | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 100 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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.
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.
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 --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;
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(-)