Re: [PATCH] wifi: mwifiex: Restore USB8897 chipset support

[Brian Norris <briannorris@chromium.org>]

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