[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZsbRNbcq6tBeKPID@pengutronix.de>
Date: Thu, 22 Aug 2024 07:48:37 +0200
From: Sascha Hauer <s.hauer@...gutronix.de>
To: Brian Norris <briannorris@...omium.org>,
Francesco Dolcini <francesco@...cini.it>,
Kalle Valo <kvalo@...nel.org>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
kernel@...gutronix.de
Subject: Re: [PATCH 11/31] wifi: mwifiex: use priv index as bss_num
On Tue, Aug 20, 2024 at 01:55:36PM +0200, Sascha Hauer wrote:
> Instead of looking up an unused bss_num each time we add a virtual
> interface, associate a fixed bss_num to each priv and for simplicity
> just use the array index.
>
> With bss_num unique to each priv mwifiex_get_priv_by_id() doesn't need
> the bss_type argument anymore, so it's removed.
I misunderstood the driver here. I thought bss_num specifies the
instance and bss_type specifies the type of this instance. It's the
other way round: bss_num is the nth instance of type bss_type. Also
the device doesn't have 16 instances of configurable type, instead
it only has 1 station mode instance. Hence, when
bss_type == MWIFIEX_BSS_TYPE_STA every other bss_num than 0 is invalid.
Likewise there are also only three instances of type
MWIFIEX_BSS_TYPE_UAP available.
I made some assumptions on this misunderstanding throughout this series,
so it needs rework.
It would be really great to have some documentation about these devices.
Sascha
>
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> ---
> drivers/net/wireless/marvell/mwifiex/cfg80211.c | 11 ++---
> drivers/net/wireless/marvell/mwifiex/cmdevt.c | 6 +--
> drivers/net/wireless/marvell/mwifiex/main.c | 1 +
> drivers/net/wireless/marvell/mwifiex/main.h | 54 ++++--------------------
> drivers/net/wireless/marvell/mwifiex/sta_event.c | 3 +-
> drivers/net/wireless/marvell/mwifiex/txrx.c | 9 ++--
> 6 files changed, 19 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> index 784f342a9bf23..d5a2c8f726b9e 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
> @@ -952,8 +952,6 @@ mwifiex_init_new_priv_params(struct mwifiex_private *priv,
> return -EOPNOTSUPP;
> }
>
> - priv->bss_num = mwifiex_get_unused_bss_num(adapter, priv->bss_type);
> -
> spin_lock_irqsave(&adapter->main_proc_lock, flags);
> adapter->main_locked = false;
> spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> @@ -2999,8 +2997,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
> return ERR_PTR(-EINVAL);
> }
>
> - priv = mwifiex_get_unused_priv_by_bss_type(
> - adapter, MWIFIEX_BSS_TYPE_STA);
> + priv = mwifiex_get_unused_priv(adapter);
> if (!priv) {
> mwifiex_dbg(adapter, ERROR,
> "could not get free private struct\n");
> @@ -3029,8 +3026,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
> return ERR_PTR(-EINVAL);
> }
>
> - priv = mwifiex_get_unused_priv_by_bss_type(
> - adapter, MWIFIEX_BSS_TYPE_UAP);
> + priv = mwifiex_get_unused_priv(adapter);
> if (!priv) {
> mwifiex_dbg(adapter, ERROR,
> "could not get free private struct\n");
> @@ -3056,8 +3052,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
> return ERR_PTR(-EINVAL);
> }
>
> - priv = mwifiex_get_unused_priv_by_bss_type(
> - adapter, MWIFIEX_BSS_TYPE_P2P);
> + priv = mwifiex_get_unused_priv(adapter);
> if (!priv) {
> mwifiex_dbg(adapter, ERROR,
> "could not get free private struct\n");
> diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> index 4f814110f750e..d91351384c6bb 100644
> --- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> +++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
> @@ -496,8 +496,7 @@ int mwifiex_process_event(struct mwifiex_adapter *adapter)
> (u16) eventcause;
>
> /* Get BSS number and corresponding priv */
> - priv = mwifiex_get_priv_by_id(adapter, EVENT_GET_BSS_NUM(eventcause),
> - EVENT_GET_BSS_TYPE(eventcause));
> + priv = mwifiex_get_priv_by_id(adapter, EVENT_GET_BSS_NUM(eventcause));
> if (!priv)
> priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>
> @@ -847,8 +846,7 @@ int mwifiex_process_cmdresp(struct mwifiex_adapter *adapter)
>
> /* Get BSS number and corresponding priv */
> priv = mwifiex_get_priv_by_id(adapter,
> - HostCmd_GET_BSS_NO(le16_to_cpu(resp->seq_num)),
> - HostCmd_GET_BSS_TYPE(le16_to_cpu(resp->seq_num)));
> + HostCmd_GET_BSS_NO(le16_to_cpu(resp->seq_num)));
> if (!priv)
> priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
> /* Clear RET_BIT from HostCmd */
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
> index 7cb90a6a8ccab..888f2801d6f2a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.c
> +++ b/drivers/net/wireless/marvell/mwifiex/main.c
> @@ -85,6 +85,7 @@ static int mwifiex_register(void *card, struct device *dev,
> if (!adapter->priv[i])
> goto error;
>
> + adapter->priv[i]->bss_num = i;
> adapter->priv[i]->adapter = adapter;
> adapter->priv_num++;
> }
> diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
> index 541bc50a9561c..2938e55a38d79 100644
> --- a/drivers/net/wireless/marvell/mwifiex/main.h
> +++ b/drivers/net/wireless/marvell/mwifiex/main.h
> @@ -1297,20 +1297,12 @@ mwifiex_copy_rates(u8 *dest, u32 pos, u8 *src, int len)
> * upon the BSS type and BSS number.
> */
> static inline struct mwifiex_private *
> -mwifiex_get_priv_by_id(struct mwifiex_adapter *adapter,
> - u8 bss_num, u8 bss_type)
> +mwifiex_get_priv_by_id(struct mwifiex_adapter *adapter, u8 bss_num)
> {
> - int i;
> -
> - for (i = 0; i < adapter->priv_num; i++) {
> - if (adapter->priv[i]->bss_mode == NL80211_IFTYPE_UNSPECIFIED)
> - continue;
> + if (bss_num >= MWIFIEX_MAX_BSS_NUM)
> + return NULL;
>
> - if ((adapter->priv[i]->bss_num == bss_num) &&
> - (adapter->priv[i]->bss_type == bss_type))
> - break;
> - }
> - return ((i < adapter->priv_num) ? adapter->priv[i] : NULL);
> + return adapter->priv[bss_num];
> }
>
> /*
> @@ -1332,47 +1324,19 @@ mwifiex_get_priv(struct mwifiex_adapter *adapter,
> return ((i < adapter->priv_num) ? adapter->priv[i] : NULL);
> }
>
> -/*
> - * This function checks available bss_num when adding new interface or
> - * changing interface type.
> - */
> -static inline u8
> -mwifiex_get_unused_bss_num(struct mwifiex_adapter *adapter, u8 bss_type)
> -{
> - u8 i, j;
> - int index[MWIFIEX_MAX_BSS_NUM];
> -
> - memset(index, 0, sizeof(index));
> - for (i = 0; i < adapter->priv_num; i++)
> - if (adapter->priv[i]->bss_type == bss_type &&
> - !(adapter->priv[i]->bss_mode ==
> - NL80211_IFTYPE_UNSPECIFIED)) {
> - index[adapter->priv[i]->bss_num] = 1;
> - }
> - for (j = 0; j < MWIFIEX_MAX_BSS_NUM; j++)
> - if (!index[j])
> - return j;
> - return -1;
> -}
> -
> /*
> * This function returns the first available unused private structure pointer.
> */
> static inline struct mwifiex_private *
> -mwifiex_get_unused_priv_by_bss_type(struct mwifiex_adapter *adapter,
> - u8 bss_type)
> +mwifiex_get_unused_priv(struct mwifiex_adapter *adapter)
> {
> - u8 i;
> + int i;
>
> for (i = 0; i < adapter->priv_num; i++)
> - if (adapter->priv[i]->bss_mode ==
> - NL80211_IFTYPE_UNSPECIFIED) {
> - adapter->priv[i]->bss_num =
> - mwifiex_get_unused_bss_num(adapter, bss_type);
> - break;
> - }
> + if (adapter->priv[i]->bss_mode == NL80211_IFTYPE_UNSPECIFIED)
> + return adapter->priv[i];
>
> - return ((i < adapter->priv_num) ? adapter->priv[i] : NULL);
> + return NULL;
> }
>
> /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c
> index b5f3821a6a8f2..15f057d010a3d 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
> @@ -456,8 +456,7 @@ void mwifiex_process_multi_chan_event(struct mwifiex_private *priv,
> for (i = 0; i < intf_num; i++) {
> bss_type = grp_info->bss_type_numlist[i] >> 4;
> bss_num = grp_info->bss_type_numlist[i] & BSS_NUM_MASK;
> - intf_priv = mwifiex_get_priv_by_id(adapter, bss_num,
> - bss_type);
> + intf_priv = mwifiex_get_priv_by_id(adapter, bss_num);
> if (!intf_priv) {
> mwifiex_dbg(adapter, ERROR,
> "Invalid bss_type bss_num\t"
> diff --git a/drivers/net/wireless/marvell/mwifiex/txrx.c b/drivers/net/wireless/marvell/mwifiex/txrx.c
> index f44e22f245110..21cfee3290377 100644
> --- a/drivers/net/wireless/marvell/mwifiex/txrx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/txrx.c
> @@ -31,8 +31,7 @@ int mwifiex_handle_rx_packet(struct mwifiex_adapter *adapter,
>
> local_rx_pd = (struct rxpd *) (skb->data);
> /* Get the BSS number from rxpd, get corresponding priv */
> - priv = mwifiex_get_priv_by_id(adapter, local_rx_pd->bss_num &
> - BSS_NUM_MASK, local_rx_pd->bss_type);
> + priv = mwifiex_get_priv_by_id(adapter, local_rx_pd->bss_num & BSS_NUM_MASK);
> if (!priv)
> priv = mwifiex_get_priv(adapter, MWIFIEX_BSS_ROLE_ANY);
>
> @@ -165,8 +164,7 @@ static int mwifiex_host_to_card(struct mwifiex_adapter *adapter,
> struct mwifiex_txinfo *tx_info;
>
> tx_info = MWIFIEX_SKB_TXCB(skb);
> - priv = mwifiex_get_priv_by_id(adapter, tx_info->bss_num,
> - tx_info->bss_type);
> + priv = mwifiex_get_priv_by_id(adapter, tx_info->bss_num);
> if (!priv) {
> mwifiex_dbg(adapter, ERROR,
> "data: priv not found. Drop TX packet\n");
> @@ -281,8 +279,7 @@ int mwifiex_write_data_complete(struct mwifiex_adapter *adapter,
> return 0;
>
> tx_info = MWIFIEX_SKB_TXCB(skb);
> - priv = mwifiex_get_priv_by_id(adapter, tx_info->bss_num,
> - tx_info->bss_type);
> + priv = mwifiex_get_priv_by_id(adapter, tx_info->bss_num);
> if (!priv)
> goto done;
>
>
> --
> 2.39.2
>
>
--
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 |
Powered by blists - more mailing lists