linux-kernel - Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support iw61x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZsuWExGZyY8Tvu6s@mozart.vkv.me>
Date: Sun, 25 Aug 2024 13:37:39 -0700
From: Calvin Owens <calvin@...nvd.org>
To: Sascha Hauer <s.hauer@...gutronix.de>
Cc: David Lin <yu-hao.lin@....com>, linux-wireless@...r.kernel.org,
	linux-kernel@...r.kernel.org, kvalo@...nel.org,
	johannes@...solutions.net, briannorris@...omium.org,
	francesco@...cini.it, tsung-hsien.hsieh@....com,
	kernel@...gutronix.de, calvin@...nvd.org
Subject: Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support iw61x

On Thursday 08/22 at 14:56 +0200, Sascha Hauer wrote:
> On Fri, Aug 09, 2024 at 05:44:50PM +0800, David Lin wrote:
> > This series adds support for IW61x which is a new family of 2.4/5 GHz
> > dual-band 1x1 Wi-Fi 6, Bluetooth/Bluetooth Low Energy 5.2 and 15.4
> > tri-radio single chip by NXP. These devices support 20/40/80MHz
> > single spatial stream in both STA and AP mode. Communication to the
> > IW61x is done via SDIO interface
> > 
> > This driver is a derivative of existing Mwifiex [1] and based on similar
> > full-MAC architecture [2]. It has been tested with i.MX8M Mini evaluation
> > kits in both AP and STA mode.
> > 
> > All code passes sparse and checkpatch
> > 
> > Data sheet (require registration):
> > https://www.nxp.com/products/wireless-connectivity/wi-fi-plus-bluetooth-
> > plus-802-15-4/2-4-5-ghz-dual-band-1x1-wi-fi-6-802-11ax-plus-bluetooth-5-
> > 4-plus-802-15-4-tri-radio-solution:IW612
> > 
> > Known gaps to be addressed in the following patches,
> >   - Enable 11ax capabilities. This initial patch support up to 11ac.
> >   - Support DFS channel. This initial patch doesn't support DFS channel in
> >     both AP/STA mode.
> > 
> > This patch is presented as a request for comment with the intention of being
> > made into a patch after initial feedbacks are addressed
> > 
> > [1] We had considered adding IW61x to mwifiex driver, however due to
> >     FW architecture, host command interface and supported features are
> >     significantly different, we have to create the new nxpwifi driver.
> >     Subsequent NXP chipsets will be added and sustained in this new driver.
> 
> I added IW61x support to the mwifiex driver and besides the VDLL
> handling which must be added I didn't notice any differences. There
> might be other differences, but I doubt that these can't be integrated
> into the mwifiex driver.

Hi Sascha,

I'd also love to see this patchset, if you're able to share it. I can
test on an IW612 if that's helpful at all.

> Honestly I don't think adding a new driver is a good ideai, given how big
> wifi drivers are and how limited the review bandwidth is.
> 
> What we'll end up with is that we'll receive the same patches for both
> drivers, or worse, only for one driver while the other stays unpatched.

I have some concrete experience with "in-tree driver forks" like this:
a pair of SCSI drivers named mpt2sas and mpt3sas.

The latter was created as a near copy of the former:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f92363d12359

The result was *exactly* what you forsee happening here: both drivers
were constantly missing fixes from the other, and they were just subtly
different enough that it wasn't simple to "port" patches from one to the
other. It was a frustrating experience for everybody involved. I think
their git histories prove your point, I'd encourage everyone with a
horse in this race to take a look at them.

It took three years to finally unify them:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c84b06a48c4d

I doubt anyone would disagree that wifi drivers are much more complex
than SCSI drivers. It would be strictly *worse* here, and the path to
unifying them strictly longer.

Thanks,
Calvin

> I even found some of the bugs and deficiencies I am just fixing for the
> mwifiex driver in the nxpwifi driver as well. So please direct your
> effort to improving the existing driver rather than putting more burden
> to the maintainers by adding a new driver. I am sure this is the faster
> path to get the necessary changes upstream, plus users of the mwifiex
> driver will profit from these changes as well.
> 
> Of course I don't have to decide this. The wifi maintainer(s) will have
> the final word, but these are my 2 cents on this topic.
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |