Re: [EXT] Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support iw61x

[Calvin Owens <calvin@wbinvd.org>]

On Monday 08/26 at 02:33 +0000, David Lin wrote:
> > From: Calvin Owens <calvin@wbinvd.org>
> > Sent: Monday, August 26, 2024 4:38 AM
> > To: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: David Lin <yu-hao.lin@nxp.com>; linux-wireless@vger.kernel.org;
> > linux-kernel@vger.kernel.org; kvalo@kernel.org; johannes@sipsolutions.net;
> > briannorris@chromium.org; francesco@dolcini.it; Pete Hsieh
> > <tsung-hsien.hsieh@nxp.com>; kernel@pengutronix.de; calvin@wbinvd.org
> > Subject: [EXT] Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support
> > iw61x
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report
> > this email' button
> >
> >
> > 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://ww/
> > > > w.nxp.com%2Fproducts%2Fwireless-connectivity%2Fwi-fi-plus-bluetooth-
> > > >
> > &data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cff25728795724a618a5208dcc5
> > 45c
> > > >
> > 5fd%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63860215067862
> > 3224%
> > > >
> > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> > TiI6
> > > >
> > Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=U0Cej8ysBD%2Fg1Sa4Ia
> > Ph63Ot
> > > > iTcemadiCfMINYM%2BRL4%3D&reserved=0
> > > > plus-802-15-4/2-4-5-ghz-dual-band-1x1-wi-fi-6-802-11ax-plus-bluetoot
> > > > h-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%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcom
> > mit%2F%3Fid%3Df92363d12359&data=05%7C02%7Cyu-hao.lin%40nxp.com%7
> > Cff25728795724a618a5208dcc545c5fd%7C686ea1d3bc2b4c6fa92cd99c5c3016
> > 35%7C0%7C0%7C638602150678637352%7CUnknown%7CTWFpbGZsb3d8eyJW
> > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> > %7C%7C%7C&sdata=mzrLLqJNee7vIdV47j8xVSU%2FByjh%2FnNKnRsx1nw3yNo
> > %3D&reserved=0
> >
> > 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%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcom
> > mit%2F%3Fid%3Dc84b06a48c4d&data=05%7C02%7Cyu-hao.lin%40nxp.com%7
> > Cff25728795724a618a5208dcc545c5fd%7C686ea1d3bc2b4c6fa92cd99c5c3016
> > 35%7C0%7C0%7C638602150678649431%7CUnknown%7CTWFpbGZsb3d8eyJW
> > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> > %7C%7C%7C&sdata=UGjDfngO1POWuydIfmOL%2BR%2BqJ1BoDQW6NboQUV
> > q2Xh8%3D&reserved=0
> >
> > 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 think Nxpwifi will support NXP new WiFi chips and Mwifiex will support existed NXP WiFi chips.

Hi David,

I understand that. I don't think that really changes anything: there
will still be many future patches which need to be applied to both,
because the bug being fixed existed before the fork. As the forked
driver diverges, that will only become more difficult and error prone.

Thanks,
Calvin

> David