Received-SPF: pass (google.com: domain of linux-kernel+bounces-82571-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 27 Feb 2024 02:58:10 +0100
From: Andrew Lunn <andrew@lunn.ch>
To: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org,
	linux-kernel@vger.kernel.org, bryan.whitehead@microchip.com,
	richardcochran@gmail.com, UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net 3/3] net: lan743x: Address problems with wake option
 flags configuration sequences
Message-ID: <b83b74b7-3221-4747-8b71-17738c18c042@lunn.ch>
References: <20240226080934.46003-1-Raju.Lakkaraju@microchip.com>
 <20240226080934.46003-4-Raju.Lakkaraju@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240226080934.46003-4-Raju.Lakkaraju@microchip.com>

On Mon, Feb 26, 2024 at 01:39:34PM +0530, Raju Lakkaraju wrote:
> Wake options handling has been reworked as follows:
> a. We only enable secure on magic packet when both secure and magic wol
>    options are requested together.
> b. If secure-on magic packet had been previously enabled, and a subsequent
>    command does not include it, we add it. This was done to workaround a
>    problem with the 'pm-suspend' application which is unaware of secure-on
>    magic packet being enabled and can unintentionally disable it prior to
>    putting the system into suspend.

This seems to be in a bit of a grey area. But 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 speci‐
                  fying 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.

d clears everything. All other things enable one option. There does
not appear to be a way to disable a single option, and i would say,
adding options is incremental.

So:

> a. We only enable secure on magic packet when both secure and magic wol
>    options are requested together.

I don't think they need to be requested together. I think you can
first enable Wake on MagicPacket and then in a second call to ethtool
Enable SecureOn password for MagicPacket. I also don't think it would
unreasonable to accept Enable SecureOn password for MagicPacket and
have that imply Wake on MagicPacket.

And:

> b. If secure-on magic packet had been previously enabled, and a subsequent
>    command does not include it, we add it.

If there has not been a d, secure-on magic packet should remain
enabled until there is a d.

	Andrew