| Message ID | 20231205210237.209332-1-knaerzche@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: mwifiex: Restore USB8897 chipset support | expand |
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(-)
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(-)
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 --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
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(-)