Received-SPF: pass (google.com: domain of linux-wireless+bounces-3533-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <e50e2336dd2717e7271e7e6f341437637192f717.camel@sipsolutions.net>
Subject: Re: [PATCH 15/15] wifi: cfg80211/mac80211: move puncturing into
 chandef
From: Johannes Berg <johannes@sipsolutions.net>
To: Ping-Ke Shih <pkshih@realtek.com>, "linux-wireless@vger.kernel.org"
	 <linux-wireless@vger.kernel.org>
Date: Tue, 13 Feb 2024 13:41:58 +0100
In-Reply-To: <a17862ae423422b636c76f116e712e7ebed51c09.camel@realtek.com>
References: <20240129184108.49639-17-johannes@sipsolutions.net>
	 <20240129194108.307183a5d2e5.I4d7fe2f126b2366c1312010e2900dfb2abffa0f6@changeid>
	 <f489a916f416cb37b527868c4be0c59547bdeb4d.camel@realtek.com>
	 <94bd67a6f261d945917067334b633c78be665c6b.camel@sipsolutions.net>
	 <a17862ae423422b636c76f116e712e7ebed51c09.camel@realtek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

Hi,

> > The other thing here is that I'm not entirely sure how the driver works=
,
> > chances are that this was previously a bug, and now is still a bug,
> > unless the driver doesn't really support channel contexts, or any form
> > of concurrency.
>=20
> This function is to initialize a station instance in firmware while
> associating, and the field of firmware command is to tell MAC hardware
> the sub-channels it can use to transmit, which should rely on=20
> bitmap of puncturing. Initially, we just wanted the field value to
> be ~0 (0xFFFF) to prevent TX stuck, but not fully implemented puncturing
> feature.
>=20
> I think this is the reason you are confused.

Not sure that explanations helps ;-)

If you have this per station how do you handle CCA? Which was kind of
the reason I moved it all back to the chandef? Not that this didn't make
the code simpler (in mac80211) either as a nice side effect :-)

> Thanks for this hint. I will check my colleagues about the detail of=20
> puncturing behavior next week, because people are offline for the lunar
> new year.=C2=A0

Right.

> I will also check people about the beacon CSA mentioned in
> another discussion thread.

Thanks!

> > Though it _looks_ like you only support one channel context there, so
> > maybe also only one vif, and it doesn't matter? I'd probably still move
> > it over to the chan.c code though, it really does belong there more as
> > discussed in the commit message of this change.
> >=20
> > But I didn't want to make those more semantic changes because I don't
> > know what logic your device applies here.
>=20
> We are going to support MCC and MLO, so we will/must consider more than
> one channel context. Currently, rtw89 just consider 'deflink' not actuall=
y
> 'links' that is the next main work we are doing.=20

For MLO you have just one vif still, so it doesn't matter.

Looks like MCC is something with multi-vif (looking at your other
patchset) so there that makes sense. Not that I know what "MCC" means :)

> > And sorry about the locking bug! Not sure how that happened :(
> >=20
>=20
> I will send a patch to fix locking ahead.=20
>=20

Great, thanks a lot!

johannes