Re: [PATCH v2,2/2] wifi: mac80211: Refactor STA CSA parsing flow

[Johannes Berg <johannes@xxxxxxxxxxxxxxxx>]

On Mon, 2023-11-13 at 10:11 +0800, Michael-CY Lee wrote:
> 
> +	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
> +	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> +						      &center_freq1,
> +						      &center_freq2)) {

Unless I missed it, I see two places here calling it together, so seems
reasonable to fill in a chandef here instead?

> +	new_chan_width = ieee80211_operating_class_to_chan_width(op_class);
> +	if (!ieee80211_operating_class_to_center_freq(op_class, chan,
> +						      &center_freq1,
> +						      &center_freq2)) {


Here you have it too.


> +		new_chan_width = NL80211_CHAN_WIDTH_20;
> +		center_freq1 = chan->center_freq;

And actually you could just have a chandef created with

    cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_WIDTH_20)

which mirrors the failure case here, and just not update it when
something like

    ieee80211_update_chandef_from_op_class(op_class, &chandef)

returned false (not that I necessarily think that name should be used.)

Or just pass the channel, and make it create one with WIDTH_20 in the
failure case?

    ieee80211_create_chandef_from_opclass(chan, op_class, &chandef);

which is is maybe even nicer?

I'm also not quite sure why you're converting to operation elements
first?

johannes