Message ID | 20240809094533.1660-1-yu-hao.lin@nxp.com |
---|---|
Headers | show |
Series | wifi: nxpwifi: create nxpwifi to support iw61x | expand |
Hi Kalle, I found Nxpwifi patch v2 is put in "Deferred" state quickly. Patch v2 is mainly to address the comments from Johannes and it actually took quite some efforts. We understand there are areas to improve and we are committed to continue enhance/maintain the driver. Could you let me know your plan for reviewing Nxpwifi? Is there anything we can do to move this forward? Thanks, David > From: David Lin <yu-hao.lin@nxp.com> > Sent: Friday, August 9, 2024 5:45 PM > To: linux-wireless@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; kvalo@kernel.org; johannes@sipsolutions.net; > briannorris@chromium.org; francesco@dolcini.it; Pete Hsieh > <tsung-hsien.hsieh@nxp.com>; David Lin <yu-hao.lin@nxp.com> > Subject: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support iw61x > > 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. > > [2] Some features, as of now, WPA2/WPA3 personal/enterprise are offloaded > to host wpa_supplicant/hostapd. > > v2: > - Rename ioctl.h and sta_ioctl.c to cfg.h and sta_cfg.c. > - Remove header file semaphore.h. > - Use static value for cookie instead of run time random number. > - Use ERR_PTR(), IS_ERR() and PTR_ERR(). > - Use Kernel defined return error code. > - Remove unnecessary private ie definitions. > - Remove mutex async_mutex and related code. > - Consolidate multiple workqueue into one. > - Add the support for PSK SHA256. > - Use tasklet for Rx handler. > - Remove unused functions. > - Remove compile warning. > > David Lin (43): > wifi: nxpwifi: add 11ac.c > wifi: nxpwifi: add 11ac.h > wifi: nxpwifi: add 11h.c > wifi: nxpwifi: add 11n_aggr.c > wifi: nxpwifi: add 11n_aggr.h > wifi: nxpwifi: add 11n.c > wifi: nxpwifi: add 11n.h > wifi: nxpwifi: add 11n_rxreorder.c > wifi: nxpwifi: add 11n_rxreorder.h > wifi: nxpwifi: add cfg80211.c > wifi: nxpwifi: add cfg80211.h > wifi: nxpwifi: add cfg.h > wifi: nxpwifi: add cfp.c > wifi: nxpwifi: add cmdevt.c > wifi: nxpwifi: add cmdevt.h > wifi: nxpwifi: add debugfs.c > wifi: nxpwifi: add decl.h > wifi: nxpwifi: add ethtool.c > wifi: nxpwifi: add fw.h > wifi: nxpwifi: add ie.c > wifi: nxpwifi: add init.c > wifi: nxpwifi: add join.c > wifi: nxpwifi: add main.c > wifi: nxpwifi: add main.h > wifi: nxpwifi: add scan.c > wifi: nxpwifi: add sdio.c > wifi: nxpwifi: add sdio.h > wifi: nxpwifi: add sta_cfg.c > wifi: nxpwifi: add sta_cmd.c > wifi: nxpwifi: add sta_event.c > wifi: nxpwifi: add sta_rx.c > wifi: nxpwifi: add sta_tx.c > wifi: nxpwifi: add txrx.c > wifi: nxpwifi: add uap_cmd.c > wifi: nxpwifi: add uap_event.c > wifi: nxpwifi: add uap_txrx.c > wifi: nxpwifi: add util.c > wifi: nxpwifi: add util.h > wifi: nxpwifi: add wmm.c > wifi: nxpwifi: add wmm.h > wifi: nxpwifi: add nxp sdio vendor id and iw61x device id > wifi: nxpwifi: add Makefile and Kconfig files for nxpwifi compilation > wifi: nxpwifi: add nxpwifi related information to MAINTAINERS > > MAINTAINERS | 7 + > drivers/net/wireless/Kconfig | 1 + > drivers/net/wireless/Makefile | 1 + > drivers/net/wireless/nxp/Kconfig | 17 + > drivers/net/wireless/nxp/Makefile | 3 + > drivers/net/wireless/nxp/nxpwifi/11ac.c | 366 ++ > drivers/net/wireless/nxp/nxpwifi/11ac.h | 33 + > drivers/net/wireless/nxp/nxpwifi/11h.c | 433 ++ > drivers/net/wireless/nxp/nxpwifi/11n.c | 851 ++++ > drivers/net/wireless/nxp/nxpwifi/11n.h | 163 + > drivers/net/wireless/nxp/nxpwifi/11n_aggr.c | 276 ++ > drivers/net/wireless/nxp/nxpwifi/11n_aggr.h | 21 + > .../net/wireless/nxp/nxpwifi/11n_rxreorder.c | 917 ++++ > .../net/wireless/nxp/nxpwifi/11n_rxreorder.h | 72 + > drivers/net/wireless/nxp/nxpwifi/Kconfig | 22 + > drivers/net/wireless/nxp/nxpwifi/Makefile | 38 + > drivers/net/wireless/nxp/nxpwifi/cfg.h | 445 ++ > drivers/net/wireless/nxp/nxpwifi/cfg80211.c | 3773 +++++++++++++++++ > drivers/net/wireless/nxp/nxpwifi/cfg80211.h | 19 + > drivers/net/wireless/nxp/nxpwifi/cfp.c | 484 +++ > drivers/net/wireless/nxp/nxpwifi/cmdevt.c | 1285 ++++++ > drivers/net/wireless/nxp/nxpwifi/cmdevt.h | 92 + > drivers/net/wireless/nxp/nxpwifi/debugfs.c | 1041 +++++ > drivers/net/wireless/nxp/nxpwifi/decl.h | 294 ++ > drivers/net/wireless/nxp/nxpwifi/ethtool.c | 58 + > drivers/net/wireless/nxp/nxpwifi/fw.h | 2249 ++++++++++ > drivers/net/wireless/nxp/nxpwifi/ie.c | 501 +++ > drivers/net/wireless/nxp/nxpwifi/init.c | 694 +++ > drivers/net/wireless/nxp/nxpwifi/join.c | 915 ++++ > drivers/net/wireless/nxp/nxpwifi/main.c | 1666 ++++++++ > drivers/net/wireless/nxp/nxpwifi/main.h | 1478 +++++++ > drivers/net/wireless/nxp/nxpwifi/scan.c | 2806 ++++++++++++ > drivers/net/wireless/nxp/nxpwifi/sdio.c | 2648 ++++++++++++ > drivers/net/wireless/nxp/nxpwifi/sdio.h | 340 ++ > drivers/net/wireless/nxp/nxpwifi/sta_cfg.c | 1307 ++++++ > drivers/net/wireless/nxp/nxpwifi/sta_cmd.c | 3233 ++++++++++++++ > drivers/net/wireless/nxp/nxpwifi/sta_event.c | 864 ++++ > drivers/net/wireless/nxp/nxpwifi/sta_rx.c | 244 ++ > drivers/net/wireless/nxp/nxpwifi/sta_tx.c | 209 + > drivers/net/wireless/nxp/nxpwifi/txrx.c | 358 ++ > drivers/net/wireless/nxp/nxpwifi/uap_cmd.c | 1169 +++++ > drivers/net/wireless/nxp/nxpwifi/uap_event.c | 491 +++ > drivers/net/wireless/nxp/nxpwifi/uap_txrx.c | 499 +++ > drivers/net/wireless/nxp/nxpwifi/util.c | 946 +++++ > drivers/net/wireless/nxp/nxpwifi/util.h | 108 + > drivers/net/wireless/nxp/nxpwifi/wmm.c | 1379 ++++++ > drivers/net/wireless/nxp/nxpwifi/wmm.h | 78 + > include/linux/mmc/sdio_ids.h | 3 + > 48 files changed, 34897 insertions(+) > create mode 100644 drivers/net/wireless/nxp/Kconfig create mode 100644 > drivers/net/wireless/nxp/Makefile create mode 100644 > drivers/net/wireless/nxp/nxpwifi/11ac.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11ac.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11h.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n_aggr.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n_aggr.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n_rxreorder.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/11n_rxreorder.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/Kconfig > create mode 100644 drivers/net/wireless/nxp/nxpwifi/Makefile > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cfg.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cfg80211.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cfg80211.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cfp.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cmdevt.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/cmdevt.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/debugfs.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/decl.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/ethtool.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/fw.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/ie.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/init.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/join.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/main.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/main.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/scan.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sdio.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sdio.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_cfg.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_cmd.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_event.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_rx.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_tx.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/txrx.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/uap_cmd.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/uap_event.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/uap_txrx.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/util.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/util.h > create mode 100644 drivers/net/wireless/nxp/nxpwifi/wmm.c > create mode 100644 drivers/net/wireless/nxp/nxpwifi/wmm.h > > > base-commit: 555ba98448f8916bff87067853a7e931949e6b57 > -- > 2.34.1
David Lin <yu-hao.lin@nxp.com> writes: > I found Nxpwifi patch v2 is put in "Deferred" state quickly. The way I use patchwork states is described here: https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#checking_state_of_patches_from_patchwork Basically I try to follow the "Inbox Zero" method and keep the amount of patches in New state (my "inbox") low and the Deferred state is my todo list. > Patch v2 is mainly to address the comments from Johannes and it > actually took quite some efforts. We understand there are areas to > improve and we are committed to continue enhance/maintain the driver. > > Could you let me know your plan for reviewing Nxpwifi? Reviewing new drivers take a lot of time, at the moment I'm following what other reviewers say before I'll look at it myself. The process is so slow and patience is needed. The last thing I want to see that once the driver is accepted NXP disappears and we end up having an unmaintained driver. Way too many companies do that. > Is there anything we can do to move this forward? Yes, get involved with the community and help us, don't just expect that we do everything for you gratis. Especially helping Brian with mwifiex review/testing helps us (we get a better driver) and also helps you (you learn how the community works and you gain trust in the community). An excellent example is Realtek. Few years back Realtek was not involved with upstream development at all. But now Ping is doing an awesome job with maintaining ALL Realtek drivers, including the old drivers, and I even trust him so much that I pull directly from this tree. This is what NXP should aim for.
> From: Kalle Valo <kvalo@kernel.org> > Sent: Thursday, August 15, 2024 5:36 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; > johannes@sipsolutions.net; briannorris@chromium.org; > francesco@dolcini.it; Pete Hsieh <tsung-hsien.hsieh@nxp.com> > Subject: [EXT] Re: [PATCH v2 00/43] wifi: nxpwifi: create nxpwifi to support > iw61x > > David Lin <yu-hao.lin@nxp.com> writes: > > > I found Nxpwifi patch v2 is put in "Deferred" state quickly. > > The way I use patchwork states is described here: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireles > s.wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fsubmittingpatch > es%23checking_state_of_patches_from_patchwork&data=05%7C02%7Cyu-ha > o.lin%40nxp.com%7C8d360234b61d4e2c13e108dcbd0da0a4%7C686ea1d3bc > 2b4c6fa92cd99c5c301635%7C0%7C0%7C638593113436356863%7CUnknown > %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW > wiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=1XA1Ax66zhnU72efN0wAghLJ > %2F3DR1%2B8PAxXkiHphA%2FY%3D&reserved=0 > > Basically I try to follow the "Inbox Zero" method and keep the amount of > patches in New state (my "inbox") low and the Deferred state is my todo list. > > > Patch v2 is mainly to address the comments from Johannes and it > > actually took quite some efforts. We understand there are areas to > > improve and we are committed to continue enhance/maintain the driver. > > > > Could you let me know your plan for reviewing Nxpwifi? > > Reviewing new drivers take a lot of time, at the moment I'm following what > other reviewers say before I'll look at it myself. The process is so slow and > patience is needed. Understood. > > The last thing I want to see that once the driver is accepted NXP disappears > and we end up having an unmaintained driver. Way too many companies do > that. > Yes, we are committed to maintaining nxpwifi. > > Is there anything we can do to move this forward? > > Yes, get involved with the community and help us, don't just expect that we > do everything for you gratis. Especially helping Brian with mwifiex > review/testing helps us (we get a better driver) and also helps you (you learn > how the community works and you gain trust in the community). > Yes, we will continue to help to maintain mwifiex, also we will like to involve and learn how the community works. > An excellent example is Realtek. Few years back Realtek was not involved > with upstream development at all. But now Ping is doing an awesome job > with maintaining ALL Realtek drivers, including the old drivers, and I even > trust him so much that I pull directly from this tree. This is what NXP should > aim for. > > -- > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw > ork.kernel.org%2Fproject%2Flinux-wireless%2Flist%2F&data=05%7C02%7Cyu- > hao.lin%40nxp.com%7C8d360234b61d4e2c13e108dcbd0da0a4%7C686ea1d3 > bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638593113436367796%7CUnknow > n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha > WwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=4ly%2Bien1%2FscxRSdEdN0C > hvRMH%2Bh8kbYfEKbjxWyqodw%3D&reserved=0 > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwireles > s.wiki.kernel.org%2Fen%2Fdevelopers%2Fdocumentation%2Fsubmittingpatch > es&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C8d360234b61d4e2c13e108 > dcbd0da0a4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638593 > 113436374405%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ > QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=va > i2UN%2BKu2I5wKfmUoL8LEk7qBXmIuIK0eB4rtwoRDs%3D&reserved=0 Thanks for the information. Thanks, David
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. Sascha
On Thu, Aug 22, 2024 at 02:56:25PM +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. Maybe you can share an RFC patch with what you currently have available to support IW61x within the current mwifiex driver? Given what David @NXP wrote here > > [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. David, given the code, he should be able to highlight the limitation of such approach and hopefully we can find a good path forward? One of the challenges with the current mwifiex driver is that it supports quite a few wireless devices, and any new addition must be done in such a way to not break the old stuff. Not to mention the "Odd Fixes" maintenance status of the driver, quoting Brian: "My only interest in mwifiex is in making sure existing hardware (especially those used on Chromebooks) doesn't get significantly worse.". Francesco
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 |
> From: Francesco Dolcini <francesco@dolcini.it> > Sent: Saturday, August 24, 2024 9:49 PM > To: Sascha Hauer <s.hauer@pengutronix.de>; David Lin <yu-hao.lin@nxp.com> > Cc: 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 > 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 Thu, Aug 22, 2024 at 02:56:25PM +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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > > w.nxp.com%2Fproducts%2Fwireless-connectivity%2Fwi-fi-plus-bluetooth- > > > > &data=05%7C02%7Cyu-hao.lin%40nxp.com%7C6d7cf356022443738a3d08dcc4 > 437 > > > > 926%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63860104128552 > 0681% > > > > 7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > TiI6 > > > > Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Z5iuaainSgM%2BVdUBh > AK4RHcv > > > vERhiw3yu0S5kRKsldM%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. > > Maybe you can share an RFC patch with what you currently have available to > support IW61x within the current mwifiex driver? > > Given what David @NXP wrote here I don't think he can add IW61x to mwifiex without issues if the code for VDLL is missing. Without the code to handle VDLL, command timeout will happen when firmware requests VDLL from driver. This is new API which is not needed for the firmware of legacy devices. After this modification, a full QA cycle should be done to confirm the driver can work without issues. If you try to add these code to Mwifiex, you should also guarantee the code won't affect existed devices of Mwifiex. BTW, adding the support for 11ax is not a trivial job. > > > > [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. > > David, given the code, he should be able to highlight the limitation of such > approach and hopefully we can find a good path forward? > > One of the challenges with the current mwifiex driver is that it supports quite a > few wireless devices, and any new addition must be done in such a way to not > break the old stuff. Not to mention the "Odd Fixes" > maintenance status of the driver, quoting Brian: "My only interest in mwifiex is > in making sure existing hardware (especially those used on > Chromebooks) doesn't get significantly worse.". > > Francesco Yes. This is also one consideration for us to create Nxpwifi driver to support NXP new chips. David
> 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. David
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
> From: Calvin Owens <calvin@wbinvd.org> > Sent: Monday, August 26, 2024 10:50 AM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de>; 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 > Subject: Re: [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 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-blueto > > > > > oth- > > > > > > > > > &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-blue > > > > > toot > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > 42 > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > 60237 > > > > 3871805816%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luM > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=bLjsRTRsR%2 > BTtA > > > jUIVDY396ZF%2BIkwwUFhAubTCin3IVk%3D&reserved=0 > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fco > m > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > 42 > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > 60237 > > > > 3871815005%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > V2luM > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=RfY6N6WWXI > n0gZP > > > SBoRySz5eeU8WkFH2HvFHLVNgu3Q%3D&reserved=0 > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fco > m > > > > 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 > Nxpwifi is not only a fork from Mwifiex. Especially after we modified Nxpwifi based on the comments from Johannes. I think the real bugs fixes of Mwifiex will become less frequently. We can monitor these patches and apply them from Mwifiex to Nxpwifi. If we fix issues of Nxpwifi which is also related to Mwifiex, we will submit patches back to Mwifiex. Thanks, David
On Monday 08/26 at 02:56 +0000, David Lin wrote: > > From: Calvin Owens <calvin@wbinvd.org> > > Sent: Monday, August 26, 2024 10:50 AM > > To: David Lin <yu-hao.lin@nxp.com> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>; 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 > > Subject: Re: [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 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-blueto > > > > > > oth- > > > > > > > > > > > > &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-blue > > > > > > toot > > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > > 42 > > > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > > 60237 > > > > > > 3871805816%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > > V2luM > > > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=bLjsRTRsR%2 > > BTtA > > > > jUIVDY396ZF%2BIkwwUFhAubTCin3IVk%3D&reserved=0 > > > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fco > > m > > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > > 42 > > > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > > 60237 > > > > > > 3871815005%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > > V2luM > > > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=RfY6N6WWXI > > n0gZP > > > > SBoRySz5eeU8WkFH2HvFHLVNgu3Q%3D&reserved=0 > > > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fco > > m > > > > > > 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 > > > > Nxpwifi is not only a fork from Mwifiex. Especially after we modified Nxpwifi > based on the comments from Johannes. I understand you've done real work here. But at the same time, nearly 1/3rd of the changeset against the original driver is renaming things: {0}[calvin ~/linux/drivers/net/wireless/nxp/nxpwifi] tar cf ~/nxpwifi-v2.tar . {0}[calvin ~/linux/drivers/net/wireless/nxp/nxpwifi] cd ../../marvell/mwifiex {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] tar xf ~/nxpwifi-v2.tar {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git add * {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git diff --cached --shortstat 42 files changed, 13878 insertions(+), 14274 deletions(-) {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] sed -i 's/nxpwifi/mwifiex/g' * {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] sed -i 's/NXPWIFI/MWIFIEX/g' * {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git add * {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git diff --cached --shortstat 42 files changed, 9940 insertions(+), 10336 deletions(-) {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] cat * | wc -l 45591 I'd consider that a fork. For those following at home, I pushed a branch to github with nxpwifi v2 applied as a single patchbomb to mwifiex, with the renames backed out: https://github.com/jcalvinowens/linux/tree/work/nxpwifi-on-mwifiex-no-renames For my own selfish part, I agree with Sascha: I'd prefer to see that changeset as patches against the existing driver, because I know from experience that having a community built around one driver will enable me to deliver better results for my users, and will make it easier for me to contribute when I find bugs to fix. But this is just one random opinion, I'm not who you need to convince :) Thanks, Calvin > I think the real bugs fixes of Mwifiex will become less frequently. > We can monitor these patches and apply them from Mwifiex to Nxpwifi. > > If we fix issues of Nxpwifi which is also related to Mwifiex, we will > submit patches back to Mwifiex. > > Thanks, > David >
> From: Calvin Owens <calvin@wbinvd.org> > Sent: Monday, August 26, 2024 1:31 PM > To: David Lin <yu-hao.lin@nxp.com> > Cc: Sascha Hauer <s.hauer@pengutronix.de>; 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: Re: [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 Monday 08/26 at 02:56 +0000, David Lin wrote: > > > From: Calvin Owens <calvin@wbinvd.org> > > > Sent: Monday, August 26, 2024 10:50 AM > > > To: David Lin <yu-hao.lin@nxp.com> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>; > > > 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 > > > Subject: Re: [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 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-bl > > > > > > > ueto > > > > > > > oth- > > > > > > > > > > > > > > > > &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- > > > > > > > blue > > > > > > > toot > > > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%25 > > > > > > 2Fgi%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cfeaa265eadca4cef88 > > > > > > 3608dcc5903c0d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > 6 > > > > > > 02470501904923%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ > QI > > > > > > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7uf > > > > > Rl6fwgw1oxLa4DBQxpwrm9D6ojbdFYNAuHRCA5vk%3D&reserved=0 > > > > > > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > > > 42 > > > > > > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > > > 60237 > > > > > > > > > 3871805816%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > > > V2luM > > > > > > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=bLjsRTRsR%2 > > > BTtA > > > > > jUIVDY396ZF%2BIkwwUFhAubTCin3IVk%3D&reserved=0 > > > > > > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fc > > > > o > > > m > > > > > > > > > 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%25 > > > > > > 2Fgi%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cfeaa265eadca4cef88 > > > > > > 3608dcc5903c0d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > 6 > > > > > > 02470501916954%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ > QI > > > > > > joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=wbG > > > > > SKlaMVBnzXon1ZEEAmctWCBCkVDFqWld9d6zLvOQ%3D&reserved=0 > > > > > > > > > t.kernel%2F&data=05%7C02%7Cyu-hao.lin%40nxp.com%7C582c3c0573b74f83 > > > 42 > > > > > > > > > 3408dcc579bc4a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638 > > > 60237 > > > > > > > > > 3871815005%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi > > > V2luM > > > > > > > > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=RfY6N6WWXI > > > n0gZP > > > > > SBoRySz5eeU8WkFH2HvFHLVNgu3Q%3D&reserved=0 > > > > > > > > > .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fc > > > > o > > > m > > > > > > > > > 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 > > > > > > > Nxpwifi is not only a fork from Mwifiex. Especially after we modified > > Nxpwifi based on the comments from Johannes. > > I understand you've done real work here. But at the same time, nearly 1/3rd of > the changeset against the original driver is renaming things: > > {0}[calvin ~/linux/drivers/net/wireless/nxp/nxpwifi] tar cf > ~/nxpwifi-v2.tar . > {0}[calvin ~/linux/drivers/net/wireless/nxp/nxpwifi] > cd ../../marvell/mwifiex > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] tar xf > ~/nxpwifi-v2.tar > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git add * > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git diff --cached > --shortstat > 42 files changed, 13878 insertions(+), 14274 deletions(-) > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] sed -i > 's/nxpwifi/mwifiex/g' * > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] sed -i > 's/NXPWIFI/MWIFIEX/g' * > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git add * > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] git diff --cached > --shortstat > 42 files changed, 9940 insertions(+), 10336 deletions(-) > {0}[calvin ~/linux/drivers/net/wireless/marvell/mwifiex] cat * | wc -l > 45591 > > I'd consider that a fork. For those following at home, I pushed a branch to > github with nxpwifi v2 applied as a single patchbomb to mwifiex, with the > renames backed out: > > > https://github.co/ > m%2Fjcalvinowens%2Flinux%2Ftree%2Fwork%2Fnxpwifi-on-mwifiex-no-renam > es&data=05%7C02%7Cyu-hao.lin%40nxp.com%7Cfeaa265eadca4cef883608dcc > 5903c0d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386024705 > 01925106%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2 > luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=D%2BO4x > pCU1OMwPYkMq7T6X9Cf95Gdt7bNhnfqNzaOyQA%3D&reserved=0 > > For my own selfish part, I agree with Sascha: I'd prefer to see that changeset as > patches against the existing driver, because I know from experience that > having a community built around one driver will enable me to deliver better > results for my users, and will make it easier for me to contribute when I find > bugs to fix. > > But this is just one random opinion, I'm not who you need to convince :) > > Thanks, > Calvin > Yes. From your commit, it is very clear that Nxpwifi has a lot of changes which are used for: Cleanup original issues of Mwifiex for checkpatch. Remove command response file due to command table is used (command response handler can be hooked to command node when command is searched at first time). Only updated API/interface, host command and SDIO new mode are supported. Only support separate auth/assoc. Modifications based on comments from Johannes (please check changelog for patch v2). Nxpwifi will be a good base line to continue to add new features and new NXP WiFi new chips. It is also clean and easy to be maintained. Thanks, David
On Sat, Aug 24, 2024 at 03:48:39PM +0200, Francesco Dolcini wrote: > On Thu, Aug 22, 2024 at 02:56:25PM +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. > > Maybe you can share an RFC patch with what you currently have available > to support IW61x within the current mwifiex driver? I just did, see: https://lore.kernel.org/linux-wireless/20240826072648.167004-1-s.hauer@pengutronix.de/ Sascha