The referenced commit broke initializing macb on the EVB-KSZ9477 eval board. There, of_mdiobus_register was called even for the fixed-link representing the SPI-connected switch PHY, 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 macb f0028000.ethernet: broken fixed-link specification
Cc: stable@vger.kernel.org Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup") Signed-off-by: Ahmad Fatoum a.fatoum@pengutronix.de --- drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-)
Fixes since v1: Added one more error path for failing to register fixed-link Fixed a whitespace issue
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index dc09f9a8a49b..ef6ce8691443 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\n"); + 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:
On Mon, Aug 20, 2018 at 02:12:35PM +0200, Ahmad Fatoum wrote:
The referenced commit broke initializing macb on the EVB-KSZ9477 eval board. There, of_mdiobus_register was called even for the fixed-link representing the SPI-connected switch PHY, with the result that the driver attempts to enumerate PHYs on a non-existent MDIO bus:
libphy: MACB_mii_bus: probed
So there are two different things here:
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
These are the result of the fixed-link being considered a PHY in of_mdiobus_register(). Patch 2 fixes that, turns it into a single warning.
macb f0028000.ethernet: broken fixed-link specification
Why is of_phy_register_fixed_link(np) failing?
Cc: stable@vger.kernel.org Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup") Signed-off-by: Ahmad Fatoum a.fatoum@pengutronix.de
drivers/net/ethernet/cadence/macb_main.c | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-)
Fixes since v1: Added one more error path for failing to register fixed-link Fixed a whitespace issue
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index dc09f9a8a49b..ef6ce8691443 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;
}
As a separate patch, please can you use the error code which of_phy_register_fixed_link() returns, not -ENODEV. It is possible it is returning EPROBE_DEFER.
Andrew
On 08/20/2018 03:55 PM, Andrew Lunn wrote:
Why is of_phy_register_fixed_link(np) failing?
Apparently, the fixed-link's gpio's FLAG_REQUESTED bit remained set causing gpiod_request_commit to return -EBUSY in (v4.18.0):
[<c0667750>] (gpiod_request_commit) from [<c066890c>] (gpiod_request+0x64/0x88) [<c066890c>] (gpiod_request) from [<c066a338>] (gpiod_get_from_of_node+0x48/0x13c) [<c066a338>] (gpiod_get_from_of_node) from [<c094b92c>] (mdiobus_register_device+0x70/0x124) [<c094b92c>] (mdiobus_register_device) from [<c0949d2c>] (phy_device_register+0xc/0xa0) [<c0949d2c>] (phy_device_register) from [<c094e824>] (fixed_phy_register+0xe8/0x1f8) [<c094e824>] (fixed_phy_register) from [<c0b84260>] (of_phy_register_fixed_link+0x150/0x1e4) [<c0b84260>] (of_phy_register_fixed_link) from [<c0964908>] (macb_probe+0x548/0xa7c) [<c0964908>] (macb_probe) from [<c086ebec>] (platform_drv_probe+0x48/0x98)
But that's a separate issue, I'll remove this line from the commit message...
On Mon, Aug 20, 2018 at 05:56:53PM +0200, Ahmad Fatoum wrote:
On 08/20/2018 03:55 PM, Andrew Lunn wrote:
Why is of_phy_register_fixed_link(np) failing?
Apparently, the fixed-link's gpio's FLAG_REQUESTED bit remained set causing gpiod_request_commit to return -EBUSY in (v4.18.0):
[<c0667750>] (gpiod_request_commit) from [<c066890c>] (gpiod_request+0x64/0x88) [<c066890c>] (gpiod_request) from [<c066a338>] (gpiod_get_from_of_node+0x48/0x13c) [<c066a338>] (gpiod_get_from_of_node) from [<c094b92c>] (mdiobus_register_device+0x70/0x124) [<c094b92c>] (mdiobus_register_device) from [<c0949d2c>] (phy_device_register+0xc/0xa0) [<c0949d2c>] (phy_device_register) from [<c094e824>] (fixed_phy_register+0xe8/0x1f8) [<c094e824>] (fixed_phy_register) from [<c0b84260>] (of_phy_register_fixed_link+0x150/0x1e4) [<c0b84260>] (of_phy_register_fixed_link) from [<c0964908>] (macb_probe+0x548/0xa7c) [<c0964908>] (macb_probe) from [<c086ebec>] (platform_drv_probe+0x48/0x98)
But that's a separate issue, I'll remove this line from the commit message...
I would actually say, this is your real issue here. The warnings are annoying, but i don't think they are fatal. This -EBUSY is what is stopping the driver from loading, causing the real regression.
I'm guessing, but i think you will find the driver is loading once, but hits a EPROBE_DEFFER condition, after getting the gpio. It does not release the gpio correctly. Sometime later, it gets loaded again, but the gpio is now in use, so you get the -EBUSY.
So check the error paths, and make sure cleanup is being done correct. It could also be a phylib core bug...
Andrew
linux-stable-mirror@lists.linaro.org