Received-SPF: pass (google.com: domain of linux-kernel+bounces-96641-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: <Raju.Lakkaraju@microchip.com>
To: <andrew@lunn.ch>
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
Thread-Topic: [PATCH net 3/3] net: lan743x: Address problems with wake option
 flags configuration sequences
Thread-Index: AQHaaIt96K5hsZSvW06kwmptrB0k3LErcdiAgAIeqGA=
Date: Fri, 8 Mar 2024 08:20:51 +0000
Message-ID: <LV8PR11MB87008454A629EE15B9CE14099F272@LV8PR11MB8700.namprd11.prod.outlook.com>
References: <20240226080934.46003-1-Raju.Lakkaraju@microchip.com>
 <20240226080934.46003-4-Raju.Lakkaraju@microchip.com>
 <78d7e538-9fa0-490e-bcfb-0a5943ad80c9@lunn.ch>
In-Reply-To: <78d7e538-9fa0-490e-bcfb-0a5943ad80c9@lunn.ch>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?cMhQVJhbBD60p8KuaK6oLuSnH0MK1Id4Ex394pL9CRV8yZo/nu5fbr8pNHwG?=
 =?us-ascii?Q?Fq5GLVl4nVQzB53jfkvIB0jTlMUPwVQvA9WAS6DXKGzjVKZ3H9IQK8I824e7?=
 =?us-ascii?Q?+KDZI9Q7Gbe8RCpYdETEt2sqP7GUmOZQHplblFkIDiTimMyx9aCA71Gk7u0f?=
 =?us-ascii?Q?tQDZKRSwGCT5W5o4d3qCectwhtEtm44BcV8TDAcnZ8AFREwkpijTrv77X9hF?=
 =?us-ascii?Q?K8w+/6lIngjOD7fDh6brwlGiF+GkvwA5NsPhfalcM83M9mQn/5k9qcP/UUUD?=
 =?us-ascii?Q?gWewYjQCs1sANv0elLz8D681URhIIinOUZoBc1lc4RT0qj6XPVLrdh/3xMwU?=
 =?us-ascii?Q?L4FhIPsBa62ysr8YIOp5YYf3MSgWdpzOpSBBif672vYs1GM5JZdel1dl65VG?=
 =?us-ascii?Q?ZCEUlBXkaZCiG504fagreYx61UQIMq8IntmUv88p7zoSiwI4PqtzquMMkzFX?=
 =?us-ascii?Q?EuvezGLi25MIWDuHm6slsm6CPkRdAKsAttALxOkg3nWARXMRLEkF4XzmXMWc?=
 =?us-ascii?Q?3TdFgvnNhGZyxY3iCvSZhD7Ubgh2KYLyTSbwmOUl40XvBwWsn9rMiaw2K9yG?=
 =?us-ascii?Q?7mO8Vsr5ubJSP7WX/gsIbl17w10vnLMlZJtOyYM++fe6xubDFFpTYTsvo7zF?=
 =?us-ascii?Q?StklFXN91r9D7+zcV5ju1N0QvXP2QrQcHfU4XXAfzdmZbHXFjJBOAoU1FXpL?=
 =?us-ascii?Q?AYu/UoXr9zFD4m+y3rmVGF0/Xeb+E22Jtpr2Ug+sabd/eMmg4SnVlyR8JH4G?=
 =?us-ascii?Q?uiJjv2eFtA/SQ3GHT6Lig5htVNEqdTq9wTZVTf2OuIXHmr9zMTG9Wkg342LK?=
 =?us-ascii?Q?mHk4RyBYAxfGBo+ygLjr0LcMTnu6S3NxcSg2zam6yQx/Ae2TaZPjpIQ3FB4f?=
 =?us-ascii?Q?TIXkQ78XNT3hpKYXfgVqUqBoHbXpPjzv2FMtfpGItrDd/IoBmQE6aTAbVEdf?=
 =?us-ascii?Q?+Aflwin69KxUA6q5QhSBRv1ip5KHnYOL8IGuIgee13R4SoA7Tym31i/Zn2o5?=
 =?us-ascii?Q?G7uZ6mVGiSaEP8Y189e3jbc7VONIB0HEnJcN9ptENoa3mInkZ8TKNRaCxs+0?=
 =?us-ascii?Q?QlRylkWbWz/xsYVAZa8zytB+GjizLwhC6X6BcMJhZZ1RWoIyrE54QNGD8lFh?=
 =?us-ascii?Q?LJdgcZ3sX/5ahkf6a/RX3NAAUGtc6zke+Qo5NH+47lg5A0t94vOYLwDGqEe1?=
 =?us-ascii?Q?FLMRtgoeIrhK3eCLoa67x2jEOlE6tDnD+98MuOLNJ1I/Ee+X2mScFP7dUhZS?=
 =?us-ascii?Q?gROt19ABXxsbozJBTYV5vaS2VwCO027eRE+d2Jm1HXLf2Qn/g88OA0QWmYrt?=
 =?us-ascii?Q?y0l5On00Y5xb1MfYS78ipzXWWYBW2r0+1PBUw1UhRIs/vHGZZUhg6AaVoXvQ?=
 =?us-ascii?Q?l8g/ciYjXz1YupW5ETFkc5qHALtmojXQzflCjFyY4ENgTI6kDI7m+Jp+0okq?=
 =?us-ascii?Q?mOIxvCsRHwM7WKgbgIy5ef5odoUxfBgQFe83mi2P1sziNDfDDOBBkHj49mwA?=
 =?us-ascii?Q?3oMEgHOdh+8z1hCjCvVsesnxEj7vCOvePw3MPibgBRvekX9mPIsXAWIPqnH9?=
 =?us-ascii?Q?aqNPfnvMDpzVWKLWu/lzFM1Ao+XSrxiw/p3QtlY2WgvrN0xyzjGz0PCqz9jb?=
 =?us-ascii?Q?4Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: LV8PR11MB8700.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7b69500e-7218-4633-2fce-08dc3f48aad6
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Mar 2024 08:20:51.5611
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: kc0MI4GE6KoLy4basn4uoSnMQ33SmTZvknixkX0g6JnhQ3DKZquiUKZjY8KNPHZK4Z4/8QFn9njNw3UnvXhIP4zzxpMaxzpWDZjv82WJELs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7213

Hi Andrew,

Thank you for valuable information.

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, March 7, 2024 5:23 AM
> To: Raju Lakkaraju - I30499 <Raju.Lakkaraju@microchip.com>
> Cc: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org; linux-
> kernel@vger.kernel.org; Bryan Whitehead - C21958
> <Bryan.Whitehead@microchip.com>; richardcochran@gmail.com;
> UNGLinuxDriver <UNGLinuxDriver@microchip.com>
> Subject: Re: [PATCH net 3/3] net: lan743x: Address problems with wake opt=
ion
> flags configuration sequences
>=20
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
>=20
> 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.
>=20
> So it appears unclear what should happen here.
>=20
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/bcm-phy-
> lib.c#L909
>=20
> WAKE_MAGICSECURE is a standalone option. You do not need WAKE_MAGIC.
> And even i you did request both WAKE_MAGIC and WAKE_MAGICSECURE,
> the WAKE_MAGIC would be ignored.
>=20
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/dp83822.c#=
L153
>=20
> WAKE_MAGICSECURE is a standalone option. You do not need WAKE_MAGIC.
> However, unlike the broadcom device, you can have both WAKE_MAGIC and
> WAKE_MAGICSECURE at the same time. They are not mutually exclusive.
>=20
> This also looks to be true for other dp8**** devices.
>=20
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/mscc/mscc_=
mai
> n.c#L318
>=20
> WAKE_MAGICSECURE is a standalone option. You do not need WAKE_MAGIC.
> Also, you can have both WAKE_MAGIC and WAKE_MAGICSECURE at the same
> time. They are not mutually exclusive.
>=20
> So i think your point a. above is questionable. Can the hardware support =
both
> magic and secure magic at the same time? If so, follow the TI way of doin=
g it.

Yes. I will do.

> If you cannot do both at the same time, and that is requested, you should
> probably return -EOPNOTSUPP. That is probably better than what the
> broadcom driver does, silently ignore WAKE_MAGIC.
>=20
> > 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.
>=20

Ok. I will try to fix in 'pm-suspend' application

> The kernel should not be working around broken userspace. But i also
> suspect this is to do with it being unclear if WOL options are incrementa=
l or
> not. Since it seems that they are not incremental, it does not matter if =
"If
> secure-on magic packet had been previously enable". pm-suspend is setting
> Wol how it wants it, which you say is plain magic. So magic is what the P=
HY
> driver should do. Feel free to submit patches to pm-suspend to make it
> understand secure magic, or not touch WoL at all with the assumption it h=
as
> already been setup by something else.
>=20
>           Andrew

Thanks,
Raju