Re: [PATCH] net: ethernet: fsl: don't en/disable refclk on open/close

[Richard Leitner]

On 10/23/2017 12:48 AM, Florian Fainelli wrote> On 10/22/2017 03:30 PM, 
Richard Leitner wrote:
> > On 10/22/2017 08:31 PM, Florian Fainelli wrote:
> > > On 10/22/2017 06:11 AM, Richard Leitner wrote:

...

> > Andrew Lunn suggested to make the PHY driver a clock driver and let it
> > export the refclk... But IMHO that "feels" a bit strange. Due to the
> > fact in our case the clock is generated by the SoC and the PHY is an
> > external chip which only consumes it.
>
> It depends which *MII mode you use. Unless you are using reverse MII,
> the PHY supplies the RX clock and the MAC supplies the RX clock, so
> solving this for all modes becomes quite tricky. Andrew's suggestion
> makes sense, but considering that the MAC and PHY need to be connected
> to each other, it may be tricky to request the PHY's clock before you
> actually attached to the PHY...
> > But back to this patch: Is it OK the way it fixes the issue?
>
> Fugang and Andrew probably know this hardware a lot better, I would have
> to look at the code path a bit more to understand if an alternative
> solution is possible. It sounds like your patch could create a power
> consumption regression, so maybe add a check for the PHY ID that is
> problematic by doing something like:
>
> if (priv->phydev->drv && priv->phydev->drv->phy_id == XXXX_XXXX) where
> XXXX_XXXX is the LAN870 PHY ID (obtained from MII register 2 & 3).

I already had a patch which does this check and triggers a reset of the 
PHY after the refclk was enabled (in fec_enet_clk_enable) in case the 
phy_id matches. This would IMHO avoid all power consumption 
regressions... It was something like:

switch (priv->phydev->drv->phy_id) {
case XXXX:
case XXXX:
	ret = fec_reset_phy(ndev);
	if (ret)
		goto failed_reset;
	if (ndev->phydev) {
		ret = phy_init_hw(ndev->phydev);
		if (ret)
			goto failed_reset;
	}
}

Would that be better?

Basically I'm fine with any kind of solution... It's up to you guys... 
You're the experts ;-)