Re: [PATCH net-next] net: phy: dp8382x: keep WOL setting across suspends

[POPESCU Catalin]

Hi,

On 08/03/2024 17:50, POPESCU Catalin wrote:
> On 07.03.24 09:30, POPESCU Catalin wrote:
>> On 07.03.24 00:04, Andrew Lunn wrote:
>>> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>>>
>>>
>>> On Wed, Mar 06, 2024 at 06:14:46PM +0100, Catalin Popescu wrote:
>>>> Unlike other ethernet PHYs from TI, PHY dp83822x has WOL enabled
>>>> at reset.
>>> This is rather odd behaviour. Is this stated in the datasheet?
>> Yes. I've checked all TI ethernet PHYs datasheets that are supported by
>> linux and they all, but dp8382x, have WOL disabled by default. Hence,
>> dp83822.c is the only driver that forcefully disable WOL at init.
>>>> @@ -572,11 +584,17 @@ static int dp83826_config_init(struct phy_device *phydev)
>>>>                          return ret;
>>>>          }
>>>>
>>>> +     if (dp83822->wol_enabled)
>>>> +             return 0;
>>>>          return dp8382x_disable_wol(phydev);
>>>>     }
>>>>
>>>>     static int dp8382x_config_init(struct phy_device *phydev)
>>>>     {
>>>> +     struct dp83822_private *dp83822 = phydev->priv;
>>>> +
>>>> +     if (dp83822->wol_enabled)
>>>> +             return 0;
>>>>          return dp8382x_disable_wol(phydev);
>>> Since it is rather odd behaviour, there might be some BIOSes which
>>> disable WoL. So i would not rely on it being enabled by
>>> default. Explicitly enable it.
>> I see, I'll make the change.
> It looks like the issue I'm trying to address in this patch is not
> specific to dp8382x. Right now, depending on if the PHY is reset or not
> during resume (either through mdio_device reset_gpio/reset_ctrl or
> phy_driver soft_reset callback), the WOL configuration is either the PHY
> reset value or the BIOS value. I could still make the patch but it
> doesn't really make sense to address only dp8382x.
>
> Also, I'm a bit confused as I'm not sure if this issue is already
> addressed by userspace or not (e.g. udevd that would reapply WOL
> configuration after suspend).
>
> If issue should be definitely addressed in the kernel instead of
> userspace, then PAL should enforce WOL configuration for any PHY by
> calling set_wol callback after soft_reset (probably, at the end of
> phy_init_hw or after phy_resume).
Does it make sense to address it in the kernel ?
>
>>>        Andrew
>>>
>>> ---
>>> pw-bot: cr