Hello Ping-Ke, On Wed, Jul 14, 2021 at 3:48 AM Pkshih <pkshih@xxxxxxxxxxx> wrote: > > > > -----Original Message----- > > From: Martin Blumenstingl [mailto:martin.blumenstingl@xxxxxxxxxxxxxx] > > Sent: Wednesday, July 14, 2021 12:51 AM > > To: Yan-Hsuan Chuang; Pkshih; Tzu-En Huang > > Cc: linux-wireless@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; 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. > > 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). > > 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
