Re: [PATCH V3 net] net: stmmac: fix MAC WoL unwork if PHY doesn't support WoL

[Florian Fainelli <f.fainelli@gmail.com>]

On 4/28/2021 5:26 AM, Andrew Lunn wrote:
>>  static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>>  {
>>  	struct stmmac_priv *priv = netdev_priv(dev);
>> -	u32 support = WAKE_MAGIC | WAKE_UCAST;
>> +	struct ethtool_wolinfo wol_phy = { .cmd = ETHTOOL_GWOL };
>> +	u32 support = WAKE_MAGIC | WAKE_UCAST | WAKE_MAGICSECURE | WAKE_BCAST;
> 
> Reverse christmass tree please.
> 
>>  
>> -	if (!device_can_wakeup(priv->device))
>> -		return -EOPNOTSUPP;
>> +	if (wol->wolopts & ~support)
>> +		return -EINVAL;
> 
> Maybe -EOPNOTSUPP would be better.
> 
>>  
>> -	if (!priv->plat->pmt) {
>> +	/* First check if can WoL from PHY */
>> +	phylink_ethtool_get_wol(priv->phylink, &wol_phy);
> 
> This could return an error. In which case, you probably should not
> trust wol_phy.
> 
>> +	if (wol->wolopts & wol_phy.supported) {
> 
> This returns true if the PHY supports one or more of the requested WoL
> sources.
> 
>>  		int ret = phylink_ethtool_set_wol(priv->phylink, wol);
> 
> and here you request the PHY to enable all the requested WoL
> sources. If it only supports a subset, it is likely to return
> -EOPNOTSUPP, or -EINVAL, and do nothing. So here you only want to
> enable those sources the PHY actually supports. And let the MAC
> implement the rest.

And when your resubmit, I do not believe that unwork is a word, you
could provide the following subject:

net: stmmac: Fix MAC WoL not working if PHY does not support WoL

or something like that.

Thanks!
-- 
Florian