From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brad Mouring Subject: Re: [PATCH] net: macb: Fix regression breaking non-MDIO fixed-link PHYs Date: Tue, 14 Aug 2018 21:03:52 -0500 Message-ID: <20180815020352.GA105279@artie> References: <20180814141240.9085-1-a.fatoum@pengutronix.de> <20180814155812.cjuacvkhqeuctcry@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8bit Cc: Ahmad Fatoum , "David S. Miller" , Nicolas Ferre , , , , , Andrew Lunn , Florian Fainelli To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Return-path: Received: from mx0a-00010702.pphosted.com ([148.163.156.75]:34278 "EHLO mx0b-00010702.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726000AbeHOEyH (ORCPT ); Wed, 15 Aug 2018 00:54:07 -0400 Content-Disposition: inline In-Reply-To: <20180814155812.cjuacvkhqeuctcry@pengutronix.de> Sender: netdev-owner@vger.kernel.org List-ID: Hello Ahmed, Uwe, On Tue, Aug 14, 2018 at 05:58:12PM +0200, Uwe Kleine-König wrote: > Hello Ahmad, > > > On Tue, Aug 14, 2018 at 04:12:40PM +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 > > 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: > > Fixes: 739de9a1563a ("net: macb: Reorganize macb_mii bringup") > > I added the people involved in 739de9a1563a to Cc. Maybe they want to > comment. So not stripping the remaining part of the original mail. > > Best regards > Uwe You should probably prod Andrew Lunn, he suggested that I move the fixed link code from macb_mii_init() to _probe(). Here, you're at least partially directly undoing that. (ref: https://www.mail-archive.com/netdev@vger.kernel.org/msg221018.html) > > Signed-off-by: Ahmad Fatoum > > --- > > drivers/net/ethernet/cadence/macb_main.c | 26 +++++++++++++++--------- > > 1 file changed, 16 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > > index a6c911bb5ce2..d202a03c42ed 100644 > > --- a/drivers/net/ethernet/cadence/macb_main.c > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > @@ -481,11 +481,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); > > @@ -568,7 +563,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)); > > @@ -591,10 +586,21 @@ 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; > > > > @@ -606,9 +612,9 @@ static int macb_mii_init(struct macb *bp) > > > > err_out_unregister_bus: > > mdiobus_unregister(bp->mii_bus); > > +err_out_free_mdiobus: > > if (np && of_phy_is_fixed_link(np)) > > of_phy_deregister_fixed_link(np); > > -err_out_free_mdiobus: > > of_node_put(bp->phy_node); > > mdiobus_free(bp->mii_bus); > > err_out: > > -- > > 2.18.0 > > > > > > > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://urldefense.proofpoint.com/v2/url?u=http-3A__www.pengutronix.de_&d=DwIDAw&c=I_0YwoKy7z5LMTVdyO6YCiE2uzI1jjZZuIPelcSjixA&r=8iZb7YNOSMVIG_mTIHDL03ZcObgQI_gGlWrSewdGETA&m=IQAK1YsKs7Z2bvZxuajiSXw3asFiEztQKYkvy-LpBn8&s=XKrOhFxQshoEcDxMZVSATnJW2cbaD16mQofKdEbJVW0&e= | -- Brad Mouring Senior Software Engineer National Instruments 512-683-6610 / bmouring@ni.com