mbox series

[00/43] wifi: nxpwifi: create nxpwifi to support iw61x

Message ID 20240621075208.513497-1-yu-hao.lin@nxp.com (mailing list archive)
Headers show
Series wifi: nxpwifi: create nxpwifi to support iw61x | expand

Message

David Lin June 21, 2024, 7:51 a.m. UTC 
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.

David Lin (43):
  wifi: nxpwifi: add 11ac.c
  wifi: nxpwifi: add 11ac.h
  wifi: nxpwifi: add 11h.c
  wifi: nxpwifi: add 11n.c
  wifi: nxpwifi: add 11n.h
  wifi: nxpwifi: add 11n_aggr.c
  wifi: nxpwifi: add 11n_aggr.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 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 ioctl.h
  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_cmd.c
  wifi: nxpwifi: add sta_event.c
  wifi: nxpwifi: add sta_ioctl.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        |  432 ++
 drivers/net/wireless/nxp/nxpwifi/11n.c        |  851 ++++
 drivers/net/wireless/nxp/nxpwifi/11n.h        |  163 +
 drivers/net/wireless/nxp/nxpwifi/11n_aggr.c   |  278 ++
 drivers/net/wireless/nxp/nxpwifi/11n_aggr.h   |   21 +
 .../net/wireless/nxp/nxpwifi/11n_rxreorder.c  |  928 ++++
 .../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/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    | 1042 +++++
 drivers/net/wireless/nxp/nxpwifi/decl.h       |  299 ++
 drivers/net/wireless/nxp/nxpwifi/ethtool.c    |   58 +
 drivers/net/wireless/nxp/nxpwifi/fw.h         | 2262 ++++++++++
 drivers/net/wireless/nxp/nxpwifi/ie.c         |  502 +++
 drivers/net/wireless/nxp/nxpwifi/init.c       |  696 +++
 drivers/net/wireless/nxp/nxpwifi/ioctl.h      |  445 ++
 drivers/net/wireless/nxp/nxpwifi/join.c       |  910 ++++
 drivers/net/wireless/nxp/nxpwifi/main.c       | 1726 ++++++++
 drivers/net/wireless/nxp/nxpwifi/main.h       | 1507 +++++++
 drivers/net/wireless/nxp/nxpwifi/scan.c       | 2894 +++++++++++++
 drivers/net/wireless/nxp/nxpwifi/sdio.c       | 2646 ++++++++++++
 drivers/net/wireless/nxp/nxpwifi/sdio.h       |  340 ++
 drivers/net/wireless/nxp/nxpwifi/sta_cmd.c    | 3229 ++++++++++++++
 drivers/net/wireless/nxp/nxpwifi/sta_event.c  |  858 ++++
 drivers/net/wireless/nxp/nxpwifi/sta_ioctl.c  | 1320 ++++++
 drivers/net/wireless/nxp/nxpwifi/sta_rx.c     |  244 ++
 drivers/net/wireless/nxp/nxpwifi/sta_tx.c     |  215 +
 drivers/net/wireless/nxp/nxpwifi/txrx.c       |  362 ++
 drivers/net/wireless/nxp/nxpwifi/uap_cmd.c    | 1170 +++++
 drivers/net/wireless/nxp/nxpwifi/uap_event.c  |  483 +++
 drivers/net/wireless/nxp/nxpwifi/uap_txrx.c   |  498 +++
 drivers/net/wireless/nxp/nxpwifi/util.c       |  751 ++++
 drivers/net/wireless/nxp/nxpwifi/util.h       |   90 +
 drivers/net/wireless/nxp/nxpwifi/wmm.c        | 1397 ++++++
 drivers/net/wireless/nxp/nxpwifi/wmm.h        |   95 +
 include/linux/mmc/sdio_ids.h                  |    3 +
 48 files changed, 34928 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/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/ioctl.h
 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_cmd.c
 create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_event.c
 create mode 100644 drivers/net/wireless/nxp/nxpwifi/sta_ioctl.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: 238d636723a30311e20fde0a361662e829fe488b

Comments

Brian Norris June 21, 2024, 5:53 p.m. UTC | #1 
On Fri, Jun 21, 2024 at 12:52 AM David Lin <yu-hao.lin@nxp.com> wrote:
> This driver is a derivative of existing Mwifiex [1] and based on similar
> full-MAC architecture [2].

For the record, mwifiex is a fairly awful driver. For one, its locking
schemes are generally unhelpful or nonexistent, and sometimes
placebo-like (as in, they look like they are protecting certain data,
but they do a very poor job of it). So I'm not sure this is a
promising start. It was just yesterday, in fact, that I was telling a
colleague that if mwifiex was proposed for inclusion in mainline
today, I would reject it.

Also, I'm far from interested in reviewing a new driver here. My only
interest in mwifiex is in making sure existing hardware (especially
those used on Chromebooks) doesn't get significantly worse. That
interest doesn't extend to "nxpwifi".

I just want to be up-front about it, and that you might as well drop
me from the CC list. (Of course, that's not a requirement. I can
ignore email too.)

Brian
Johannes Berg June 21, 2024, 6:20 p.m. UTC | #2 
On Fri, 2024-06-21 at 15:51 +0800, David Lin wrote:
> 
>   wifi: nxpwifi: add ioctl.h

even the name here sounds questionable :)

>  48 files changed, 34928 insertions(+)
> 

This is ... huge. I don't know who could possibly review it at all.

A quick look suggests that it's got a bunch of things we probably really
don't want to do that way any more, like

using semaphores in a wifi driver:

> +#include <linux/semaphore.h>

having a bunch of (sometimes wrong!) element definitions in a driver:

> +struct ieee_types_aid {
...
> +	u16 aid;

embedding a (default?) wireless_dev when clearly the driver supports
more than one netdev/wdev:

> +	struct wireless_dev wdev;

Having multiple own workqueues is probably also unreasonable:

> +	struct workqueue_struct *dfs_cac_workqueue;
> +	struct workqueue_struct *dfs_chan_sw_workqueue;
> +	struct workqueue_struct *workqueue;
> +	struct workqueue_struct *rx_workqueue;
> +	struct workqueue_struct *host_mlme_workqueue;

as is a misnamed mutex, but really you could use wiphy work and likely
not have a mutex at all:

> +	/* mutex for scan */
> +	struct mutex async_mutex;

(even mac80211 only has one mutex left, and that's for a specific case
where otherwise we have some issues!)

questionable locking schemes, as evidenced simply by "is something
locked" variables existing:

> +	bool rx_locked;
> +	bool main_locked;

locking code, rather than data?

> +	/* spin lock for main process */
> +	spinlock_t main_proc_lock;

but also simple things like not wanting to use ERR_PTR()?

> +static int nxpwifi_register(void *card, struct device *dev,
> +			    struct nxpwifi_if_ops *if_ops, void **padapter)

(padapter is an out parameter)

Why random numbers for cookies instead of just assigning from a static
variable:

> +		*cookie = get_random_u32() | 1;

Open-coding -EPERM?

> +	if (nxpwifi_deinit_priv_params(priv))
> +		return -1;

Using -EFAULT for FW errors seems like a really bad idea:

> +	if (nxpwifi_drv_get_data_rate(priv, &rate)) {
> +		nxpwifi_dbg(priv->adapter, ERROR,
> +			    "getting data rate error\n");
> +		return -EFAULT;


But I really just scrolled through this briefly, this wasn't a real
review. I don't know who could do a real review, but as is, it looks
like someone _should_.

johannes