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

Josh Cartwright
2016-05-02T18:51:14Z

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]