linux-kernel - RE: [EXT] 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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <PAWPR04MB99106570E36AD6E198B566F09CE62@PAWPR04MB9910.eurprd04.prod.outlook.com>
Date: Tue, 21 Jan 2025 17:17:56 +0000
From: Jeff Chen <jeff.chen_1@....com>
To: Francesco Dolcini <francesco@...cini.it>
CC: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"briannorris@...omium.org" <briannorris@...omium.org>, "kvalo@...nel.org"
	<kvalo@...nel.org>, Pete Hsieh <tsung-hsien.hsieh@....com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>
Subject: RE: [EXT] Re: [PATCH 2/2] wifi: mwifiex: Fix the wrong hardware
 setting for HT40.

> -----Original Message-----
> From: Francesco Dolcini <francesco@...cini.it>
> Sent: Monday, January 20, 2025 7:12 PM
> 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; Pete
> Hsieh <tsung-hsien.hsieh@....com>; s.hauer@...gutronix.de
> 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@....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

Hi Francesco,
Thank you for the review and advice.
I will revise the patch and provide more detailed descriptions.

Jeff