Re: rtw88: rtw_{read,write}_rf locking questions

Hello Ping-Ke,

On Wed, Jul 14, 2021 at 3:48 AM Pkshih [off-list ref] wrote:

quoted

-----Original Message-----
From: Martin Blumenstingl [mailto:martin.blumenstingl@googlemail.com]
Sent: Wednesday, July 14, 2021 12:51 AM
To: Yan-Hsuan Chuang; Pkshih; Tzu-En Huang
Cc: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Neo Jou;
Jernej Skrabec
Subject: rtw88: rtw_{read,write}_rf locking questions

Hello rtw88 maintainers and contributors,

there is an ongoing effort where Jernej and I are working on adding
SDIO support to the rtw88 driver.
The hardware we use at the moment is RTL8822BS and RTL8822CS.
Work-in-progress code can be found in Jernej's repo (note: this may be
rebased): [0]

Thanks for your nice work!

A quick update: we got scanning and authentication to work.

quoted

We are at a point where we can communicate with the SDIO card and
successfully upload the firmware to it.
Right now I have two questions about the locking in
rtw_{read,write}_rf from hci.h:
1) A spinlock is used to protect RF register access. This is
problematic for SDIO, more information below. Would you accept a patch
to convert this into a mutex? I don't have any rtw88 PCIe card for
testing any regressions there myself.

I think it's okay.

Great, thanks for confirming this!
I'll send a series of patches with locking preparations (patches which
add SDIO support will come later as we're still trying to narrow down
a few issues).

quoted

2) I would like to understand why the RF register access needs to be
protected by a lock. From what I can tell RF register access doesn't
seem to be used from IRQ handlers.

The use of lock isn't because we want to access the RF register in IRQ
handlers. The reasons are
1. The ieee80211 iterative vif function we use is atomic type, so we can't
   use mutex.
   Do you change the type of iterative function?

yes, that is part of the "locking preparation" patches I mentioned above

2. RF register access isn't an atomic. If more than one threads access the
   register at the same time, the value will be wrong.

Understood, thanks for pointing this out.


Best regards,
Martin