Re: [PATCH rtw-next] wifi: rtw88: sdio: use indirect IO for device registers before power-on

Re: [PATCH rtw-next] wifi: rtw88: sdio: use indirect IO for device registers before power-on

[Andrey Skvortsov]

Hi,

This patch was recently backported to stable kernels (v6.12.58) and it broke
wlan on PinePhone, that uses 8723cs SDIO chip. The same problem
appears of course on latest 6.18-rc6. Reverting this change resolves
the problem.

```
$ sudo dmesg | grep -i rtw88
[   24.940551] rtw88_8723cs mmc1:0001:1: WOW Firmware version 11.0.0, H2C version 0
[   24.953085] rtw88_8723cs mmc1:0001:1: Firmware version 11.0.0, H2C version 0
[   24.955892] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
[   24.973135] rtw88_8723cs mmc1:0001:1: sdio write8 failed (0x1c): -110
[   24.980673] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
...
[   25.446691] rtw88_8723cs mmc1:0001:1: sdio read8 failed (0x100): -110
[   25.453569] rtw88_8723cs mmc1:0001:1: mac power on failed
[   25.459077] rtw88_8723cs mmc1:0001:1: failed to power on mac
[   25.464841] rtw88_8723cs mmc1:0001:1: failed to setup chip efuse info
[   25.464856] rtw88_8723cs mmc1:0001:1: failed to setup chip information
[   25.478341] rtw88_8723cs mmc1:0001:1: probe with driver rtw88_8723cs failed with error -114
```

On 25-07-24 08:48, Ping-Ke Shih wrote:
> The register REG_SYS_CFG1 is used to determine chip basic information
> as arguments of following flows, such as download firmware and load PHY
> parameters, so driver read the value early (before power-on).
> 
> However, the direct IO is disallowed before power-on, or it causes wrong
> values, which driver recognizes a chip as a wrong type RF_1T1R, but
> actually RF_2T2R, causing driver warns:
> 
>   rtw88_8822cs mmc1:0001:1: unsupported rf path (1)
> 
> Fix it by using indirect IO before power-on.
> 
> Reported-by: Piotr Oniszczuk <piotr.oniszczuk@gmail.com>
> Closes: https://lore.kernel.org/linux-wireless/699C22B4-A3E3-4206-97D0-22AB3348EBF6@gmail.com/T/#t
> Suggested-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> ---
>  drivers/net/wireless/realtek/rtw88/sdio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index cc2d4fef3587..99d7c629eac6 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -144,6 +144,10 @@ static u32 rtw_sdio_to_io_address(struct rtw_dev *rtwdev, u32 addr,
>  
>  static bool rtw_sdio_use_direct_io(struct rtw_dev *rtwdev, u32 addr)
>  {
> +	if (!test_bit(RTW_FLAG_POWERON, rtwdev->flags) &&
> +	    !rtw_sdio_is_bus_addr(addr))
> +		return false;
> +
>  	return !rtw_sdio_is_sdio30_supported(rtwdev) ||
>  		rtw_sdio_is_bus_addr(addr);
>  }
> -- 
> 2.25.1
> 

-- 
Best regards,
Andrey Skvortsov

RE: [PATCH rtw-next] wifi: rtw88: sdio: use indirect IO for device registers before power-on

[Ping-Ke Shih]

Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> Hi,
> 
> This patch was recently backported to stable kernels (v6.12.58) and it broke
> wlan on PinePhone, that uses 8723cs SDIO chip. The same problem
> appears of course on latest 6.18-rc6. Reverting this change resolves
> the problem.
> 
> ```
> $ sudo dmesg | grep -i rtw88
> [   24.940551] rtw88_8723cs mmc1:0001:1: WOW Firmware version 11.0.0, H2C version 0
> [   24.953085] rtw88_8723cs mmc1:0001:1: Firmware version 11.0.0, H2C version 0
> [   24.955892] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
> [   24.973135] rtw88_8723cs mmc1:0001:1: sdio write8 failed (0x1c): -110
> [   24.980673] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
> ...
> [   25.446691] rtw88_8723cs mmc1:0001:1: sdio read8 failed (0x100): -110
> [   25.453569] rtw88_8723cs mmc1:0001:1: mac power on failed
> [   25.459077] rtw88_8723cs mmc1:0001:1: failed to power on mac
> [   25.464841] rtw88_8723cs mmc1:0001:1: failed to setup chip efuse info
> [   25.464856] rtw88_8723cs mmc1:0001:1: failed to setup chip information
> [   25.478341] rtw88_8723cs mmc1:0001:1: probe with driver rtw88_8723cs failed with error -114
> ```
> 

Check original link of this patch [1] that 8822cs read incorrect from 0xf0
resulting in "rtw88_8822cs mmc1:0001:1: unsupported rf path (1)".

I wonder if we can add additional checking rule of chip ID, like:

--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -144,8 +144,10 @@ static u32 rtw_sdio_to_io_address(struct rtw_dev *rtwdev, u32 addr,

 static bool rtw_sdio_use_direct_io(struct rtw_dev *rtwdev, u32 addr)
 {
+       bool might_indirect_under_power_off = rtwdev->chip->id != RTW_CHIP_TYPE_8703B;
+
        if (!test_bit(RTW_FLAG_POWERON, rtwdev->flags) &&
-           !rtw_sdio_is_bus_addr(addr))
+           !rtw_sdio_is_bus_addr(addr) && might_indirect_under_power_off)
                return false;

        return !rtw_sdio_is_sdio30_supported(rtwdev) ||

[1] https://lore.kernel.org/linux-wireless/699C22B4-A3E3-4206-97D0-22AB3348EBF6@gmail.com/T/#t

Re: [PATCH rtw-next] wifi: rtw88: sdio: use indirect IO for device registers before power-on

[Andrey Skvortsov]

On 25-11-24 08:16, Ping-Ke Shih wrote:
> Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> > Hi,
> > 
> > This patch was recently backported to stable kernels (v6.12.58) and it broke
> > wlan on PinePhone, that uses 8723cs SDIO chip. The same problem
> > appears of course on latest 6.18-rc6. Reverting this change resolves
> > the problem.
> > 
> > ```
> > $ sudo dmesg | grep -i rtw88
> > [   24.940551] rtw88_8723cs mmc1:0001:1: WOW Firmware version 11.0.0, H2C version 0
> > [   24.953085] rtw88_8723cs mmc1:0001:1: Firmware version 11.0.0, H2C version 0
> > [   24.955892] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
I see it fails with timeout on the first operation to read chip version.

> > [   24.973135] rtw88_8723cs mmc1:0001:1: sdio write8 failed (0x1c): -110
> > [   24.980673] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
> > ...
> > [   25.446691] rtw88_8723cs mmc1:0001:1: sdio read8 failed (0x100): -110
> > [   25.453569] rtw88_8723cs mmc1:0001:1: mac power on failed
> > [   25.459077] rtw88_8723cs mmc1:0001:1: failed to power on mac
> > [   25.464841] rtw88_8723cs mmc1:0001:1: failed to setup chip efuse info
> > [   25.464856] rtw88_8723cs mmc1:0001:1: failed to setup chip information
> > [   25.478341] rtw88_8723cs mmc1:0001:1: probe with driver rtw88_8723cs failed with error -114
> > ```
> > 
> 
> Check original link of this patch [1] that 8822cs read incorrect from 0xf0
> resulting in "rtw88_8822cs mmc1:0001:1: unsupported rf path (1)".
Sorry, I haven't meant, that this patch should be reverted. I see,
that it solves real problem. I've meant, that patch caused this
regression was identified and it was confirmed by reverting it.

> I wonder if we can add additional checking rule of chip ID, like:

I've tried suggested change and wlan is recognized again.

> 
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -144,8 +144,10 @@ static u32 rtw_sdio_to_io_address(struct rtw_dev *rtwdev, u32 addr,
> 
>  static bool rtw_sdio_use_direct_io(struct rtw_dev *rtwdev, u32 addr)
>  {
> +       bool might_indirect_under_power_off = rtwdev->chip->id != RTW_CHIP_TYPE_8703B;
> +
>         if (!test_bit(RTW_FLAG_POWERON, rtwdev->flags) &&
> -           !rtw_sdio_is_bus_addr(addr))
> +           !rtw_sdio_is_bus_addr(addr) && might_indirect_under_power_off)
>                 return false;
> 
>         return !rtw_sdio_is_sdio30_supported(rtwdev) ||
> 
> [1] https://lore.kernel.org/linux-wireless/699C22B4-A3E3-4206-97D0-22AB3348EBF6@gmail.com/T/#t
> 

-- 
Best regards,
Andrey Skvortsov

RE: [PATCH rtw-next] wifi: rtw88: sdio: use indirect IO for device registers before power-on

[Ping-Ke Shih]

Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> On 25-11-24 08:16, Ping-Ke Shih wrote:
> > Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> > > Hi,
> > >
> > > This patch was recently backported to stable kernels (v6.12.58) and it broke
> > > wlan on PinePhone, that uses 8723cs SDIO chip. The same problem
> > > appears of course on latest 6.18-rc6. Reverting this change resolves
> > > the problem.
> > >
> > > ```
> > > $ sudo dmesg | grep -i rtw88
> > > [   24.940551] rtw88_8723cs mmc1:0001:1: WOW Firmware version 11.0.0, H2C version 0
> > > [   24.953085] rtw88_8723cs mmc1:0001:1: Firmware version 11.0.0, H2C version 0
> > > [   24.955892] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
> I see it fails with timeout on the first operation to read chip version.
> 
> > > [   24.973135] rtw88_8723cs mmc1:0001:1: sdio write8 failed (0x1c): -110
> > > [   24.980673] rtw88_8723cs mmc1:0001:1: sdio read32 failed (0xf0): -110
> > > ...
> > > [   25.446691] rtw88_8723cs mmc1:0001:1: sdio read8 failed (0x100): -110
> > > [   25.453569] rtw88_8723cs mmc1:0001:1: mac power on failed
> > > [   25.459077] rtw88_8723cs mmc1:0001:1: failed to power on mac
> > > [   25.464841] rtw88_8723cs mmc1:0001:1: failed to setup chip efuse info
> > > [   25.464856] rtw88_8723cs mmc1:0001:1: failed to setup chip information
> > > [   25.478341] rtw88_8723cs mmc1:0001:1: probe with driver rtw88_8723cs failed with error -114
> > > ```
> > >
> >
> > Check original link of this patch [1] that 8822cs read incorrect from 0xf0
> > resulting in "rtw88_8822cs mmc1:0001:1: unsupported rf path (1)".
> Sorry, I haven't meant, that this patch should be reverted. I see,
> that it solves real problem. I've meant, that patch caused this
> regression was identified and it was confirmed by reverting it.

I just want to point out the original problem we encountered, and the user
weirdly get unexpected value from 0xF0 without any error. 

> 
> > I wonder if we can add additional checking rule of chip ID, like:
> 
> I've tried suggested change and wlan is recognized again.
> 

I will check if vendor driver does similar things, and then I can make
a formal patch for 8723CS.