Re: [PATCH 02/13] wifi: nl80211: send underlying multi-hardware channel capabilities to user space

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2024-03-29 at 19:51 +0530, Vasanthakumar Thiagarajan wrote:
> 
> On 3/28/2024 5:31 PM, Johannes Berg wrote:
> > On Thu, 2024-03-28 at 15:48 +0530, Karthikeyan Periyasamy wrote:
> > > On 3/28/2024 1:19 PM, Johannes Berg wrote:
> > > > On Thu, 2024-03-28 at 12:59 +0530, Karthikeyan Periyasamy wrote:
> > > > > +/**
> > > > > + * nl80211_multi_hw_attrs - multi-hw attributes
> > > > > + *
> > > > > + * @NL80211_MULTI_HW_ATTR_INVALID: invalid
> > > > > + * @NL80211_MULTI_HW_ATTR_IDX: (u8) multi-HW index to refer the underlying HW
> > > > > + *	for which the supported channel list is advertised. Internally refer
> > > > > + *	the index of the wiphy's @hw_chans array.
> > > > Is there a good reason to expose this? Seems pretty internal to me, and
> > > > not sure what userspace would do with it?
> > > 
> > > Hostapd use this hw index for the channel switch cmd.
> > 
> > What, where? I don't see that in this patchset? And why??
> > 
> > In any case, unless I just missed it and you're going to tell me to look
> > at patch N, we don't need it here, and then I'd prefer to keep it an
> > internal detail until it's needed.
> > 
> > > The hw index used as a sanity check to identify whether the user
> > > requested channel fall under the different hw or not.
> > 
> > You mean within hostapd itself? That doesn't make sense, it can derive
> > that information differently.
> > 
> > > In split-phy hardware, 5GHz band supported by two physical hardware's.
> > > First supports 5GHz Low band and second supports 5GHz High band.
> > 
> > In your hardware design anyway, but yeah, I get it.
> > 
> > > In this case, user space cannot use band vise check here to identify
> > > given channel or freq supported in the given hardware.
> > 
> > No, but it also doesn't need an index assigned by the kernel for that.
> > 
> 
> The only purpose of hw index is to link hw_chans to per-hardware 
> interface combination capability so that each hardware can be
> identified during interface combination checks capability vs
> current state. Thinking if we can embed the channel list also
> into per-hardware interface combination signaling by giving the
> pointer?

Maybe? It might mean more allocations where the use is concerned since
it can't be "static const" that way.

I found the code that needs it later, just that Karthikeyan was using
the wrong explanation for it ... I'd hoped he'd understand your own code
better ;-)

johannes