On Sun, 2025-01-12 at 13:40 +0530, Sarika Sharma wrote:
> On 1/10/2025 2:49 PM, Johannes Berg wrote:
> > On Fri, 2025-01-10 at 09:54 +0530, Sarika Sharma wrote:
> > >
> > > struct station_info {
> > >
> > > + struct link_station_info deflink;
> >
> > Having a deflink here seems kind of questionable?
>
> ohh! why so? In other structures like vif, sdata, sta, we did the same
> way. So why can't we have the same way here as well?
Well, (a) I tend to think in hindsight it was sort of a mistake, but (b)
we couldn't really do it differently due to all drivers. But here you're
already touching all the code anyway, and it's mostly cfg80211/mac80211,
so why bother?
> > Why not pass multiple pointers?
>
> Sorry, I didn't get you.
> Did you mean keep the sinfo structure and call this for filling all link
> level, non-mL, ML information?
I think I meant that we could have per link pointers passed to the
method, but we might as well just have them in the sinfo struct as you
introduce in later patches.
> > > - if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&
> > > + if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&
> > >
> >
> > A little less renaming would probably also make this easier to review.
>
> Sure, but this is corresponding to filling link level data, that's why
> just renaming sinfo to link_sinfo.
Yeah, but does it matter so much? :)
I guess the flipside is that this way it's easier to review that all
values fall into the right place, but I think the data structure
organisation you did mostly ensures that anyway? Not sure though.
johannes
