On Thu, 2024-07-11 at 09:21 +0530, Aditya Kumar Singh wrote:
>
> * In the worker, go over all the contexts again and for all such context
> which is marked with radar detected, add it to a local linked list.
Why is the local list needed first?
> +++ b/include/net/mac80211.h
> @@ -257,6 +257,7 @@ struct ieee80211_chan_req {
> * after RTS/CTS handshake to receive SMPS MIMO transmissions;
> * this will always be >= @rx_chains_static.
> * @radar_enabled: whether radar detection is enabled on this channel.
> + * @radar_detected: whether radar got detected on this channel.
> * @drv_priv: data area for driver use, will always be aligned to
> * sizeof(void *), size is determined in hw information.
> */
> @@ -269,6 +270,7 @@ struct ieee80211_chanctx_conf {
> u8 rx_chains_static, rx_chains_dynamic;
>
> bool radar_enabled;
> + bool radar_detected;
I'm not sure why you're adding this to the driver visible part of the
chanctx, I don't think that really makes sense since setting it must be
done by mac80211 through the API function to trigger all the work?
> +++ b/net/mac80211/ieee80211_i.h
> @@ -1329,6 +1329,11 @@ enum mac80211_scan_state {
>
> DECLARE_STATIC_KEY_FALSE(aql_disable);
>
> +struct radar_info {
> + struct list_head list;
> + struct cfg80211_chan_def chandef;
> +};
If it _really_ is needed this can be local to the C file.
> + INIT_LIST_HEAD(&radar_info_list);
but really, why?
> list_for_each_entry(ctx, &local->chanctx_list, list) {
> if (ctx->replace_state == IEEE80211_CHANCTX_REPLACES_OTHER)
> continue;
>
> - num_chanctx++;
> - chandef = ctx->conf.def;
> + if (ctx->conf.radar_detected) {
> + ctx->conf.radar_detected = false;
> + num_chanctx++;
> +
> + radar_info = kzalloc(sizeof(*radar_info), GFP_KERNEL);
> + if (WARN_ON(!radar_info))
> + continue;
that clearly shouldn't be a WARN_ON,
> +
> + INIT_LIST_HEAD(&radar_info->list);
> + radar_info->chandef = ctx->conf.def;
> + list_add_tail(&radar_info->list, &radar_info_list);
but I also don't really see why you couldn't just call
cfg80211_radar_event() here.
> + if (num_chanctx > 1) {
> + /* XXX: multi-channel is not supported yet in case of non-MLO */
> + if (WARN_ON(!(wiphy->flags & WIPHY_FLAG_SUPPORTS_MLO)))
> + trigger_event = false;
> + }
I don't see how that'd happen in the first place?
johannes
