Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0881ee1d0ae43ae18ab4b71ec8b5fd62263bbb69.camel@sipsolutions.net>
Subject: Re: [PATCH] wifi: cfg80211: call reg_notifier for self managed
 wiphy from driver hint
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Wen Gong <quic_wgong@quicinc.com>, ath11k@lists.infradead.org
Cc:     linux-wireless@vger.kernel.org
Date:   Thu, 19 Jan 2023 14:52:14 +0100
In-Reply-To: <20221214093937.14987-1-quic_wgong@quicinc.com>
References: <20221214093937.14987-1-quic_wgong@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.3 (3.46.3-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-12-14 at 04:39 -0500, Wen Gong wrote:
> Currently the regulatory driver does not call the regulatory callback
> reg_notifier for self managed wiphys. Sometimes driver needs cfg80211
> to calculate the info of ieee80211_channel such as flags and power,
> and driver needs to get the info of ieee80211_channel after hint of
> driver, but driver does not know when calculation of the info of
> ieee80211_channel become finished, so add notify to driver after
> reg_process_self_managed_hint() from cfg80211 is a good way, then
> driver could get the correct info in callback of reg_notifier.

Seems reasonable - but maybe unexpected to some drivers, perhaps it
should be opt-in?

Though I guess not many drivers actually use this infrastructure in the
first place?

> @@ -3095,6 +3107,13 @@ static void notify_self_managed_wiphys(struct regu=
latory_request *request)
>  		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED &&
>  		    request->initiator =3D=3D NL80211_REGDOM_SET_BY_USER)
>  			reg_call_notifier(wiphy, request);
> +
> +		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED &&
> +		    request->initiator =3D=3D NL80211_REGDOM_SET_BY_DRIVER &&
> +		    request->wiphy_idx =3D=3D get_wiphy_idx(wiphy)) {
> +			reg_call_notifier(wiphy, request);
> +			request->wiphy_idx =3D WIPHY_IDX_INVALID;
> +		}

Why set the request->wiphy_idx here? Should this even go through
reg_process_pending_hints() at all?

> +	driver_request =3D kzalloc(sizeof(struct regulatory_request), GFP_KERNE=
L);
> +	if (!driver_request)
> +		return;
> +
> +	memcpy(driver_request, &request, sizeof(*driver_request));

kmemdup()?

> +	queue_regulatory_request(driver_request);

But again not sure you should do this, rather than calling the notifier
directly?

I mean, you could just do reg_call_notifier() here, it's already async?

johannes