Hello Ping-Ke, On Thu, Mar 23, 2023 at 12:16 PM Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> 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? Thank you and best regards, Martin