Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAFBinCDMPPJ7qW7xTkep1Trg+zP0B9Jxei6sgjqmF4NDA1JAhQ@mail.gmail.com>
 <3c61fae611294e5098e6e0044a7a4199@realtek.com>
In-Reply-To: <3c61fae611294e5098e6e0044a7a4199@realtek.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Thu, 15 Jul 2021 00:47:23 +0200
Message-ID: <CAFBinCA4mDrBCjcNazW_mUW9NkC9sq-AULFJEh7z3Aj5oCyrxQ@mail.gmail.com>
Subject: Re: rtw88: rtw_{read,write}_rf locking questions
To:     Pkshih <pkshih@realtek.com>
Cc:     Yan-Hsuan Chuang <tony0620emma@gmail.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Neo Jou <neojou@gmail.com>,
        Jernej Skrabec <jernej.skrabec@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello Ping-Ke,

On Wed, Jul 14, 2021 at 3:48 AM Pkshih <pkshih@realtek.com> wrote:
>
>
> > -----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.

> > 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