Re: [PATCH 01/13] wifi: cfg80211: Add provision to advertise multiple radio in one wiphy

Johannes Berg
2024-03-28T07:47:01Z

That's a big set, not sure I can review it all at once :)

> +struct ieee80211_chans_per_hw {
> + u32 n_chans;
> + struct ieee80211_channel chans[];

That should probably use __counted_by() these days.

> + * @hw_chans: list of the channels supported by every constituent underlying
> + * hardware. Drivers abstracting multiple discrete hardware (radio) under
> + * one wiphy can advertise the list of channels supported by each physical
> + * hardware in this list. Underlying hardware specific channel list can be
> + * used while describing interface combination for each of them.

I'd expect there to be a limit on channels being within a single band on
a single "hardware"?

> + * @num_hw: number of underlying hardware for which the channels list are

"number of [...] hardware" sounds odd to me, perhaps 'devices'?

> + * advertised in @hw_chans, 0 if multi hardware is not support. Expect >2
> + * if multi hardware is support.

>1, I guess, not >2 - and ==1 makes no sense really, maybe it should say
that?

> +static bool
> +cfg80211_hw_chans_in_supported_list(struct wiphy *wiphy,
> + const struct ieee80211_chans_per_hw *chans)
> +{
> + enum nl80211_band band;
> + struct ieee80211_supported_band *sband;
> + int i, j;
> +
> + for (i = 0; i < chans->n_chans; i++) {
> + bool found = false;

So if there should be a limit on single band per HW then probably this
check could be written more efficiently: finding the band first based on
the first channel, and then checking all the channels exist within the
band.

johannes