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

[Florian Fainelli]

On 02/05/16 11:36, Josh Cartwright wrote:
> 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.

In fact, not just on remove, anytime there is an opportunity to save
power (interface down, closed) and putting the PHY into reset is usually
guaranteed to be saving more power than e.g: a BMCR power down.

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

Well, this is actually being worked on at the moment by Sergei, since
there is not necessarily a reason why PHYLIB can't deal with that:

https://lkml.org/lkml/2016/4/28/831
-- 
Florian