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: <stable@xxxxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx/msg221018.html)
> > Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxx
