* [PATCH net v2 1/2] of: of_mdio: Correct loop scanning logic
2020-06-19 18:47 [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes Florian Fainelli
@ 2020-06-19 18:47 ` Florian Fainelli
2020-06-19 18:47 ` [PATCH net v2 2/2] net: phy: Check harder for errors in get_phy_id() Florian Fainelli
2020-06-19 20:40 ` [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2020-06-19 18:47 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Jakub Kicinski, Rob Herring, Frank Rowand,
Dajun Jin, Alexandre Belloni, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
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()")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index a04afe79529c..ef6f818ce5b3 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -314,10 +314,15 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
child, addr);
if (of_mdiobus_child_is_phy(child)) {
+ /* -ENODEV is the return code that PHYLIB has
+ * standardized on to indicate that bus
+ * scanning should continue.
+ */
rc = of_mdiobus_register_phy(mdio, child, addr);
- if (rc && rc != -ENODEV)
+ if (!rc)
+ break;
+ if (rc != -ENODEV)
goto unregister;
- break;
}
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net v2 2/2] net: phy: Check harder for errors in get_phy_id()
2020-06-19 18:47 [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes Florian Fainelli
2020-06-19 18:47 ` [PATCH net v2 1/2] of: of_mdio: Correct loop scanning logic Florian Fainelli
@ 2020-06-19 18:47 ` Florian Fainelli
2020-06-19 20:40 ` [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2020-06-19 18:47 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Heiner Kallweit, Russell King,
David S. Miller, Jakub Kicinski, Rob Herring, Frank Rowand,
Dajun Jin, Alexandre Belloni, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
Commit 02a6efcab675 ("net: phy: allow scanning busses with missing
phys") added a special condition to return -ENODEV in case -ENODEV or
-EIO was returned from the first read of the MII_PHYSID1 register.
In case the MDIO bus data line pull-up is not strong enough, the MDIO
bus controller will not flag this as a read error. This can happen when
a pluggable daughter card is not connected and weak internal pull-ups
are used (since that is the only option, otherwise the pins are
floating).
The second read of MII_PHYSID2 will be correctly flagged an error
though, but now we will return -EIO which will be treated as a hard
error, thus preventing MDIO bus scanning loops to continue succesfully.
Apply the same logic to both register reads, thus allowing the scanning
logic to proceed.
Fixes: 02a6efcab675 ("net: phy: allow scanning busses with missing phys")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy_device.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 04946de74fa0..85ba95b598b5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -794,8 +794,10 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id,
/* Grab the bits from PHYIR2, and put them in the lower half */
phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
- if (phy_reg < 0)
- return -EIO;
+ if (phy_reg < 0) {
+ /* returning -ENODEV doesn't stop bus scanning */
+ return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
+ }
*phy_id |= phy_reg;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes
2020-06-19 18:47 [PATCH net v2 0/2] net: phy: MDIO bus scanning fixes Florian Fainelli
2020-06-19 18:47 ` [PATCH net v2 1/2] of: of_mdio: Correct loop scanning logic Florian Fainelli
2020-06-19 18:47 ` [PATCH net v2 2/2] net: phy: Check harder for errors in get_phy_id() Florian Fainelli
@ 2020-06-19 20:40 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-06-19 20:40 UTC (permalink / raw)
To: f.fainelli
Cc: netdev, andrew, hkallweit1, linux, kuba, robh+dt, frowand.list,
adajunjin, alexandre.belloni, linux-kernel, devicetree
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 19 Jun 2020 11:47:45 -0700
> This patch series fixes two problems with the current MDIO bus scanning
> logic which was identified while moving from 4.9 to 5.4 on devices that
> do rely on scanning the MDIO bus at runtime because they use pluggable
> cards.
>
> Changes in v2:
>
> - added comment explaining the special value of -ENODEV
> - added Andrew's Reviewed-by tag
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread