Hello Francesco,
Thank you for reviewing the patch.
> -----Original Message-----
> From: Francesco Dolcini <francesco@xxxxxxxxxx>
> Sent: Monday, January 20, 2025 7:12 PM
> To: Jeff Chen <jeff.chen_1@xxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> briannorris@xxxxxxxxxxxx; kvalo@xxxxxxxxxx; francesco@xxxxxxxxxx; Pete
> Hsieh <tsung-hsien.hsieh@xxxxxxx>; s.hauer@xxxxxxxxxxxxxx
> Subject: [EXT] Re: [PATCH 2/2] wifi: mwifiex: Fix the wrong hardware setting for
> HT40.
>
>
> Can you expand this a little bit?
>
> - Is this a regression?
- No, this is not a regression.
> - What is the impact of this missing configuration? It's not working at all?
Without this configuration, the connection operates at 20MHz even if the
access point supports 40MHz bandwidth. This means that while the device can
connect, it does so with reduced bandwidth, potentially affecting performance
and throughput.
> It's working in some unexpected way (please explain)?
As mentioned above, the connection operates at 20MHz even if the
access point supports 40MHz bandwidth.
> - Should this backported to stable (probably given the answer before it should
> be obvious the answer to this question)?
Considering that HT40 mode is not commonly used on the 2.4GHz band due to
limited bandwidth and potential interference, backporting this fix may not be
necessary.
>
> Anything else worth mentioning?
>
There is nothing additional to mention.
> >
> > Signed-off-by: Jeff Chen <jeff.chen_1@xxxxxxx>
> > ---
> > 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
