Received-SPF: pass (google.com: domain of linux-wireless+bounces-6073-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <902dd36fa5ab0503377e558b92505fe499f666fa.camel@sipsolutions.net>
Subject: Re: [PATCH 02/13] wifi: nl80211: send underlying multi-hardware
 channel capabilities to user space
From: Johannes Berg <johannes@sipsolutions.net>
To: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>, Karthikeyan
	Periyasamy <quic_periyasa@quicinc.com>, ath12k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org, Jakub Kicinski
	 <kuba@kernel.org>
Date: Wed, 10 Apr 2024 09:59:02 +0200
In-Reply-To: <b455f267-9552-be3b-95b0-a036bfa8e14a@quicinc.com>
References: <20240328072916.1164195-1-quic_periyasa@quicinc.com>
	 <20240328072916.1164195-3-quic_periyasa@quicinc.com>
	 <6d92d0ba4a8764cd91cc20c4bd35613bcc41dfcd.camel@sipsolutions.net>
	 <9d5c2f9f-19b5-af4d-8018-1eb97fac10d6@quicinc.com>
	 <9d0f309da45ae657cd2ce0bc11a93d66e856ef64.camel@sipsolutions.net>
	 <b455f267-9552-be3b-95b0-a036bfa8e14a@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Fri, 2024-03-29 at 19:51 +0530, Vasanthakumar Thiagarajan wrote:
>=20
> 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. Internall=
y 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?
> > >=20
> > > Hostapd use this hw index for the channel switch cmd.
> >=20
> > What, where? I don't see that in this patchset? And why??
> >=20
> > In any case, unless I just missed it and you're going to tell me to loo=
k
> > at patch N, we don't need it here, and then I'd prefer to keep it an
> > internal detail until it's needed.
> >=20
> > > The hw index used as a sanity check to identify whether the user
> > > requested channel fall under the different hw or not.
> >=20
> > You mean within hostapd itself? That doesn't make sense, it can derive
> > that information differently.
> >=20
> > > In split-phy hardware, 5GHz band supported by two physical hardware's=
.
> > > First supports 5GHz Low band and second supports 5GHz High band.
> >=20
> > In your hardware design anyway, but yeah, I get it.
> >=20
> > > In this case, user space cannot use band vise check here to identify
> > > given channel or freq supported in the given hardware.
> >=20
> > No, but it also doesn't need an index assigned by the kernel for that.
> >=20
>=20
> The only purpose of hw index is to link hw_chans to per-hardware=20
> 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