[PATCH v3 net 0/1] net: macb: Fix regression breaking non-MDIO fixed-link PHYs

[PATCH v3 net 0/1] net: macb: Fix regression breaking non-MDIO fixed-link PHYs

[Ahmad Fatoum]

commit 739de9a1563a ("net: macb: Reorganize macb_mii bringup") broke
initializing macb on the EVB-KSZ9477 eval board.

The board's SoC has a macb MAC connected to the KSZ9477 over RGMII, this
is represented as a fixed-link as follows in the (non-mainline) device tree:

	macb0: ethernet@f0028000 {
		phy-mode = "rgmii";
		gpios = <&pioB 28 GPIO_ACTIVE_LOW>;
		reset-gpios = <&pioC 31 GPIO_ACTIVE_LOW>;
		status = "okay";
		
		fixed-link {
			speed = <1000>;
			full-duplex;
		};
	};

The following patch fixes the regression by partially reverting the offending
commit and can be backported to stable.

Follow-up patches that popped up during review will be sent when net-next opens.
They improve error reporting and add a "mdio" subtree binding to contain macb's
MDIO-managed PHYs, similar to the binding in other drivers.

This is v3. v2 started with
Message-Id: <20180820121238.7779-1-a.fatoum@pengutronix.de>.
without explicit marking as v2.
v1 started with Message-Id: <20180814141240.9085-1-a.fatoum@pengutronix.de>.

Changes introduced in v1 and v2 are noted in-line in their respective patches.


Ahmad Fatoum (1):
  net: macb: Fix regression breaking non-MDIO fixed-link PHYs

 drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++---------
 1 file changed, 17 insertions(+), 10 deletions(-)

-- 
2.18.0

[PATCH v3 net 1/1] net: macb: Fix regression breaking non-MDIO fixed-link PHYs

[Ahmad Fatoum]

commit 739de9a1563a ("net: macb: Reorganize macb_mii bringup") broke
initializing macb on the EVB-KSZ9477 eval board.
There, of_mdiobus_register was called even for the fixed-link representing
the RGMII-link to the switch with the result that the driver attempts to
enumerate PHYs on a non-existent MDIO bus:

	libphy: MACB_mii_bus: probed
	mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
        [snip]
	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31

The "MDIO" bus registration succeeds regardless, having claimed the reset GPIO,
and calling of_phy_register_fixed_link later on fails because it tries
to claim the same GPIO:

	macb f0028000.ethernet: broken fixed-link specification

Fix this by registering the fixed-link before calling mdiobus_register.

Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---

Notes:
    Changes since v2:
    	Amend Commit message to address the double GPIO registration
    	s/SPI/RGMII/ in commit message (SPI is only for switch register access)
    	Change error message to include DT path to fixed-link
    Changes since v1:
    	Add one more error path for failing to register fixed-link
    	Fix a whitespace issue

 drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++---------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dc09f9a8a49b..f46b854d34b5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -482,11 +482,6 @@ static int macb_mii_probe(struct net_device *dev)
 
 	if (np) {
 		if (of_phy_is_fixed_link(np)) {
-			if (of_phy_register_fixed_link(np) < 0) {
-				dev_err(&bp->pdev->dev,
-					"broken fixed-link specification\n");
-				return -ENODEV;
-			}
 			bp->phy_node = of_node_get(np);
 		} else {
 			bp->phy_node = of_parse_phandle(np, "phy-handle", 0);
@@ -569,7 +564,7 @@ static int macb_mii_init(struct macb *bp)
 {
 	struct macb_platform_data *pdata;
 	struct device_node *np;
-	int err;
+	int err = -ENXIO;
 
 	/* Enable management port */
 	macb_writel(bp, NCR, MACB_BIT(MPE));
@@ -592,12 +587,23 @@ static int macb_mii_init(struct macb *bp)
 	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
 
 	np = bp->pdev->dev.of_node;
-	if (pdata)
-		bp->mii_bus->phy_mask = pdata->phy_mask;
+	if (np && of_phy_is_fixed_link(np)) {
+		if (of_phy_register_fixed_link(np) < 0) {
+			dev_err(&bp->pdev->dev,
+				"broken fixed-link specification %pOF\n", np);
+			goto err_out_free_mdiobus;
+		}
+
+		err = mdiobus_register(bp->mii_bus);
+	} else {
+		if (pdata)
+			bp->mii_bus->phy_mask = pdata->phy_mask;
+
+		err = of_mdiobus_register(bp->mii_bus, np);
+	}
 
-	err = of_mdiobus_register(bp->mii_bus, np);
 	if (err)
-		goto err_out_free_mdiobus;
+		goto err_out_free_fixed_link;
 
 	err = macb_mii_probe(bp->dev);
 	if (err)
@@ -607,6 +613,7 @@ static int macb_mii_init(struct macb *bp)
 
 err_out_unregister_bus:
 	mdiobus_unregister(bp->mii_bus);
+err_out_free_fixed_link:
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
 err_out_free_mdiobus:
-- 
2.18.0

Re: [PATCH v3 net 1/1] net: macb: Fix regression breaking non-MDIO fixed-link PHYs

[David Miller]

From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Tue, 21 Aug 2018 17:35:48 +0200

> commit 739de9a1563a ("net: macb: Reorganize macb_mii bringup") broke
> initializing macb on the EVB-KSZ9477 eval board.
> There, of_mdiobus_register was called even for the fixed-link representing
> the RGMII-link to the switch with the result that the driver attempts to
> enumerate PHYs on a non-existent MDIO bus:
> 
> 	libphy: MACB_mii_bus: probed
> 	mdio_bus f0028000.ethernet-ffffffff: fixed-link has invalid PHY address
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 0
>         [snip]
> 	mdio_bus f0028000.ethernet-ffffffff: scan phy fixed-link at address 31
> 
> The "MDIO" bus registration succeeds regardless, having claimed the reset GPIO,
> and calling of_phy_register_fixed_link later on fails because it tries
> to claim the same GPIO:
> 
> 	macb f0028000.ethernet: broken fixed-link specification
> 
> Fix this by registering the fixed-link before calling mdiobus_register.
> 
> Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Applied and queued up for -stable, thanks.