linux-kernel - Re: [PATCH 2/2] wifi: mwifiex: Fix the wrong hardware setting for HT40.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z44vj59nWIiswq7s@gaggiata.pivistrello.it>
Date: Mon, 20 Jan 2025 12:12:15 +0100
From: Francesco Dolcini <francesco@...cini.it>
To: Jeff Chen <jeff.chen_1@....com>
Cc: linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
	briannorris@...omium.org, kvalo@...nel.org, francesco@...cini.it,
	tsung-hsien.hsieh@....com, s.hauer@...gutronix.de
Subject: Re: [PATCH 2/2] wifi: mwifiex: Fix the wrong hardware setting for
 HT40.

Hello Jeff,
thanks for the patch.

On Mon, Jan 20, 2025 at 03:40:11PM +0800, Jeff Chen wrote:
> Add the missing bandwidth configuration for HT40.

Can you expand this a little bit?

- Is this a regression?
- What is the impact of this missing configuration? It's not working at all?
  It's working in some unexpected way (please explain)?
- Should this backported to stable (probably given the answer before it should
  be obvious the answer to this question)?

Anything else worth mentioning?

> 
> Signed-off-by: Jeff Chen <jeff.chen_1@....com>
> ---
>  drivers/net/wireless/marvell/mwifiex/11n.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c
> index 66f0f5377ac1..4ae0b4aaa09a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/11n.c
> +++ b/drivers/net/wireless/marvell/mwifiex/11n.c
> @@ -308,7 +308,7 @@ mwifiex_cmd_append_11n_tlv(struct mwifiex_private *priv,
>  	int ret_len = 0;
>  	struct ieee80211_supported_band *sband;
>  	struct ieee_types_header *hdr;
> -	u8 radio_type;
> +	u8 radio_type, secch_offset;
>  
>  	if (!buffer || !*buffer)
>  		return ret_len;
> @@ -401,13 +401,15 @@ mwifiex_cmd_append_11n_tlv(struct mwifiex_private *priv,
>  		chan_list->chan_scan_param[0].radio_type =
>  			mwifiex_band_to_radio_type((u8) bss_desc->bss_band);
>  
> -		if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40 &&
> -		    bss_desc->bcn_ht_oper->ht_param &
> -		    IEEE80211_HT_PARAM_CHAN_WIDTH_ANY)
> -			SET_SECONDARYCHAN(chan_list->chan_scan_param[0].
> -					  radio_type,
> -					  (bss_desc->bcn_ht_oper->ht_param &
> -					  IEEE80211_HT_PARAM_CHA_SEC_OFFSET));
> +		if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) {
> +			if (bss_desc->bcn_ht_oper->ht_param & IEEE80211_HT_PARAM_CHAN_WIDTH_ANY) {
> +				chan_list->chan_scan_param[0].radio_type |= (CHAN_BW_40MHZ << 2);

setting `radio_type |= (CHAN_BW_40MHZ << 2)` seems the only real change on this
patch, correct? Anything else is cosmetic, correct?

would doing just this change be equivalent, right?

	SET_SECONDARYCHAN(chan_list->chan_scan_param[0].
			  radio_type | (CHAN_BW_40MHZ << 2),
			  (bss_desc->bcn_ht_oper->ht_param &
			  IEEE80211_HT_PARAM_CHA_SEC_OFFSET));


Francesco