Re: [PATCH v2] net: macb: do not scan PHYs manually

[Josh Cartwright]

On Fri, Apr 29, 2016 at 02:40:53PM +0200, Nicolas Ferre wrote:
[..]
> >  static int macb_mii_init(struct macb *bp)
> >  {
> >  	struct macb_platform_data *pdata;
> >  	struct device_node *np;
> > -	int err = -ENXIO, i;
> > +	int err = -ENXIO;
> >  
> >  	/* Enable management port */
> >  	macb_writel(bp, NCR, MACB_BIT(MPE));
> > @@ -446,33 +497,10 @@ static int macb_mii_init(struct macb *bp)
> >  	dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
> >  
> >  	np = bp->pdev->dev.of_node;
> > -	if (np) {
> > -		/* try dt phy registration */
> > -		err = of_mdiobus_register(bp->mii_bus, np);
> > -
> > -		/* fallback to standard phy registration if no phy were
> > -		 * found during dt phy registration
> > -		 */
> > -		if (!err && !phy_find_first(bp->mii_bus)) {
> > -			for (i = 0; i < PHY_MAX_ADDR; i++) {
> > -				struct phy_device *phydev;
> > -
> > -				phydev = mdiobus_scan(bp->mii_bus, i);
> > -				if (IS_ERR(phydev)) {
> > -					err = PTR_ERR(phydev);
> > -					break;
> > -				}
> > -			}
> > -
> > -			if (err)
> > -				goto err_out_unregister_bus;
> > -		}
> > -	} else {
> > -		if (pdata)
> > -			bp->mii_bus->phy_mask = pdata->phy_mask;
> > -
> > -		err = mdiobus_register(bp->mii_bus);
> > -	}
> > +	if (np)
> > +		err = macb_mii_of_init(bp, np);
> > +	else
> > +		err = macb_mii_pdata_init(bp, pdata);
> >  
> >  	if (err)
> >  		goto err_out_free_mdiobus;
> 
> I'm okay with this. Thanks for having taken the initiative to implement it.

Unfortunately, I don't think it's going to be as straightforward
as I originally thought.  Still doable, but more complicated.

In particular, the macb bindings allow for a user to specify a
'reset-gpios' property _at the PHY_ level, which is consumed by the
macb to adjust the PHY reset state on remove.

My question is: why is the PHY reset GPIO management not the
responsibility of the PHY driver/core itself?

  Josh

Attachment: signature.asc
Description: PGP signature