[PATCH] wifi: rtw88: fix race condition when doing H2C command

[PATCH] wifi: rtw88: fix race condition when doing H2C command

[Ping-Ke Shih]

From: Ji-Pin Jou <neo_jou@realtek.com>

For SDIO/USB interface, since the tranferring speed is slower than
that in PCIE, it may have race condition when the driver sets down
H2C command to the FW.

In the function rtw_fw_send_h2c_command, before the patch, box_reg
is written first, then box_ex_reg is written. FW starts to work and
fetch the value of box_ex_reg,  when the most significant byte of
box_reg(4 bytes) is written. Meanwhile, for SDIO/USB interface,
since the transferring speed is slow, the driver is still in writing
the new value of box_ex_reg through the bus, and FW may get the
wrong value of box_ex_reg at the moment.

To prevent the above driver/FW racing situation, box_ex_reg is
written first then box_reg. Furthermore, it is written in 4 bytes at
a time, instead of written in one byte one by one. It can increase
the speed for SDIO/USB interface.

Signed-off-by: Ji-Pin Jou <neo_jou@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 8 +++-----
 drivers/net/wireless/realtek/rtw88/fw.h | 5 +++++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 0b5f903c0f366..b290811d75e1c 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -311,10 +311,10 @@ EXPORT_SYMBOL(rtw_fw_c2h_cmd_isr);
 static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
 				    u8 *h2c)
 {
+	struct rtw_h2c_cmd *h2c_cmd = (struct rtw_h2c_cmd *)h2c;
 	u8 box;
 	u8 box_state;
 	u32 box_reg, box_ex_reg;
-	int idx;
 	int ret;
 
 	rtw_dbg(rtwdev, RTW_DBG_FW,
@@ -356,10 +356,8 @@ static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
 		goto out;
 	}
 
-	for (idx = 0; idx < 4; idx++)
-		rtw_write8(rtwdev, box_reg + idx, h2c[idx]);
-	for (idx = 0; idx < 4; idx++)
-		rtw_write8(rtwdev, box_ex_reg + idx, h2c[idx + 4]);
+	rtw_write32(rtwdev, box_ex_reg, le32_to_cpu(h2c_cmd->msg_ext));
+	rtw_write32(rtwdev, box_reg, le32_to_cpu(h2c_cmd->msg));
 
 	if (++rtwdev->h2c.last_box_num >= 4)
 		rtwdev->h2c.last_box_num = 0;
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index a5a965803a3cc..bca610dc99ab7 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -81,6 +81,11 @@ struct rtw_c2h_adaptivity {
 	u8 option;
 } __packed;
 
+struct rtw_h2c_cmd {
+	__le32 msg;
+	__le32 msg_ext;
+} __packed;
+
 enum rtw_rsvd_packet_type {
 	RSVD_BEACON,
 	RSVD_DUMMY,
-- 
2.25.1

Re: [PATCH] wifi: rtw88: fix race condition when doing H2C command

[Sascha Hauer]

On Thu, Nov 24, 2022 at 02:44:42PM +0800, Ping-Ke Shih wrote:
> From: Ji-Pin Jou <neo_jou@realtek.com>
> 
> For SDIO/USB interface, since the tranferring speed is slower than
> that in PCIE, it may have race condition when the driver sets down
> H2C command to the FW.
> 
> In the function rtw_fw_send_h2c_command, before the patch, box_reg
> is written first, then box_ex_reg is written. FW starts to work and
> fetch the value of box_ex_reg,  when the most significant byte of
> box_reg(4 bytes) is written. Meanwhile, for SDIO/USB interface,
> since the transferring speed is slow, the driver is still in writing
> the new value of box_ex_reg through the bus, and FW may get the
> wrong value of box_ex_reg at the moment.
> 
> To prevent the above driver/FW racing situation, box_ex_reg is
> written first then box_reg. Furthermore, it is written in 4 bytes at
> a time, instead of written in one byte one by one. It can increase
> the speed for SDIO/USB interface.
> 
> Signed-off-by: Ji-Pin Jou <neo_jou@realtek.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

This solves the issue with the RTW88 USB patch set reported here:

https://lore.kernel.org/linux-wireless/20221122145527.GA29978@pengutronix.de/

Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
>  drivers/net/wireless/realtek/rtw88/fw.c | 8 +++-----
>  drivers/net/wireless/realtek/rtw88/fw.h | 5 +++++
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
> index 0b5f903c0f366..b290811d75e1c 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> @@ -311,10 +311,10 @@ EXPORT_SYMBOL(rtw_fw_c2h_cmd_isr);
>  static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
>  				    u8 *h2c)
>  {
> +	struct rtw_h2c_cmd *h2c_cmd = (struct rtw_h2c_cmd *)h2c;
>  	u8 box;
>  	u8 box_state;
>  	u32 box_reg, box_ex_reg;
> -	int idx;
>  	int ret;
>  
>  	rtw_dbg(rtwdev, RTW_DBG_FW,
> @@ -356,10 +356,8 @@ static void rtw_fw_send_h2c_command(struct rtw_dev *rtwdev,
>  		goto out;
>  	}
>  
> -	for (idx = 0; idx < 4; idx++)
> -		rtw_write8(rtwdev, box_reg + idx, h2c[idx]);
> -	for (idx = 0; idx < 4; idx++)
> -		rtw_write8(rtwdev, box_ex_reg + idx, h2c[idx + 4]);
> +	rtw_write32(rtwdev, box_ex_reg, le32_to_cpu(h2c_cmd->msg_ext));
> +	rtw_write32(rtwdev, box_reg, le32_to_cpu(h2c_cmd->msg));
>  
>  	if (++rtwdev->h2c.last_box_num >= 4)
>  		rtwdev->h2c.last_box_num = 0;
> diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
> index a5a965803a3cc..bca610dc99ab7 100644
> --- a/drivers/net/wireless/realtek/rtw88/fw.h
> +++ b/drivers/net/wireless/realtek/rtw88/fw.h
> @@ -81,6 +81,11 @@ struct rtw_c2h_adaptivity {
>  	u8 option;
>  } __packed;
>  
> +struct rtw_h2c_cmd {
> +	__le32 msg;
> +	__le32 msg_ext;
> +} __packed;
> +
>  enum rtw_rsvd_packet_type {
>  	RSVD_BEACON,
>  	RSVD_DUMMY,
> -- 
> 2.25.1
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] wifi: rtw88: fix race condition when doing H2C command

[Kalle Valo]

Ping-Ke Shih <pkshih@realtek.com> wrote:

> From: Ji-Pin Jou <neo_jou@realtek.com>
> 
> For SDIO/USB interface, since the tranferring speed is slower than
> that in PCIE, it may have race condition when the driver sets down
> H2C command to the FW.
> 
> In the function rtw_fw_send_h2c_command, before the patch, box_reg
> is written first, then box_ex_reg is written. FW starts to work and
> fetch the value of box_ex_reg,  when the most significant byte of
> box_reg(4 bytes) is written. Meanwhile, for SDIO/USB interface,
> since the transferring speed is slow, the driver is still in writing
> the new value of box_ex_reg through the bus, and FW may get the
> wrong value of box_ex_reg at the moment.
> 
> To prevent the above driver/FW racing situation, box_ex_reg is
> written first then box_reg. Furthermore, it is written in 4 bytes at
> a time, instead of written in one byte one by one. It can increase
> the speed for SDIO/USB interface.
> 
> Signed-off-by: Ji-Pin Jou <neo_jou@realtek.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>

Patch applied to wireless-next.git, thanks.

823092a53556 wifi: rtw88: fix race condition when doing H2C command

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20221124064442.28042-1-pkshih@realtek.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches