2023-01-16 13:12:24

by Michael Walle

[permalink] [raw]
Subject: [PATCH net-next 0/6] net: phy: Remove probe_capabilities

With all the drivers which used .probe_capabilities converted to the
new c45 MDIO access methods, we can now decide based upon these whether
a bus driver supports c45 and we can get rid of the not widely used
probe_capabilites.

Unfortunately, due to a now broader support of c45 scans, this will
trigger a bug on some boards with a (c22-only) Micrel PHY. These PHYs
don't ignore c45 accesses correctly, thinking they are addressed
themselves and distrupt the MDIO access. To avoid this, a blacklist
for c45 scans is introduced.

To: Heiner Kallweit <[email protected]>
To: Russell King <[email protected]>
To: "David S. Miller" <[email protected]>
To: Eric Dumazet <[email protected]>
To: Jakub Kicinski <[email protected]>
To: Paolo Abeni <[email protected]>
To: Felix Fietkau <[email protected]>
To: John Crispin <[email protected]>
To: Sean Wang <[email protected]>
To: Mark Lee <[email protected]>
To: Lorenzo Bianconi <[email protected]>
To: Matthias Brugger <[email protected]>
To: Bryan Whitehead <[email protected]>
To: [email protected]
To: Giuseppe Cavallaro <[email protected]>
To: Alexandre Torgue <[email protected]>
To: Jose Abreu <[email protected]>
To: Maxime Coquelin <[email protected]>
To: Joel Stanley <[email protected]>
To: Andrew Jeffery <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Andrew Lunn <[email protected]>
Signed-off-by: Michael Walle <[email protected]>

---
Andrew Lunn (6):
net: mdio: Move mdiobus_scan() within file
net: mdio: Rework scanning of bus ready for quirks
net: mdio: Add workaround for Micrel PHYs which are not C45 compatible
net: mdio: scan bus based on bus capabilities for C22 and C45
net: phy: Decide on C45 capabilities based on presence of method
net: phy: Remove probe_capabilities

drivers/net/ethernet/adi/adin1110.c | 1 -
drivers/net/ethernet/freescale/xgmac_mdio.c | 1 -
drivers/net/ethernet/marvell/pxa168_eth.c | 2 +-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 1 -
drivers/net/ethernet/microchip/lan743x_main.c | 2 -
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 3 -
drivers/net/mdio/mdio-aspeed.c | 1 -
drivers/net/phy/mdio_bus.c | 194 +++++++++++++++-------
drivers/net/phy/phy_device.c | 2 +-
include/linux/micrel_phy.h | 2 +
include/linux/phy.h | 10 +-
11 files changed, 138 insertions(+), 81 deletions(-)
---
base-commit: c12e2e5b76b2e739ccdf196bee960412b45d5f85
change-id: 20230116-net-next-remove-probe-capabilities-03d401439fc6

Best regards,
--
Michael Walle <[email protected]>


2023-01-16 13:23:02

by Michael Walle

[permalink] [raw]
Subject: [PATCH net-next 1/6] net: mdio: Move mdiobus_scan() within file

From: Andrew Lunn <[email protected]>

No functional change, just place it earlier in preparation for some
refactoring.

While at it, correct the comment format and one typo.

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Michael Walle <[email protected]>
---
drivers/net/phy/mdio_bus.c | 101 ++++++++++++++++++++++-----------------------
1 file changed, 50 insertions(+), 51 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 902e1c88ef58..61c33c6098a1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -506,6 +506,56 @@ static int mdiobus_create_device(struct mii_bus *bus,
return ret;
}

+/**
+ * mdiobus_scan - scan a bus for MDIO devices.
+ * @bus: mii_bus to scan
+ * @addr: address on bus to scan
+ *
+ * This function scans the MDIO bus, looking for devices which can be
+ * identified using a vendor/product ID in registers 2 and 3. Not all
+ * MDIO devices have such registers, but PHY devices typically
+ * do. Hence this function assumes anything found is a PHY, or can be
+ * treated as a PHY. Other MDIO devices, such as switches, will
+ * probably not be found during the scan.
+ */
+struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
+{
+ struct phy_device *phydev = ERR_PTR(-ENODEV);
+ int err;
+
+ switch (bus->probe_capabilities) {
+ case MDIOBUS_NO_CAP:
+ case MDIOBUS_C22:
+ phydev = get_phy_device(bus, addr, false);
+ break;
+ case MDIOBUS_C45:
+ phydev = get_phy_device(bus, addr, true);
+ break;
+ case MDIOBUS_C22_C45:
+ phydev = get_phy_device(bus, addr, false);
+ if (IS_ERR(phydev))
+ phydev = get_phy_device(bus, addr, true);
+ break;
+ }
+
+ if (IS_ERR(phydev))
+ return phydev;
+
+ /* For DT, see if the auto-probed phy has a corresponding child
+ * in the bus node, and set the of_node pointer in this case.
+ */
+ of_mdiobus_link_mdiodev(bus, &phydev->mdio);
+
+ err = phy_device_register(phydev);
+ if (err) {
+ phy_device_free(phydev);
+ return ERR_PTR(-ENODEV);
+ }
+
+ return phydev;
+}
+EXPORT_SYMBOL(mdiobus_scan);
+
/**
* __mdiobus_register - bring up all the PHYs on a given bus and attach them to bus
* @bus: target mii_bus
@@ -679,57 +729,6 @@ void mdiobus_free(struct mii_bus *bus)
}
EXPORT_SYMBOL(mdiobus_free);

-/**
- * mdiobus_scan - scan a bus for MDIO devices.
- * @bus: mii_bus to scan
- * @addr: address on bus to scan
- *
- * This function scans the MDIO bus, looking for devices which can be
- * identified using a vendor/product ID in registers 2 and 3. Not all
- * MDIO devices have such registers, but PHY devices typically
- * do. Hence this function assumes anything found is a PHY, or can be
- * treated as a PHY. Other MDIO devices, such as switches, will
- * probably not be found during the scan.
- */
-struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
-{
- struct phy_device *phydev = ERR_PTR(-ENODEV);
- int err;
-
- switch (bus->probe_capabilities) {
- case MDIOBUS_NO_CAP:
- case MDIOBUS_C22:
- phydev = get_phy_device(bus, addr, false);
- break;
- case MDIOBUS_C45:
- phydev = get_phy_device(bus, addr, true);
- break;
- case MDIOBUS_C22_C45:
- phydev = get_phy_device(bus, addr, false);
- if (IS_ERR(phydev))
- phydev = get_phy_device(bus, addr, true);
- break;
- }
-
- if (IS_ERR(phydev))
- return phydev;
-
- /*
- * For DT, see if the auto-probed phy has a correspoding child
- * in the bus node, and set the of_node pointer in this case.
- */
- of_mdiobus_link_mdiodev(bus, &phydev->mdio);
-
- err = phy_device_register(phydev);
- if (err) {
- phy_device_free(phydev);
- return ERR_PTR(-ENODEV);
- }
-
- return phydev;
-}
-EXPORT_SYMBOL(mdiobus_scan);
-
static void mdiobus_stats_acct(struct mdio_bus_stats *stats, bool op, int ret)
{
preempt_disable();

--
2.30.2

2023-01-16 14:41:04

by Michael Walle

[permalink] [raw]
Subject: [PATCH net-next 4/6] net: mdio: scan bus based on bus capabilities for C22 and C45

From: Andrew Lunn <[email protected]>

Now that all MDIO bus drivers which set probe_capabilities to
MDIOBUS_C22_C45 have been converted to use the name API for C45
transactions, perform the scanning of the bus based on which methods
the bus provides.

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Michael Walle <[email protected]>
---
drivers/net/phy/mdio_bus.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index f173c91842e0..34790e601cb1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -710,9 +710,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
goto error_reset_gpiod;
}

- if (bus->probe_capabilities == MDIOBUS_NO_CAP ||
- bus->probe_capabilities == MDIOBUS_C22 ||
- bus->probe_capabilities == MDIOBUS_C22_C45) {
+ if (bus->read) {
err = mdiobus_scan_bus_c22(bus);
if (err)
goto error;
@@ -720,9 +718,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)

prevent_c45_scan = mdiobus_prevent_c45_scan(bus);

- if (!prevent_c45_scan &&
- (bus->probe_capabilities == MDIOBUS_C45 ||
- bus->probe_capabilities == MDIOBUS_C22_C45)) {
+ if (!prevent_c45_scan && bus->read_c45) {
err = mdiobus_scan_bus_c45(bus);
if (err)
goto error;

--
2.30.2

2023-01-18 01:28:31

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [PATCH net-next 4/6] net: mdio: scan bus based on bus capabilities for C22 and C45

On 1/16/2023 4:55 AM, Michael Walle wrote:
> From: Andrew Lunn <[email protected]>
>
> Now that all MDIO bus drivers which set probe_capabilities to
> MDIOBUS_C22_C45 have been converted to use the name API for C45
> transactions, perform the scanning of the bus based on which methods
> the bus provides.
>
> Signed-off-by: Andrew Lunn <[email protected]>
> Signed-off-by: Michael Walle <[email protected]>

Nice, cleanup is looking much better

Reviewed-by: Jesse Brandeburg <[email protected]>


2023-01-18 01:39:50

by Jesse Brandeburg

[permalink] [raw]
Subject: Re: [PATCH net-next 1/6] net: mdio: Move mdiobus_scan() within file

On 1/16/2023 4:55 AM, Michael Walle wrote:
> From: Andrew Lunn <[email protected]>
>
> No functional change, just place it earlier in preparation for some
> refactoring.
>
> While at it, correct the comment format and one typo.
>
> Signed-off-by: Andrew Lunn <[email protected]>
> Signed-off-by: Michael Walle <[email protected]>

Reviewed-by: Jesse Brandeburg <[email protected]>