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

[Giuseppe CAVALLARO <peppe.cavallaro@st.com>]

Hello Vincent

On 6/10/2016 1:00 AM, Vincent Palatin wrote:
> On Wed, Jun 8, 2016 at 5:17 PM, Andrew Lunn <andrew@lunn.ch> 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@st.com> 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