Received-SPF: pass (google.com: domain of linux-wireless+bounces-1870-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Kalle Valo <kvalo@kernel.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Miri Korenblit <miriam.rachel.korenblit@intel.com>,
  linux-wireless@vger.kernel.org,  Gregory Greenman
 <gregory.greenman@intel.com>
Subject: Re: [PATCH 5/8] wifi: mac80211: disallow drivers with HT wider than HE
References: <20240111161746.3978601-1-miriam.rachel.korenblit@intel.com>
	<20240111181514.da15fe3214d2.I4df51ad2f4c844615c168bf9bdb498925b3c77d4@changeid>
	<87wmsehf3d.fsf@kernel.org>
	<aae05a63171cf0f3c81dedc24d3b0a558ce530f5.camel@sipsolutions.net>
Date: Sat, 13 Jan 2024 08:54:50 +0200
In-Reply-To: <aae05a63171cf0f3c81dedc24d3b0a558ce530f5.camel@sipsolutions.net>
	(Johannes Berg's message of "Fri, 12 Jan 2024 19:42:27 +0100")
Message-ID: <87h6jhhged.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2024-01-12 at 15:10 +0200, Kalle Valo wrote:
>> Miri Korenblit <miriam.rachel.korenblit@intel.com> writes:
>> 
>> > +			/* currently no support for HE client where HT has 40 MHz but not HT */
>> > +			if (iftd->he_cap.has_he &&
>> > +			    iftd->types_mask & (BIT(NL80211_IFTYPE_STATION) |
>> > +						BIT(NL80211_IFTYPE_P2P_CLIENT)) &&
>> > +			    sband->ht_cap.ht_supported &&
>> > +			    sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40 &&
>> > +			    !(iftd->he_cap.he_cap_elem.phy_cap_info[0] & he_40_mhz_cap))
>> > +				return -EINVAL;
>> 
>> Should there be a warning message so that this is noticed if it ever
>> happens? I don't know.
>
> Yeah I don't really know either. I've done that a lot in the past, but
> these days I'm kind of thinking that people who develop their drivers
> should have some debug story and be able to figure it out? You know
> better perhaps ...

My worry here is the time needed to figure this all out. Especially if
we have more of these kind silent checks it could get complex quite
quick.

> Though it'd kind of suck to indent this further with WARN_ON ;-)

Yeah, WARN_ON() feels a bit too much. I was more thinking about a small
pr_err() or something like that which give at least some hint what's
happening.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches