diff mbox series

wifi: mwifiex: Restore USB8897 chipset support

Message ID 20231205210237.209332-1-knaerzche@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: mwifiex: Restore USB8897 chipset support | expand

Commit Message

Alex Bee Dec. 5, 2023, 9:02 p.m. UTC
This patch restores USB8897 support which was removed with
Commit 60a188a2715f ("mwifiex: remove USB8897 chipset support")

There are quite some devices which use this chipset with USB interface.
The firmware still exits in linux upstream firmware repo and this simple
patch is all what is required to support it in upstream linux (again).

Signed-off-by: Alex Bee <knaerzche@gmail.com>
---
Recently I upstreamed support for Geniatec XPI-3128 SBC which actually
has one any of those boards soldered to the onboard USB Host controller.
Geniatech has some boards [0], [1], [2] (maybe more) which have this
variant soldered the same way. (optional)
I've also read that "Xbox Wireless adapter for Windows" uses this chipset
(unverified).
I've also CC'ed Ganapathi Bhat who last updated the firmware for SDIO and
PCIe variant of this chipset: It would be great if the firmware
for USB variant could get an update too, as the one which we currently
have is quite old - version 15.68.4.p103, while other have some 16.*
firmware. 

[0] https://www.geniatech.com/product/xpi-3288/
[1] https://www.geniatech.com/product/xpi-imx8mm/
[2] https://www.geniatech.com/product/xpi-s905x/
 
 drivers/net/wireless/marvell/mwifiex/Kconfig |  4 ++--
 drivers/net/wireless/marvell/mwifiex/usb.c   | 14 ++++++++++++++
 drivers/net/wireless/marvell/mwifiex/usb.h   |  3 +++
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Brian Norris Dec. 6, 2023, 7:12 p.m. UTC | #1
On Tue, Dec 05, 2023 at 10:02:37PM +0100, Alex Bee wrote:
> This patch restores USB8897 support which was removed with
> Commit 60a188a2715f ("mwifiex: remove USB8897 chipset support")

Did you look at the reason for that removal?

"if both mwifiex_pcie and mwifiex_usb modules are enabled by user,
sometimes mwifiex_usb wins the race even if user wants wlan interface to
be on PCIe and USB for bluetooth. This patch solves the problem."

That sounds like a legitimate problem, even if the solution isn't
perfect. Do you have any alternatives?

I don't have such hardware, so I don't know its behaviors nor can I test
it. But it'd be nice if we could differentiate USB-only vs PCIe+USB
somehow.

> There are quite some devices which use this chipset with USB interface.
> The firmware still exits in linux upstream firmware repo and this simple
> patch is all what is required to support it in upstream linux (again).
> 
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> Recently I upstreamed support for Geniatec XPI-3128 SBC which actually
> has one any of those boards soldered to the onboard USB Host controller.
> Geniatech has some boards [0], [1], [2] (maybe more) which have this
> variant soldered the same way. (optional)
> I've also read that "Xbox Wireless adapter for Windows" uses this chipset
> (unverified).
> I've also CC'ed Ganapathi Bhat who last updated the firmware for SDIO and
> PCIe variant of this chipset: It would be great if the firmware
> for USB variant could get an update too, as the one which we currently
> have is quite old - version 15.68.4.p103, while other have some 16.*
> firmware. 

The old maintainers here seem to have gone AWOL, so I wouldn't hold my
breath on getting any support from them.

Brian

> [0] https://www.geniatech.com/product/xpi-3288/
> [1] https://www.geniatech.com/product/xpi-imx8mm/
> [2] https://www.geniatech.com/product/xpi-s905x/
>  
>  drivers/net/wireless/marvell/mwifiex/Kconfig |  4 ++--
>  drivers/net/wireless/marvell/mwifiex/usb.c   | 14 ++++++++++++++
>  drivers/net/wireless/marvell/mwifiex/usb.h   |  3 +++
>  3 files changed, 19 insertions(+), 2 deletions(-)
Brian Norris Dec. 6, 2023, 7:13 p.m. UTC | #2
(Altering CC list; sorry, I didn't notice the RESEND at first)

On Wed, Dec 06, 2023 at 11:12:02AM -0800, Brian Norris wrote:
> On Tue, Dec 05, 2023 at 10:02:37PM +0100, Alex Bee wrote:
> > This patch restores USB8897 support which was removed with
> > Commit 60a188a2715f ("mwifiex: remove USB8897 chipset support")
> 
> Did you look at the reason for that removal?
> 
> "if both mwifiex_pcie and mwifiex_usb modules are enabled by user,
> sometimes mwifiex_usb wins the race even if user wants wlan interface to
> be on PCIe and USB for bluetooth. This patch solves the problem."
> 
> That sounds like a legitimate problem, even if the solution isn't
> perfect. Do you have any alternatives?
> 
> I don't have such hardware, so I don't know its behaviors nor can I test
> it. But it'd be nice if we could differentiate USB-only vs PCIe+USB
> somehow.
> 
> > There are quite some devices which use this chipset with USB interface.
> > The firmware still exits in linux upstream firmware repo and this simple
> > patch is all what is required to support it in upstream linux (again).
> > 
> > Signed-off-by: Alex Bee <knaerzche@gmail.com>
> > ---
> > Recently I upstreamed support for Geniatec XPI-3128 SBC which actually
> > has one any of those boards soldered to the onboard USB Host controller.
> > Geniatech has some boards [0], [1], [2] (maybe more) which have this
> > variant soldered the same way. (optional)
> > I've also read that "Xbox Wireless adapter for Windows" uses this chipset
> > (unverified).
> > I've also CC'ed Ganapathi Bhat who last updated the firmware for SDIO and
> > PCIe variant of this chipset: It would be great if the firmware
> > for USB variant could get an update too, as the one which we currently
> > have is quite old - version 15.68.4.p103, while other have some 16.*
> > firmware. 
> 
> The old maintainers here seem to have gone AWOL, so I wouldn't hold my
> breath on getting any support from them.
> 
> Brian
> 
> > [0] https://www.geniatech.com/product/xpi-3288/
> > [1] https://www.geniatech.com/product/xpi-imx8mm/
> > [2] https://www.geniatech.com/product/xpi-s905x/
> >  
> >  drivers/net/wireless/marvell/mwifiex/Kconfig |  4 ++--
> >  drivers/net/wireless/marvell/mwifiex/usb.c   | 14 ++++++++++++++
> >  drivers/net/wireless/marvell/mwifiex/usb.h   |  3 +++
> >  3 files changed, 19 insertions(+), 2 deletions(-)
>
Alex Bee Dec. 6, 2023, 9:51 p.m. UTC | #3
Hi Brian,

Am 06.12.23 um 20:13 schrieb Brian Norris:
> (Altering CC list; sorry, I didn't notice the RESEND at first)
>
> On Wed, Dec 06, 2023 at 11:12:02AM -0800, Brian Norris wrote:
>> On Tue, Dec 05, 2023 at 10:02:37PM +0100, Alex Bee wrote:
>>> This patch restores USB8897 support which was removed with
>>> Commit 60a188a2715f ("mwifiex: remove USB8897 chipset support")
>> Did you look at the reason for that removal?
I did. And honestly I didn't understand it - in the first place.
>> "if both mwifiex_pcie and mwifiex_usb modules are enabled by user,
>> sometimes mwifiex_usb wins the race even if user wants wlan interface to
>> be on PCIe and USB for bluetooth. This patch solves the problem."
>>
>> That sounds like a legitimate problem, even if the solution isn't
>> perfect. Do you have any alternatives?
>>
>> I don't have such hardware, so I don't know its behaviors nor can I test
>> it. But it'd be nice if we could differentiate USB-only vs PCIe+USB
>> somehow.

I re-tried to decipher the commit message and re-checked everything and 
I think the patch is fine as is:

What they probably mean in the commit message is: There is an USB id 
clash for 1286:2046 with their "Marvell NFC-over-USB driver" [0]. So 
that has nothing to do with bluetooth :)
However Commit 8a81a96bd116 ("NFC: nfcmrvl: update USB device id") 
restricted the InterfaceSubClass and the InterfaceProtocol for those 
devices, so that this clash does no longer exist. This patch here takes 
the correct ones fot this wifi adapter (I checked with lsusb).

If it's not that I really don't know what they mean: Neither 1286:2045 
nor 1286:2046 usb ids are used anywhere else tree-wide.

[0] https://cateee.net/lkddb/web-lkddb/NFC_MRVL_USB.html

Fine?

Alex

>>> There are quite some devices which use this chipset with USB interface.
>>> The firmware still exits in linux upstream firmware repo and this simple
>>> patch is all what is required to support it in upstream linux (again).
>>>
>>> Signed-off-by: Alex Bee <knaerzche@gmail.com>
>>> ---
>>> Recently I upstreamed support for Geniatec XPI-3128 SBC which actually
>>> has one any of those boards soldered to the onboard USB Host controller.
>>> Geniatech has some boards [0], [1], [2] (maybe more) which have this
>>> variant soldered the same way. (optional)
>>> I've also read that "Xbox Wireless adapter for Windows" uses this chipset
>>> (unverified).
>>> I've also CC'ed Ganapathi Bhat who last updated the firmware for SDIO and
>>> PCIe variant of this chipset: It would be great if the firmware
>>> for USB variant could get an update too, as the one which we currently
>>> have is quite old - version 15.68.4.p103, while other have some 16.*
>>> firmware.
>> The old maintainers here seem to have gone AWOL, so I wouldn't hold my
>> breath on getting any support from them.
>>
>> Brian
>>
>>> [0] https://www.geniatech.com/product/xpi-3288/
>>> [1] https://www.geniatech.com/product/xpi-imx8mm/
>>> [2] https://www.geniatech.com/product/xpi-s905x/
>>>   
>>>   drivers/net/wireless/marvell/mwifiex/Kconfig |  4 ++--
>>>   drivers/net/wireless/marvell/mwifiex/usb.c   | 14 ++++++++++++++
>>>   drivers/net/wireless/marvell/mwifiex/usb.h   |  3 +++
>>>   3 files changed, 19 insertions(+), 2 deletions(-)
Brian Norris Dec. 14, 2023, 1:31 a.m. UTC | #4
Hi,

On Wed, Dec 06, 2023 at 10:51:03PM +0100, Alex Bee wrote:
> Am 06.12.23 um 20:13 schrieb Brian Norris:
> > On Wed, Dec 06, 2023 at 11:12:02AM -0800, Brian Norris wrote:
> > > On Tue, Dec 05, 2023 at 10:02:37PM +0100, Alex Bee wrote:
> > > > This patch restores USB8897 support which was removed with
> > > > Commit 60a188a2715f ("mwifiex: remove USB8897 chipset support")
> > > Did you look at the reason for that removal?
> I did. And honestly I didn't understand it - in the first place.
> > > "if both mwifiex_pcie and mwifiex_usb modules are enabled by user,
> > > sometimes mwifiex_usb wins the race even if user wants wlan interface to
> > > be on PCIe and USB for bluetooth. This patch solves the problem."
> > > 
> > > That sounds like a legitimate problem, even if the solution isn't
> > > perfect. Do you have any alternatives?
> > > 
> > > I don't have such hardware, so I don't know its behaviors nor can I test
> > > it. But it'd be nice if we could differentiate USB-only vs PCIe+USB
> > > somehow.
> 
> I re-tried to decipher the commit message and re-checked everything and I
> think the patch is fine as is:
> 
> What they probably mean in the commit message is: There is an USB id clash
> for 1286:2046 with their "Marvell NFC-over-USB driver" [0]. So that has
> nothing to do with bluetooth :)
> However Commit 8a81a96bd116 ("NFC: nfcmrvl: update USB device id")
> restricted the InterfaceSubClass and the InterfaceProtocol for those
> devices, so that this clash does no longer exist. This patch here takes the
> correct ones fot this wifi adapter (I checked with lsusb).

That's a nice theory, but they never mentioned NFC. I'd expect they
wouldn't have such a large omission in their description ... but maybe I
place too much faith.

It also doesn't make sense how commit 60a188a2715f would have helped
anything for such an explanation, because they were already using an
appropriately restrictive ID (via USB_DEVICE_AND_INTERFACE_INFO) in
mwifiex_usb. Disabling mwifiex_usb wouldn't do anything interesting;
disabling nfcmrvl would.

And I suspect Bluetooth is mentioned as that's typically the thing they
expect to use for USB (and not WiFi). Clearly that expectation has
changed in the intervening years though.

> If it's not that I really don't know what they mean: Neither 1286:2045 nor
> 1286:2046 usb ids are used anywhere else tree-wide.

I suppose I'm not 100% sure either, but I'm guessing that somehow both
PCIe and USB interfaces are enabled for WLAN -- i.e., we see a PCIe
11ab:2b38 and a USB 1286:2046 device show up -- and it's just a matter
of which one probes first (and downloads the relevant firmware).

That'd be an awfully silly design, and the datasheets I've seen suggest
that their pin strappings determine a single host interface for WLAN
(SDIO, USB, HSIC(?), or PCIe). But then, they also suggest the strapping
don't impact the hardware; they only affect software, which may or not
be coded well.

Personally, I only ever had SDIO modules around for 8897, so I don't
know how this PCIe+USB combo works in practice.

> [0] https://cateee.net/lkddb/web-lkddb/NFC_MRVL_USB.html
> 
> Fine?

I'm not sure. While I don't have sympathy for the Marvell/NXP folks
who've abandoned their products, I do have sympathy for the users who
may still have such systems and who we may be broken by reintrocing this
change.

On the other hand, if such users pop up again, we could probably cook a
modprobe.d entry to hack things up. I'm still not sure how to *really*
fix such an odd system in a generic way.

Brian
diff mbox series

Patch

diff --git a/drivers/net/wireless/marvell/mwifiex/Kconfig b/drivers/net/wireless/marvell/mwifiex/Kconfig
index b182f7155d66..277b75eaf91e 100644
--- a/drivers/net/wireless/marvell/mwifiex/Kconfig
+++ b/drivers/net/wireless/marvell/mwifiex/Kconfig
@@ -35,12 +35,12 @@  config MWIFIEX_PCIE
 	  mwifiex_pcie.
 
 config MWIFIEX_USB
-	tristate "Marvell WiFi-Ex Driver for USB8766/8797/8997"
+	tristate "Marvell WiFi-Ex Driver for USB8766/8797/8897/8997"
 	depends on MWIFIEX && USB
 	select FW_LOADER
 	help
 	  This adds support for wireless adapters based on Marvell
-	  8797/8997 chipset with USB interface.
+	  8797/8897/8997 chipset with USB interface.
 
 	  If you choose to build it as a module, it will be called
 	  mwifiex_usb.
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index d3ab9572e711..061be9c2ed2f 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -28,6 +28,11 @@  static const struct usb_device_id mwifiex_usb_table[] = {
 	{USB_DEVICE_AND_INTERFACE_INFO(USB8XXX_VID, USB8801_PID_2,
 				       USB_CLASS_VENDOR_SPEC,
 				       USB_SUBCLASS_VENDOR_SPEC, 0xff)},
+	/* 8897 */
+	{USB_DEVICE(USB8XXX_VID, USB8897_PID_1)},
+	{USB_DEVICE_AND_INTERFACE_INFO(USB8XXX_VID, USB8897_PID_2,
+				       USB_CLASS_VENDOR_SPEC,
+				       USB_SUBCLASS_VENDOR_SPEC, 0xff)},
 	/* 8997 */
 	{USB_DEVICE(USB8XXX_VID, USB8997_PID_1)},
 	{USB_DEVICE_AND_INTERFACE_INFO(USB8XXX_VID, USB8997_PID_2,
@@ -409,12 +414,14 @@  static int mwifiex_usb_probe(struct usb_interface *intf,
 	case USB8766_PID_1:
 	case USB8797_PID_1:
 	case USB8801_PID_1:
+	case USB8897_PID_1:
 	case USB8997_PID_1:
 		card->usb_boot_state = USB8XXX_FW_DNLD;
 		break;
 	case USB8766_PID_2:
 	case USB8797_PID_2:
 	case USB8801_PID_2:
+	case USB8897_PID_2:
 	case USB8997_PID_2:
 		card->usb_boot_state = USB8XXX_FW_READY;
 		break;
@@ -1318,6 +1325,12 @@  static int mwifiex_register_dev(struct mwifiex_adapter *adapter)
 		strcpy(adapter->fw_name, USB8997_DEFAULT_FW_NAME);
 		adapter->ext_scan = true;
 		break;
+	case USB8897_PID_1:
+	case USB8897_PID_2:
+		adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_4K;
+		strcpy(adapter->fw_name, USB8897_DEFAULT_FW_NAME);
+		adapter->ext_scan = true;
+		break;
 	case USB8766_PID_1:
 	case USB8766_PID_2:
 		adapter->tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_2K;
@@ -1615,4 +1628,5 @@  MODULE_LICENSE("GPL v2");
 MODULE_FIRMWARE(USB8766_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(USB8797_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(USB8801_DEFAULT_FW_NAME);
+MODULE_FIRMWARE(USB8897_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(USB8997_DEFAULT_FW_NAME);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.h b/drivers/net/wireless/marvell/mwifiex/usb.h
index 7e920b51994c..b7dba256e9f8 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.h
+++ b/drivers/net/wireless/marvell/mwifiex/usb.h
@@ -19,6 +19,8 @@ 
 #define USB8797_PID_2		0x2044
 #define USB8801_PID_1		0x2049
 #define USB8801_PID_2		0x204a
+#define USB8897_PID_1		0x2045
+#define USB8897_PID_2		0x2046
 #define USB8997_PID_1		0x2052
 #define USB8997_PID_2		0x204e
 
@@ -35,6 +37,7 @@ 
 #define USB8766_DEFAULT_FW_NAME	"mrvl/usb8766_uapsta.bin"
 #define USB8797_DEFAULT_FW_NAME	"mrvl/usb8797_uapsta.bin"
 #define USB8801_DEFAULT_FW_NAME	"mrvl/usb8801_uapsta.bin"
+#define USB8897_DEFAULT_FW_NAME	"mrvl/usb8897_uapsta.bin"
 #define USB8997_DEFAULT_FW_NAME	"mrvl/usbusb8997_combo_v4.bin"
 
 #define FW_DNLD_TX_BUF_SIZE	620