On Thu, Feb 01, 2024 at 12:11:15PM +0530, Vineeth Karumanchi wrote: > Hi Andrew, Krzysztof, > > > > On 31/01/24 6:48 pm, Andrew Lunn wrote: > > On Wed, Jan 31, 2024 at 01:09:19PM +0530, Vineeth Karumanchi wrote: > > > Hi Andrew, > > > > > > > > > On 31/01/24 6:56 am, Andrew Lunn wrote: > > > > On Tue, Jan 30, 2024 at 04:18:44PM +0530, Vineeth Karumanchi wrote: > > > > > "wol-arp-packet" property enables WOL with ARP packet. > > > > > It is an extension to "magic-packet for WOL. > > > > > > > > It not clear why this is needed. Is this not a standard feature of the > > > > IP? Is there no hardware bit indicating the capability? > > > > > > > > > > WOL via both ARP and Magic packet is supported by the IP version on ZU+ and > > > Versal. However, user can choose which type of packet to recognize as a WOL > > > event - magic packet or ARP. > > > > ethtool says: > > > > wol p|u|m|b|a|g|s|f|d... > > Sets Wake-on-LAN options. Not all devices support this. The argument to this option is a > > string of characters specifying which options to enable. > > p Wake on PHY activity > > u Wake on unicast messages > > m Wake on multicast messages > > b Wake on broadcast messages > > a Wake on ARP > > g Wake on MagicPacket™ > > s Enable SecureOn™ password for MagicPacket™ > > f Wake on filter(s) > > d Disable (wake on nothing). This option > > clears all previous options. > > > > So why do we need a DT property? > > > > The earlier implementation of WOL (magic-packet) was using DT property. > We added one more packet type using DT property to be in-line with the > earlier implementation. I can understand that. It also suggests we did a bad job reviewing that patch, and should of rejected it. But it was added a long time ago, and we were less strict back then. > > However, I echo with you that this feature should be in driver (CAPS). > We will re-work the implementation with the below flow: > > - Add MACB_CAPS_WOL capability to the supported platforms > - Advertise supported WOL packet types based on the CAPS in ethtool. > - Users can set packet type using ethtool. Yes, this sounds good. Maybe add to that, mark magic-packet deprecated, and a comment that ethtool should be used instead. Thanks Andrew