> -----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.
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> 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@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
Hi Francesco,
Thank you for the review and advice.
I will revise the patch and provide more detailed descriptions.
Jeff
