Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] wireless: add check of field VHT Extended NSS BW Capable
 for 160/80+80 MHz setting
To:     Wen Gong <wgong@codeaurora.org>, johannes@sipsolutions.net,
        ath11k@lists.infradead.org
Cc:     linux-wireless@vger.kernel.org
References: <20210520034021.14690-1-wgong@codeaurora.org>
From:   Arend van Spriel <arend.vanspriel@broadcom.com>
Message-ID: <fe1b7675-23ba-a86e-90db-de5803231e21@broadcom.com>
Date:   Thu, 20 May 2021 21:21:13 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210520034021.14690-1-wgong@codeaurora.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 5/20/2021 5:40 AM, Wen Gong wrote:
> Table 9-251—Supported VHT-MCS and NSS Set subfields, it has subfield VHT Extended
> NSS BW Capable, its definition is:
> Indicates whether the STA is capable of interpreting the Extended NSS BW
> Support subfield of the VHT Capabilities Information field.
> 
> This patch is to add check for the subfield.

Almost looks good to me, but...

> Signed-off-by: Wen Gong <wgong@codeaurora.org>
> ---
>   net/wireless/chan.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index 285b8076054b..f7f86bd75cc0 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c

[...]

> @@ -1037,7 +1039,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
>   		cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK;
>   		if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ &&
>   		    cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ &&
> -		    !(vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK))
> +		    !(ext_nss_cap && vht_cap->cap & IEEE80211_VHT_CAP_EXT_NSS_BW_MASK))

you should not mix logical and binary operators so add braces around the 
binary operation here.

Regards,
Arend

>   			return false;
>   		break;
>   	default:
>