diff mbox series

wifi: mwifiex: enable host mlme on sdio W8997 chipsets

Message ID 20250521101950.1220793-1-rafael@beims.me
State Superseded
Headers show
Series wifi: mwifiex: enable host mlme on sdio W8997 chipsets | expand

Commit Message

Rafael Beims May 21, 2025, 10:19 a.m. UTC
From: Rafael Beims <rafael.beims@toradex.com>

Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
This feature depends on firmware support (V2 API key), which may not be
available in all available firmwares.

Signed-off-by: Rafael Beims <rafael.beims@toradex.com>
---
Tested on a Verdin iMX8MP (SDIO-UART) and Verdin iMX8MM (SDIO-SDIO):
    - created AP's requiring WPA2 and WPA3 authentication and successfully
      connected clients to them.
    - connected as a client to AP's requiring WPA2 and WPA3 authentication
---
 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Brian Norris May 22, 2025, 5:24 p.m. UTC | #1
On Wed, May 21, 2025 at 07:19:34AM -0300, rafael@beims.me wrote:
> From: Rafael Beims <rafael.beims@toradex.com>
> 
> Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
> This feature depends on firmware support (V2 API key), which may not be
> available in all available firmwares.

Is it available in *any* W8997 firmware? Or particularly, is it
available in the firmware in linux-firmware.git? Judging by its git
history, the answer is "no", in which case this is definitely NAK'd.

At a minimum, can you post what FW version you're testing with? This is
the bare minimum required for this kind of patch on many other drivers,
although I'm not sure we've been so strict on mwifiex.

And even if it works for *some* firmware, I'd generally highly encourage
not making breaking changes like this, where there's a long history of
users on the old FW version. So, we'd need to teach the driver to know
the difference between v1 and v2 API here, and choose accordingly.

Brian

> Signed-off-by: Rafael Beims <rafael.beims@toradex.com>
> ---
> Tested on a Verdin iMX8MP (SDIO-UART) and Verdin iMX8MM (SDIO-SDIO):
>     - created AP's requiring WPA2 and WPA3 authentication and successfully
>       connected clients to them.
>     - connected as a client to AP's requiring WPA2 and WPA3 authentication
> ---
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index c1fe48448839..f039d6f19183 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -438,7 +438,7 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
>  	.can_auto_tdls = false,
>  	.can_ext_scan = true,
>  	.fw_ready_extra_delay = false,
> -	.host_mlme = false,
> +	.host_mlme = true,
>  };
>  
>  static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
> -- 
> 2.47.2
>
Francesco Dolcini May 23, 2025, 7:26 a.m. UTC | #2
Hello Brian,

On Thu, May 22, 2025 at 10:24:14AM -0700, Brian Norris wrote:
> On Wed, May 21, 2025 at 07:19:34AM -0300, rafael@beims.me wrote:
> > From: Rafael Beims <rafael.beims@toradex.com>
> > 
> > Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
> > This feature depends on firmware support (V2 API key), which may not be
> > available in all available firmwares.
> 
> Is it available in *any* W8997 firmware? Or particularly, is it
> available in the firmware in linux-firmware.git? Judging by its git
> history, the answer is "no", in which case this is definitely NAK'd.

mrvl/sdsd8997_combo_v4.bin, from linux-firmware GIT, 16.92.21.p137
version. From an off-list chat with Rafael he confirmed me that this is
what he used for testing.

> users on the old FW version. So, we'd need to teach the driver to know
> the difference between v1 and v2 API here, and choose accordingly.

This is already implemented. From mwifiex_ret_get_hw_spec()

```
        if (adapter->key_api_major_ver != KEY_API_VER_MAJOR_V2)
                adapter->host_mlme_enabled = false;
```

To me the patch is ok.

Francesco
Francesco Dolcini May 28, 2025, 10:48 a.m. UTC | #3
On Wed, May 21, 2025 at 07:19:34AM -0300, rafael@beims.me wrote:
> From: Rafael Beims <rafael.beims@toradex.com>
> 
> Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
> This feature depends on firmware support (V2 API key), which may not be
> available in all available firmwares.
> 
> Signed-off-by: Rafael Beims <rafael.beims@toradex.com>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Brian Norris May 28, 2025, 5:05 p.m. UTC | #4
Hi Francesco,

On Fri, May 23, 2025 at 09:26:04AM +0200, Francesco Dolcini wrote:
> On Thu, May 22, 2025 at 10:24:14AM -0700, Brian Norris wrote:
> > On Wed, May 21, 2025 at 07:19:34AM -0300, rafael@beims.me wrote:
> > > From: Rafael Beims <rafael.beims@toradex.com>
> > > 
> > > Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
> > > This feature depends on firmware support (V2 API key), which may not be
> > > available in all available firmwares.
> > 
> > Is it available in *any* W8997 firmware? Or particularly, is it
> > available in the firmware in linux-firmware.git? Judging by its git
> > history, the answer is "no", in which case this is definitely NAK'd.
> 
> mrvl/sdsd8997_combo_v4.bin, from linux-firmware GIT, 16.92.21.p137
> version. From an off-list chat with Rafael he confirmed me that this is
> what he used for testing.

linux-firmware.git claims to hold W16.68.1.p197.1. Either WHENCE is
wrong, or that sounds like a completely different branch. Are you sure
about that?

> > users on the old FW version. So, we'd need to teach the driver to know
> > the difference between v1 and v2 API here, and choose accordingly.
> 
> This is already implemented. From mwifiex_ret_get_hw_spec()
> 
> ```
>         if (adapter->key_api_major_ver != KEY_API_VER_MAJOR_V2)
>                 adapter->host_mlme_enabled = false;
> ```


Huh, I forgot about that. I also assumed "v2 API" wasn't something
available on most older chips, but I guess I might have been wrong.

> To me the patch is ok.

The firmware versions above don't match up to me. But if we confirm
that:
(a) this API is available in a linux-firmware.git firmware for this chip
    and
(b) someone includes a valid linux-firmware.git version that they tested
    in the changelog

then maybe this is OK. As it stands now though, no.

Brian
Francesco Dolcini May 29, 2025, 7:20 a.m. UTC | #5
Hello Brian,

On Wed, May 28, 2025 at 10:05:03AM -0700, Brian Norris wrote:
> On Fri, May 23, 2025 at 09:26:04AM +0200, Francesco Dolcini wrote:
> > On Thu, May 22, 2025 at 10:24:14AM -0700, Brian Norris wrote:
> > > On Wed, May 21, 2025 at 07:19:34AM -0300, rafael@beims.me wrote:
> > > > From: Rafael Beims <rafael.beims@toradex.com>
> > > > 
> > > > Enable the host mlme flag for W8997 chipsets so WPA3 can be used.
> > > > This feature depends on firmware support (V2 API key), which may not be
> > > > available in all available firmwares.
> > > 
> > > Is it available in *any* W8997 firmware? Or particularly, is it
> > > available in the firmware in linux-firmware.git? Judging by its git
> > > history, the answer is "no", in which case this is definitely NAK'd.
> > 
> > mrvl/sdsd8997_combo_v4.bin, from linux-firmware GIT, 16.92.21.p137
> > version. From an off-list chat with Rafael he confirmed me that this is
> > what he used for testing.
> 
> linux-firmware.git claims to hold W16.68.1.p197.1. Either WHENCE is
> wrong, or that sounds like a completely different branch. Are you sure
> about that?

Nope, I did a mistake :-). Wrong copy/paste.

[    5.956510] mwifiex_sdio mmc2:0001:1: info: FW download over, size 623352 bytes
...
[    6.825456] mwifiex_sdio mmc2:0001:1: WLAN FW is active
...
[   12.171950] mwifiex_sdio mmc2:0001:1: host_mlme: enable, key_api: 2
[   12.226206] mwifiex_sdio mmc2:0001:1: info: MWIFIEX VERSION: mwifiex 1.0 (16.68.1.p197)

root@verdin-imx8mm-14700070:~# strings /lib/firmware/mrvl/sdsd8997_combo_v4.bin |grep 16
$Id: w8997o-V4, RF878X, FP68_LINUX, 16.68.1.p197.1 $

f62026ad508304fdadd9ebb5b500a6b4  /lib/firmware/mrvl/sdsd8997_combo_v4.bin

from commit 211fbc287a0b ("linux-firmware: Update FW files for MRVL SD8997 chips")

> The firmware versions above don't match up to me. But if we confirm
> that:
> (a) this API is available in a linux-firmware.git firmware for this chip
>     and
> (b) someone includes a valid linux-firmware.git version that they tested
>     in the changelog
> 
> then maybe this is OK. As it stands now though, no.

Right, my fault, now it should be clarified. Rafael, can you prepare a
v2 patch with all this information properly written in the commit message?

Francesco
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index c1fe48448839..f039d6f19183 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -438,7 +438,7 @@  static const struct mwifiex_sdio_device mwifiex_sdio_sd8997 = {
 	.can_auto_tdls = false,
 	.can_ext_scan = true,
 	.fw_ready_extra_delay = false,
-	.host_mlme = false,
+	.host_mlme = true,
 };
 
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {