Re: [PATCH net-next v6 5/9] net: dsa: microchip: ksz9477: Add Wake on Magic Packet support

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Thu, Oct 19, 2023 at 08:29:53PM +0300, Vladimir Oltean wrote:
> On Thu, Oct 19, 2023 at 02:28:46PM +0200, Oleksij Rempel wrote:
....
> > @@ -109,10 +110,22 @@ void ksz9477_get_wol(struct ksz_device *dev, int port,
> >  
> >  	wol->supported = WAKE_PHY;
> >  
> > +	/* Check if at this moment we would be able to get the MAC address
> > +	 * and use it for WAKE_MAGIC support. This result may change dynamically
> > +	 * depending on configuration of other ports.
> > +	 */
> > +	ret = ksz_switch_macaddr_get(dev->ds, port, NULL);
> > +	if (!ret) {
> > +		wol->supported |= WAKE_MAGIC;
> > +		ksz_switch_macaddr_put(dev->ds);
> 
> I don't get it, why do you release the reference on the MAC address as
> soon as you successfully get it? Without a reference held, the
> programmed address still lingers on, but the HSR offload code, on a
> different port with a different MAC address, can change it and break WoL.

It is ksz9477_get_wol() function. We do not actually need to program
here the MAC address, we only need to test if we would be able to get
it. To show the use more or less correct information on WoL
capabilities. For example, instead showing the user that Wake on Magic
is supported, where we already know that is not the case, we can already
show correct information. May be it will be better to have
extra option for ksz_switch_macaddr_get() to not allocate and do the
refcounting or have a separate function.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |