Message-ID: <87a5f341bb.fsf@kernel.org>
Date: Thu, 17 Oct 2024 10:48:40 +0300
From: Kalle Valo <kvalo@...nel.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,  johannes@...solutions.net,
  briannorris@...omium.org,  francesco@...cini.it,
  tsung-hsien.hsieh@....com,  kernel@...gutronix.de
Subject: Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support iw61x

Sascha Hauer <s.hauer@...gutronix.de> writes:

> 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.
>
> 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 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.

Replying to an old mail but I'm with Sascha here and I'm also skeptic
about adding a new driver. Especially my worry is that after the driver
is accepted we will not hear from NXP anymore and the community has two
almost identical drivers to maintain. There have been cases that after
taking the driver the company disappears and we (the community) are left
maintaining the abandoned driver.

Also I have not seen any convincing reasons why a new driver is needed.
For me much better approach would be to extend mwifiex like Sascha
recommends.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Powered by blists - more mailing lists