Received-SPF: pass (google.com: domain of linux-kernel+bounces-91792-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: David Lin <yu-hao.lin@nxp.com>
To: Francesco Dolcini <francesco@dolcini.it>
CC: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"briannorris@chromium.org" <briannorris@chromium.org>, "kvalo@kernel.org"
	<kvalo@kernel.org>, Pete Hsieh <tsung-hsien.hsieh@nxp.com>
Subject: RE: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client
 mode
Thread-Topic: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client
 mode
Thread-Index: AQHaNIYDXGT+ith2YUesnsIyMnIG77Ee4pYAgAI47lCACBeDgA==
Date: Tue, 5 Mar 2024 07:25:56 +0000
Message-ID:
 <PA4PR04MB9638983EBD1DB4D3E8FB5474D1222@PA4PR04MB9638.eurprd04.prod.outlook.com>
References: <20231222032123.1036277-1-yu-hao.lin@nxp.com>
 <20231222032123.1036277-2-yu-hao.lin@nxp.com>
 <20240227175306.GB11034@francesco-nb>
 <PA4PR04MB96383C684AAA40C14DDB4B79D15F2@PA4PR04MB9638.eurprd04.prod.outlook.com>
In-Reply-To:
 <PA4PR04MB96383C684AAA40C14DDB4B79D15F2@PA4PR04MB9638.eurprd04.prod.outlook.com>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?ljXJdNQnjhb2463+fmvthvJHp04lT4mNhKzLLrd3FfbroGETs/nHp2/cF3mU?=
 =?us-ascii?Q?9AkMpYiRukFakYh8ivvGVO7m3BKa2ry8VA1O/Vm5iZSIZTpKOAaup1PJFQ+i?=
 =?us-ascii?Q?FpwszqiyNTJCXCjsuDYJGlMwVSOP+qqmxYnBpK5JzsuzFAHGpUtSN8idI7Ql?=
 =?us-ascii?Q?G3AXR/SdItSqKZGZGJQPaNI+1fFu3mwEFUBa/sXb2Kl/ci1mElYkLG45wEGU?=
 =?us-ascii?Q?ndLsQVgTUk3U19U9qi2b7U4AAhEbnH5vyHr4iG0B8zuuIHL56ZYA3ZSgmwUE?=
 =?us-ascii?Q?HTHAo0GcAK7GymY/4qK4QOCtFvsS2xJYl8YFBBBDKuSs0TkIHhQdn93lZGGD?=
 =?us-ascii?Q?v+6SlCqoetxVEsNra4JVlqqydi7WqHEkSL8UTWa35lfLbAnajNLYHvylAH5Y?=
 =?us-ascii?Q?tb7gzNenxO86UQSKmQI2Yd5vfTaLmXCHdT9d55RCgG3h2JpH/ghkcDDLfDeo?=
 =?us-ascii?Q?8Aiiv8CIx97C/rNkMB/I5cmLY1qY59nGK5ioO46U3baxD1dUP/ye7SVqW9M0?=
 =?us-ascii?Q?brB4ZWdxvl/cJQ1K2JOe8bjwys0m41argioW5IdjUhF2ckjQqGF3VKUXm6Pa?=
 =?us-ascii?Q?/reuet8eHo6cEu9Pca2Qoj1qFLutI8GBwRUI0Z0YVTNQOZJ92Os3A9iDZu9s?=
 =?us-ascii?Q?CGiNUKtGk/onbj7sWN6sYTE/FrIAp1YVvDLoLOKOJLRCWPDeR15Kggs8hpHs?=
 =?us-ascii?Q?LH0PiMmLSZq6JYtohB+Gkkvsnh3/sZ8cExDv7haHqTVJIgKfSahFYr/A99fH?=
 =?us-ascii?Q?txA4pI0ljX10jnhiaQYEeJ3JkayxPWZ0dz7NfRaJdfZjzUZ4OOuZmcxFBGx/?=
 =?us-ascii?Q?/SVgooHsUbofvNiYffF9I9288jlm2GYyysIPKUMNy7ze2IFqotp++gfKS4TV?=
 =?us-ascii?Q?EAIdM9IGYCSVPy+XCiZKvQW5zR5QYE7LSqOdbpYQgFMeh/ylZGV2xkT+uzN+?=
 =?us-ascii?Q?tHXqtrbFPwviGWrAFTWdLSHQGSrTzieqXG5vcsNdc8yBLpzWOK99AfeJfNPo?=
 =?us-ascii?Q?q5OhLuhAdaxPhOnt+9WaRInapk1ST7lXfeZH6ZJFHzRPIXFb36W57xOwzBRy?=
 =?us-ascii?Q?7toKPfEK4+j8uaiO7TyvkwR7dCJGEkn+WIlvV1QDf4wQT1SSyN9SMbCzHMkq?=
 =?us-ascii?Q?TGz/11BKa9DF7mnHmYKFtdu9WB3OYoFePmt3iONfxbikEnuNm1flqBqYzCOg?=
 =?us-ascii?Q?ZfHq+M9rIHgyjCxweFui0aBt1FlLWbr7+Q/eStc4PzngVreCzqWg4sruvONy?=
 =?us-ascii?Q?+mdmfSaCnN4PcYuI0lbobXB6U2hp9B+j3PHvwUO0puCrS1WYjrEsNdzAf4GW?=
 =?us-ascii?Q?jSK/zFs00FC8UX3nEOV5NvpdlMuyXokomhJpf4y+glMl82R46EH7l7O9isyZ?=
 =?us-ascii?Q?OnJGxmoEIvUQ2AiMRPN6VyiKoksDTx3e/JS7X6GxUzdMqck/MykLj2ZRp0jj?=
 =?us-ascii?Q?5/C7yZyElq/VOxJfRVhLHP0W3VeywtvGuc/PoNRaMejaWWEAjewSc2Bg7dzu?=
 =?us-ascii?Q?DOnqP/P6kQwoY5TbFHH4oBn/J9hFEG4nEI5x3MT36cAJkB8YXguDTkmm+6Dx?=
 =?us-ascii?Q?ZyvUebkWEU3wMP7qwqylEKyeTgFRYRsL7LpYKlKI?=
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-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PA4PR04MB9638.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 54ebe539-c214-4455-cb5f-08dc3ce57fae
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Mar 2024 07:25:56.6749
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: oAVs6gfzK9NUjWD5DLeVBuXerMIBFuuVUZvdlPuo9qhNvFggSGTW4WoFgTlGwmoBXzoOTreOxZrnGjWRrNbXgQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GVXPR04MB10246

> From: David Lin
> Sent: Thursday, February 29, 2024 11:53 AM
> To: 'Francesco Dolcini' <francesco@dolcini.it>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> briannorris@chromium.org; kvalo@kernel.org; Pete Hsieh
> <tsung-hsien.hsieh@nxp.com>
> Subject: RE: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for cl=
ient
> mode
>=20
> > From: Francesco Dolcini <francesco@dolcini.it>
> > Sent: Wednesday, February 28, 2024 1:53 AM
> > To: David Lin <yu-hao.lin@nxp.com>
> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> > briannorris@chromium.org; kvalo@kernel.org; francesco@dolcini.it; Pete
> > Hsieh <tsung-hsien.hsieh@nxp.com>
> > Subject: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for
> > client mode
> >
> > Caution: This is an external email. Please take care when clicking
> > links or opening attachments. When in doubt, report the message using
> > the 'Report this email' button
> >
> >
> > On Fri, Dec 22, 2023 at 11:21:22AM +0800, David Lin wrote:
> > > Add host based MLME to enable WPA3 functionalities in client mode.
> > > This feature required a firmware with the corresponding V2 Key API
> > > support. The feature (WPA3) is currently enabled and verified only
> > > on IW416. Also, verified no regression with change when host MLME is
> > > disabled.
> > >
> > > Signed-off-by: David Lin <yu-hao.lin@nxp.com>
> >
> > Currently this do not apply cleanly on wireless-next/main git tree, it
> > should be rebased to that branch as v9 (the reason is your addition of
> > fw_ready_extra_delay from a previous, now merged, patch).
> >
> > In general the patch looks good to me, however I am no expert on
> > wireless driver or the related linux subsystem. I just have a couple
> > of small comments that I would suggest address in v9 (given that you
> > need to do it as a minimum to rebase your code).
> >
>=20
> Yes. Will rebase and create patch v9.
>=20
> >
> > > ---
> > >  .../net/wireless/marvell/mwifiex/cfg80211.c   | 315
> > ++++++++++++++++++
> > >  drivers/net/wireless/marvell/mwifiex/cmdevt.c |  25 ++
> > >  drivers/net/wireless/marvell/mwifiex/decl.h   |  22 ++
> > >  drivers/net/wireless/marvell/mwifiex/fw.h     |  33 ++
> > >  drivers/net/wireless/marvell/mwifiex/init.c   |   6 +
> > >  drivers/net/wireless/marvell/mwifiex/join.c   |  66 +++-
> > >  drivers/net/wireless/marvell/mwifiex/main.c   |  54 +++
> > >  drivers/net/wireless/marvell/mwifiex/main.h   |  17 +
> > >  drivers/net/wireless/marvell/mwifiex/scan.c   |   6 +
> > >  drivers/net/wireless/marvell/mwifiex/sdio.c   |  13 +
> > >  drivers/net/wireless/marvell/mwifiex/sdio.h   |   2 +
> > >  .../net/wireless/marvell/mwifiex/sta_event.c  |  36 +-
> > >  .../net/wireless/marvell/mwifiex/sta_ioctl.c  |   3 +-
> > >  drivers/net/wireless/marvell/mwifiex/sta_tx.c |   9 +-
> > >  drivers/net/wireless/marvell/mwifiex/util.c   |  80 +++++
> > >  15 files changed, 673 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > index 7a15ea8072e6..3cee1b58465e 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> > > @@ -4202,6 +4208,302 @@ mwifiex_cfg80211_change_station(struct
> > wiphy
> > > *wiphy, struct net_device *dev,
> >
> > ...
> >
> > > +static int
> > > +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> > > +                           struct net_device *dev, const u8 *peer,
> > > +                           u64 *cookie) {
> > > +     return -1;
> >
> > See my following comment on this
> >
> > > +}
> > > +
> > >  /* station cfg80211 operations */
> > >  static struct cfg80211_ops mwifiex_cfg80211_ops =3D {
> > >       .add_virtual_intf =3D mwifiex_add_virtual_intf, @@ -4347,6
> > > +4649,16 @@ int mwifiex_register_cfg80211(struct mwifiex_adapter
> > *adapter)
> > >                           "%s: creating new wiphy\n", __func__);
> > >               return -ENOMEM;
> > >       }
> > > +     if (adapter->host_mlme_enabled) {
> > > +             mwifiex_cfg80211_ops.auth =3D
> > mwifiex_cfg80211_authenticate;
> > > +             mwifiex_cfg80211_ops.assoc =3D
> > mwifiex_cfg80211_associate;
> > > +             mwifiex_cfg80211_ops.deauth =3D
> > mwifiex_cfg80211_deauthenticate;
> > > +             mwifiex_cfg80211_ops.disassoc =3D
> > mwifiex_cfg80211_disassociate;
> > > +             mwifiex_cfg80211_ops.disconnect =3D NULL;
> > > +             mwifiex_cfg80211_ops.connect =3D NULL;
> > > +             mwifiex_cfg80211_ops.probe_client =3D
> > > +                     mwifiex_cfg80211_probe_client;
> >
> > Can you omit this one? You should get `-EOPNOTSUPP` for free with
> > probe_client set to NULL. Am I wrong?
> >
>=20
> Yes. You are right. Remove in patch v9.
>=20

This function must be hooked, otherwise AP mode can't work. I will hook thi=
s function as before but return -EOPNOTSUPP instead.
Patch v9 is almost done and will be submitted later. Thanks for your sugges=
tion.

> > > +     }
> > >       wiphy->max_scan_ssids =3D MWIFIEX_MAX_SSID_LIST_LENGTH;
> > >       wiphy->max_scan_ie_len =3D MWIFIEX_MAX_VSIE_LEN;
> > >       wiphy->mgmt_stypes =3D mwifiex_mgmt_stypes;
> >
> > ...
> >
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> > > b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> > > index a2ad2b53f016..23639aacf092 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
> > > @@ -136,6 +136,7 @@ int mwifiex_fill_new_bss_desc(struct
> > mwifiex_private *priv,
> > >       const struct cfg80211_bss_ies *ies;
> > >
> > >       rcu_read_lock();
> > > +     bss_desc->bss =3D bss;
> > what is this change for? I was not able to understand it, nor to find
> > any user of this bss parameter. This looks like an unrelated fix, but
> > no code seems to be affected.
> >
> Yes. It will be removed in patch v9.
>=20
> >
> > With these 2 comments addressed, please feel free to add to v9
> >
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> >
> >
> > And thanks for this work!
> > Francesco