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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <33e7ca4c7ba947d68d451e919837f6b7@realtek.com>
Date:   Mon, 27 Mar 2023 09:11:56 +0000
From:   Ping-Ke Shih <pkshih@...ltek.com>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>
CC:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        Yan-Hsuan Chuang <tony0620emma@...il.com>,
        Kalle Valo <kvalo@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        Chris Morgan <macroalpha82@...il.com>,
        "Nitin Gupta" <nitin.gupta981@...il.com>,
        Neo Jou <neojou@...il.com>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Larry Finger <Larry.Finger@...inger.net>
Subject: RE: [PATCH v3 2/9] wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets



> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> Sent: Friday, March 24, 2023 3:04 AM
> To: Ping-Ke Shih <pkshih@...ltek.com>
> Cc: linux-wireless@...r.kernel.org; Yan-Hsuan Chuang <tony0620emma@...il.com>; Kalle Valo
> <kvalo@...nel.org>; Ulf Hansson <ulf.hansson@...aro.org>; linux-kernel@...r.kernel.org;
> netdev@...r.kernel.org; linux-mmc@...r.kernel.org; Chris Morgan <macroalpha82@...il.com>; Nitin Gupta
> <nitin.gupta981@...il.com>; Neo Jou <neojou@...il.com>; Jernej Skrabec <jernej.skrabec@...il.com>; Larry
> Finger <Larry.Finger@...inger.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@...glemail.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