RE: [PATCH v3 2/9] wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets

[Ping-Ke Shih <pkshih@realtek.com>]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Friday, March 24, 2023 3:04 AM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: linux-wireless@vger.kernel.org; Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo
> <kvalo@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; linux-mmc@vger.kernel.org; Chris Morgan <macroalpha82@gmail.com>; Nitin Gupta
> <nitin.gupta981@gmail.com>; Neo Jou <neojou@gmail.com>; Jernej Skrabec <jernej.skrabec@gmail.com>; Larry
> Finger <Larry.Finger@lwfinger.net>
> Subject: Re: [PATCH v3 2/9] wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets
> 
> Hello Ping-Ke,
> 
> On Thu, Mar 23, 2023 at 12:16 PM Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> [...]
> > > > +       if (direct) {
> > > > +               addr = rtw_sdio_to_bus_offset(rtwdev, addr);
> > > > +               val = rtw_sdio_readl(rtwdev, addr, &ret);
> > > > +       } else if (addr & 3) {
> > >
> > > else if (IS_ALIGNED(addr, 4) {
> > I'll add these IS_ALIGNED in v4
> > Also I found an issue with RTW_WCPU_11N devices where indirect read
> > works differently (those can't use
> > REG_SDIO_INDIRECT_REG_CFG/REG_SDIO_INDIRECT_REG_DATA but need to go
> > through the normal path with WLAN_IOREG_OFFSET instead). I'll also
> > include that fix in v4
> I have a question about the "indirect" handling.
> Let me start with what I know:
> - REG_SDIO_INDIRECT_REG_CFG and REG_SDIO_INDIRECT_REG_DATA are only
> present on RTW_WCPU_11AC based chips (older RTW_WCPU_11N chips don't
> have these registers)
> - the name of REG_SDIO_INDIRECT_REG_CFG[20] is not known but we're
> polling that bit to check if REG_SDIO_INDIRECT_REG_DATA is ready to be
> read or has data from REG_SDIO_INDIRECT_REG_DATA has been written
> - REG_SDIO_INDIRECT_REG_CFG[19] configures a read operation
> - REG_SDIO_INDIRECT_REG_CFG[18] configures a write operation
> - REG_SDIO_INDIRECT_REG_CFG[17] indicates that a DWORD (32-bit) are
> written to REG_SDIO_INDIRECT_REG_DATA (+ the following 3), this bit
> seems irrelevant for read mode
> - REG_SDIO_INDIRECT_REG_CFG[16] indicates that a DWORD (16-bit) are
> written to REG_SDIO_INDIRECT_REG_DATA (+ the following 3), this bit
> seems irrelevant for read mode
> - RTW_WCPU_11N chips are simply using "addr | WLAN_IOREG_OFFSET" for
> accesses that would usually be "indirect" reads/writes on
> RTW_WCPU_11AC chips
> 
> While fixing the issue for the RTW_WCPU_11N chips I discovered that
> the "old" approach for indirect register access (without
> REG_SDIO_INDIRECT_REG_CFG and REG_SDIO_INDIRECT_REG_DATA) also works
> on RTW_WCPU_11AC chips.
> (I'm calling it the "old" approach because it's what the RTL8723DS an
> RTL8723BS vendor drivers use)
> In fact, this series is using the "old" approach for writes, but the
> new (REG_SDIO_INDIRECT_REG_CFG and REG_SDIO_INDIRECT_REG_DATA based)
> approach for reads.
> Naturally I'm curious as to why two different approaches achieve the
> same goal. Using the "old" approach (addr | WLAN_IOREG_OFFSET) means a
> lot of code could be deleted/simplified.
> 
> Now my question:
> Do you have any explanation (either from internal documentation or
> from the hardware/firmware teams) if and when the
> REG_SDIO_INDIRECT_REG_CFG and REG_SDIO_INDIRECT_REG_DATA registers
> should be used on RTW_WCPU_11AC chips?
> 

Using REG_SDIO_INDIRECT_REG_CFG and REG_SDIO_INDIRECT_REG_DATA if you are
using SDIO 3.0; otherwise, it could causes IO abnormal. Oppositely, using
"old" approach (addr | WLAN_IOREG_OFFSET) for SDIO 2.0. 

Ping-Ke