Re: [PATCH v3 01/11] cfg80211: use only HE capability to set prohibited flags in 6 GHz

[Rajkumar Manoharan <rmanohar@codeaurora.org>]

On 2020-05-27 06:43, Johannes Berg wrote:
> Hi,
> 
> This is what we have in this area:
> https://p.sipsolutions.net/d8e56772a261199a.txt
> 
> but I see it's also incomplete.
> 
>> +static bool cfg80211_is_6ghz_freq(u32 freq)
>> +{
>> +	return (freq > 5940 && freq < 7105);
>> +}
> 
> That doesn't really make sense, I don't want to see those hardcoded
> frequencies all over the place.
> 
>>  	case NL80211_CHAN_WIDTH_40:
>>  		width = 40;
>> +		if (cfg80211_is_6ghz_freq(chandef->center_freq1)) {
> 
> You can check chandef->chan->band instead. (In fact, we did)
> 
Got it..

>> +			if (!he_cap)
>> +				return false;
>> +			if (!he_cap->has_he_6ghz)
>> +				return false;
> 
> I'm not sure you should even _get_ here with a 6 GHz channel if you
> don't have 6 GHz capability? I mean, why did you register the channel 
> in
> the first place then? This seems unnecessarily complex. If the channel
> didn't exist, it was rejected long before here.
> 
Hmm... Agreed.

> However, looking at D6.0, maybe we do need some checks of the HE
> capability?
> 
>> +			if (!(he_cap->he_cap_elem.phy_cap_info[0] &
>> +			      IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G))
>> +				return false;
> 
> Looks like even D6.0 still changed something in this area...
> 
> Evidently our patch just assumed that in 6 GHz all of this is 
> supported,
> but the spec doesn't support that theory :-)
> 
IIUC the same bits are applicable for both 5 GHz & 6 GHz. I understand 
the macro doesn't
capture both.

> Can you respin this with D6.0 taken into account?
> 
Let me check again and respin after your series. Does it sound good?

-Rajkumar

_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k