Re: [PATCH] net: stmmac: dwmac-rk: keep PHY up for WoL

[Giuseppe CAVALLARO]

Hello Vincent

On 6/10/2016 1:00 AM, Vincent Palatin wrote:
> On Wed, Jun 8, 2016 at 5:17 PM, Andrew Lunn <andrew@xxxxxxx> wrote:
> > On Wed, Jun 08, 2016 at 03:25:38PM -0700, Vincent Palatin wrote:
> > > On Tue, Jun 7, 2016 at 12:23 AM, Giuseppe CAVALLARO
> > > <peppe.cavallaro@xxxxxx> wrote:
> > > > Hello
> > > >
> > > > On 6/3/2016 7:29 PM, Vincent Palatin wrote:
> > > > > Do not shutdown the PHY if Wake-on-Lan is enabled, else it cannot wake
> > > > > us up.
> > > >
> > > > I do not understand why you need that.
> > > > This is done inside the PHY layer and it is tested on our platforms
> > > > he idea is: If the parent wants to Wake the system then the PHY should
> > > > not power-down.
> > >
> > > I'm not sure I understand :
> > > you mean that this path is not called if WoL is enabled ?
> > > [ currently stmmac_pltfr_suspend() is calling priv->plat->exit() which
> > > is the rk_gmac_exit() code I'm modifying ]
> > > or the RK driver code should not power down the phy in its exit() callback ?
> >
> > Take a look at phy_suspend().
>
> phy_suspend() sends (or not) the PowerDown command to the PHY through
> the MDIO bus,  depending if WoL is disabled,
> but most of my question still stands as far as I can tell :
> I was trying to get a proper WoL support on the following setup :
>   dwmac  (inside a RK3288 SoC) connected to RTL8211 PHY
> The current upstream code for this case will call rk_gmac_exit() when
> the MAC suspends (after the PHY has already suspended). Effectively
> doing a phy_power_on(, false) which is calling regulator_disable() on
> the LDO defined by the 'phy-supply' attribute.
> So my reading is that the RK specific MAC code is turning off
> unconditionally the PHY power regulator. Unless I'm mistaken, either
> this code is incorrect for the WoL case or the naming 'phy-supply' is
> misleading and should be the MAC supply.

ok now clear. And you are right. I can conclude that the patch is ok
for me. I just ask you to resend it elaborating a bit the subject and
surrounding the code with a comment.

I do not know your SoC but indeed, when doing WoL, some parts of the
MAC + PHY must be powered so IMO it is legal that you do not cut
the power by invoking regulator.

Peppe