| Message ID | 20200619044759.11387-2-f.fainelli@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | net: phy: MDIO bus scanning fixes | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 12 lines checked |
On Thu, Jun 18, 2020 at 09:47:58PM -0700, Florian Fainelli wrote: > Commit 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()") > introduced a break of the loop on the premise that a successful > registration should exit the loop. The premise is correct but not to > code, because rc && rc != -ENODEV is just a special error condition, > that means we would exit the loop even with rc == -ENODEV which is > absolutely not correct since this is the error code to indicate to the > MDIO bus layer that scanning should continue. > > Fix this by explicitly checking for rc = 0 as the only valid condition > to break out of the loop. > > Fixes: 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()") > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > drivers/of/of_mdio.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c > index a04afe79529c..7496dc64d6b5 100644 > --- a/drivers/of/of_mdio.c > +++ b/drivers/of/of_mdio.c > @@ -315,9 +315,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) > > if (of_mdiobus_child_is_phy(child)) { > rc = of_mdiobus_register_phy(mdio, child, addr); > - if (rc && rc != -ENODEV) > + if (!rc) > + break; Maybe add in a comment here about what ENODEV means in this context? That might avoid it getting broken again in the future. > + if (rc != -ENODEV) > goto unregister; > - break; > } > } > } > -- Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c index a04afe79529c..7496dc64d6b5 100644 --- a/drivers/of/of_mdio.c +++ b/drivers/of/of_mdio.c @@ -315,9 +315,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) if (of_mdiobus_child_is_phy(child)) { rc = of_mdiobus_register_phy(mdio, child, addr); - if (rc && rc != -ENODEV) + if (!rc) + break; + if (rc != -ENODEV) goto unregister; - break; } } }
Commit 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()") introduced a break of the loop on the premise that a successful registration should exit the loop. The premise is correct but not to code, because rc && rc != -ENODEV is just a special error condition, that means we would exit the loop even with rc == -ENODEV which is absolutely not correct since this is the error code to indicate to the MDIO bus layer that scanning should continue. Fix this by explicitly checking for rc = 0 as the only valid condition to break out of the loop. Fixes: 209c65b61d94 ("drivers/of/of_mdio.c:fix of_mdiobus_register()") Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- drivers/of/of_mdio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)