On Mon, Oct 16, 2023 at 08:37:50AM -0700, Florian Fainelli wrote: > [snip] > > +void ksz9477_get_wol(struct ksz_device *dev, int port, > > + struct ethtool_wolinfo *wol) > > +{ > > + u8 pme_ctrl, pme_conf; > > + int ret; > > + > > + ret = ksz_read8(dev, REG_SW_PME_CTRL, &pme_conf); > > + if (ret) > > + return; > > + > > + if (!(pme_conf & PME_ENABLE)) > > + return; > > I suppose this works beause you have separate enable bits for WOL_LINKUP, > WOL_ENERGY and WOL_MAGICPKT, you could have also left the setting of the > PME_ENABLE bit to the set_wol() routine provided that wol->wolopts is > non-zero. Setting the PME_ENABLE bit in the set_wol() function might imply that it should also be cleared within set_wol(), necessitating refcounting due to its global bit nature. As an alternative, the reading of the REG_SW_PME_CTRL register could be replaced by checking the wakeup_source variable, which might result in some optimization. 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 |