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: <sta...@vger.kernel.org>
> > 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 <a.fat...@pengutronix.de>
> > ---
> >  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 / bmour...@ni.com

Reply via email to