Hi,
I suppose you're mostly looking for feedback from Mediatek etc. for
their multi-radio architecture. Couple of comments on the code anyway:
> + if (valid_links) {
> + for_each_set_bit(link_id, &valid_links, IEEE80211_MLD_MAX_NUM_LINKS) {
We just added some for_each_sdata_link() macro or so, so you don't need
the distinction between MLO and non-MLO. I really don't like seeing
that, if we do that all the time we have far too many places that would
do it.
> + bss_conf = rcu_dereference(sdata->vif.link_conf[link_id]);
> + if (!bss_conf)
> + continue;
> + conf = rcu_dereference(bss_conf->chanctx_conf);
> + if (conf && conf->def.chan &&
> + conf->def.chan->center_freq == freq) {
> + is_freq_match = true;
You could much easier just "return true" here, and "return false" later
after all of it, without the variable.
(Curious: do you have some coding style guidelines that this breaks?
Like "single return from function" style? Because I see it often from
Qualcomm for no real reason. Maybe I'm also just misremembering, but I
don't see a good reason in cases like this to even want to write it with
a variable. Seems more natural to just say "ok got it, return true".)
> if (prev) {
> - rx.sdata = prev;
> + if (!status->freq ||
> + ieee80211_rx_is_sdata_match(prev, status->freq)) {
> + rx.sdata = prev;
nit: you can combine it all into one condition now:
if (prev && (!status->freq || ...)) {
}
not sure that's really all that much better, but saves the reindent?
Actually, it might be better overall to move the !freq into
ieee80211_rx_is_sdata_match()? So just have
ieee80211_rx_is_sdata_match(...)
{
if (!freq)
return true;
...
}
and then you don't need the || in the caller, which simplifies that? And
also gives you a more natural place to put the comment.
johannes
