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

[Francesco Dolcini <francesco@dolcini.it>]

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@nxp.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