Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230320213508.2358213-1-martin.blumenstingl@googlemail.com>
 <20230320213508.2358213-3-martin.blumenstingl@googlemail.com>
 <f7b9dda9d852456caffc3c0572f88947@realtek.com> <CAFBinCCspK=GaCMEiHsXi=0H4Sbp2vg_4EK=8bqQLWR8+qg7Sw@mail.gmail.com>
In-Reply-To: <CAFBinCCspK=GaCMEiHsXi=0H4Sbp2vg_4EK=8bqQLWR8+qg7Sw@mail.gmail.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Thu, 23 Mar 2023 20:03:50 +0100
Message-ID: <CAFBinCAxuEyNkUxsqJ9wVxXupErcp33JCFsJ2hDupWj9MRMYGA@mail.gmail.com>
Subject: Re: [PATCH v3 2/9] wifi: rtw88: sdio: Add HCI implementation for SDIO
 based chipsets
To:     Ping-Ke Shih <pkshih@realtek.com>
Cc:     "linux-wireless@vger.kernel.org" <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" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-mmc@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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello Ping-Ke,

On Thu, Mar 23, 2023 at 12:16=E2=80=AFPM Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
[...]
> > > +       if (direct) {
> > > +               addr =3D rtw_sdio_to_bus_offset(rtwdev, addr);
> > > +               val =3D 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?


Thank you and best regards,
Martin