[PATCH v2 0/3] wifi: rtw88: USB fixes

[PATCH v2 0/3] wifi: rtw88: USB fixes

[Sascha Hauer]

This series addresses issues for the recently added RTW88 USB support
reported by Andreas Henriksson and also our customer.

The hardware can't handle urbs that have a size of multiple of the
bulkout_size (usually 512 bytes). The symptom is that the hardware
stalls completely. The issue can be reproduced by sending a suitably
sized ping packet from the device:

ping -s 394 <somehost>

(It's 394 bytes here on a RTL8822CU and RTL8821CU, the actual size may
differ on other chips, it was 402 bytes on a RTL8723DU)

Other than that qsel was not set correctly. The sympton here is that
only one of multiple bulk endpoints was used to send data.

Changes since v1:
- Use URB_ZERO_PACKET to let the USB host controller handle it automatically
  rather than working around the issue.

Sascha Hauer (3):
  wifi: rtw88: usb: Set qsel correctly
  wifi: rtw88: usb: send Zero length packets if necessary
  wifi: rtw88: usb: drop now unnecessary URB size check

 drivers/net/wireless/realtek/rtw88/usb.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

-- 
2.30.2

[PATCH v2 1/3] wifi: rtw88: usb: Set qsel correctly

[Sascha Hauer]

We have to extract qsel from the skb before doing skb_push() on it,
otherwise qsel will always be 0.

Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 4ef38279b64c9..d9e995544e405 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -471,9 +471,9 @@ static int rtw_usb_tx_write(struct rtw_dev *rtwdev,
 	u8 *pkt_desc;
 	int ep;
 
+	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(skb);
 	pkt_desc = skb_push(skb, chip->tx_pkt_desc_sz);
 	memset(pkt_desc, 0, chip->tx_pkt_desc_sz);
-	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(skb);
 	ep = qsel_to_ep(rtwusb, pkt_info->qsel);
 	rtw_tx_fill_tx_desc(pkt_info, skb);
 	rtw_tx_fill_txdesc_checksum(rtwdev, pkt_info, skb->data);
-- 
2.30.2

RE: [PATCH v2 1/3] wifi: rtw88: usb: Set qsel correctly

[Ping-Ke Shih]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Friday, February 10, 2023 7:17 PM
> To: linux-wireless@vger.kernel.org
> Cc: Neo Jou <neojou@gmail.com>; Hans Ulli Kroll <linux@ulli-kroll.de>; Ping-Ke Shih <pkshih@realtek.com>;
> Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo <kvalo@kernel.org>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Martin Blumenstingl <martin.blumenstingl@googlemail.com>;
> kernel@pengutronix.de; Alexander Hochbaum <alex@appudo.com>; Da Xue <da@libre.computer>; Bernie Huang
> <phhuang@realtek.com>; Andreas Henriksson <andreas@fatal.se>; Viktor Petrenko <g0000ga@gmail.com>; Sascha
> Hauer <s.hauer@pengutronix.de>
> Subject: [PATCH v2 1/3] wifi: rtw88: usb: Set qsel correctly
> 
> We have to extract qsel from the skb before doing skb_push() on it,
> otherwise qsel will always be 0.
> 
> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

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

> ---
>  drivers/net/wireless/realtek/rtw88/usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 4ef38279b64c9..d9e995544e405 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -471,9 +471,9 @@ static int rtw_usb_tx_write(struct rtw_dev *rtwdev,
>  	u8 *pkt_desc;
>  	int ep;
> 
> +	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(skb);
>  	pkt_desc = skb_push(skb, chip->tx_pkt_desc_sz);
>  	memset(pkt_desc, 0, chip->tx_pkt_desc_sz);
> -	pkt_info->qsel = rtw_usb_tx_queue_mapping_to_qsel(skb);
>  	ep = qsel_to_ep(rtwusb, pkt_info->qsel);
>  	rtw_tx_fill_tx_desc(pkt_info, skb);
>  	rtw_tx_fill_txdesc_checksum(rtwdev, pkt_info, skb->data);
> --
> 2.30.2

Re: [PATCH v2 1/3] wifi: rtw88: usb: Set qsel correctly

[Kalle Valo]

Sascha Hauer <s.hauer@pengutronix.de> wrote:

> We have to extract qsel from the skb before doing skb_push() on it,
> otherwise qsel will always be 0.
> 
> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

3 patches applied to wireless-next.git, thanks.

7869b834fb07 wifi: rtw88: usb: Set qsel correctly
07ce9fa6ab0e wifi: rtw88: usb: send Zero length packets if necessary
462c8db6a011 wifi: rtw88: usb: drop now unnecessary URB size check

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230210111632.1985205-2-s.hauer@pengutronix.de/

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

[PATCH v2 2/3] wifi: rtw88: usb: send Zero length packets if necessary

[Sascha Hauer]

Zero length packets are necessary when sending URBs with size
multiple of bulkout_size, otherwise the hardware just stalls.

Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index d9e995544e405..1a09c9288198a 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -271,6 +271,7 @@ static int rtw_usb_write_port(struct rtw_dev *rtwdev, u8 qsel, struct sk_buff *s
 		return -ENOMEM;
 
 	usb_fill_bulk_urb(urb, usbd, pipe, skb->data, skb->len, cb, context);
+	urb->transfer_flags |= URB_ZERO_PACKET;
 	ret = usb_submit_urb(urb, GFP_ATOMIC);
 
 	usb_free_urb(urb);
-- 
2.30.2

RE: [PATCH v2 2/3] wifi: rtw88: usb: send Zero length packets if necessary

[Ping-Ke Shih]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Friday, February 10, 2023 7:17 PM
> To: linux-wireless@vger.kernel.org
> Cc: Neo Jou <neojou@gmail.com>; Hans Ulli Kroll <linux@ulli-kroll.de>; Ping-Ke Shih <pkshih@realtek.com>;
> Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo <kvalo@kernel.org>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Martin Blumenstingl <martin.blumenstingl@googlemail.com>;
> kernel@pengutronix.de; Alexander Hochbaum <alex@appudo.com>; Da Xue <da@libre.computer>; Bernie Huang
> <phhuang@realtek.com>; Andreas Henriksson <andreas@fatal.se>; Viktor Petrenko <g0000ga@gmail.com>; Sascha
> Hauer <s.hauer@pengutronix.de>
> Subject: [PATCH v2 2/3] wifi: rtw88: usb: send Zero length packets if necessary
> 
> Zero length packets are necessary when sending URBs with size
> multiple of bulkout_size, otherwise the hardware just stalls.
> 
> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

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

> ---
>  drivers/net/wireless/realtek/rtw88/usb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index d9e995544e405..1a09c9288198a 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -271,6 +271,7 @@ static int rtw_usb_write_port(struct rtw_dev *rtwdev, u8 qsel, struct sk_buff *s
>  		return -ENOMEM;
> 
>  	usb_fill_bulk_urb(urb, usbd, pipe, skb->data, skb->len, cb, context);
> +	urb->transfer_flags |= URB_ZERO_PACKET;
>  	ret = usb_submit_urb(urb, GFP_ATOMIC);
> 
>  	usb_free_urb(urb);
> --
> 2.30.2

[PATCH v2 3/3] wifi: rtw88: usb: drop now unnecessary URB size check

[Sascha Hauer]

Now that we send URBs with the URB_ZERO_PACKET flag set we no longer
need to make sure that the URB sizes are not multiple of the
bulkout_size. Drop the check.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 1a09c9288198a..2a8336b1847a5 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -414,24 +414,11 @@ static int rtw_usb_write_data_rsvd_page(struct rtw_dev *rtwdev, u8 *buf,
 					u32 size)
 {
 	const struct rtw_chip_info *chip = rtwdev->chip;
-	struct rtw_usb *rtwusb;
 	struct rtw_tx_pkt_info pkt_info = {0};
-	u32 len, desclen;
-
-	rtwusb = rtw_get_usb_priv(rtwdev);
 
 	pkt_info.tx_pkt_size = size;
 	pkt_info.qsel = TX_DESC_QSEL_BEACON;
-
-	desclen = chip->tx_pkt_desc_sz;
-	len = desclen + size;
-	if (len % rtwusb->bulkout_size == 0) {
-		len += RTW_USB_PACKET_OFFSET_SZ;
-		pkt_info.offset = desclen + RTW_USB_PACKET_OFFSET_SZ;
-		pkt_info.pkt_offset = 1;
-	} else {
-		pkt_info.offset = desclen;
-	}
+	pkt_info.offset = chip->tx_pkt_desc_sz;
 
 	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
 }
-- 
2.30.2

RE: [PATCH v2 3/3] wifi: rtw88: usb: drop now unnecessary URB size check

[Ping-Ke Shih]

> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Friday, February 10, 2023 7:17 PM
> To: linux-wireless@vger.kernel.org
> Cc: Neo Jou <neojou@gmail.com>; Hans Ulli Kroll <linux@ulli-kroll.de>; Ping-Ke Shih <pkshih@realtek.com>;
> Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo <kvalo@kernel.org>; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Martin Blumenstingl <martin.blumenstingl@googlemail.com>;
> kernel@pengutronix.de; Alexander Hochbaum <alex@appudo.com>; Da Xue <da@libre.computer>; Bernie Huang
> <phhuang@realtek.com>; Andreas Henriksson <andreas@fatal.se>; Viktor Petrenko <g0000ga@gmail.com>; Sascha
> Hauer <s.hauer@pengutronix.de>
> Subject: [PATCH v2 3/3] wifi: rtw88: usb: drop now unnecessary URB size check
> 
> Now that we send URBs with the URB_ZERO_PACKET flag set we no longer
> need to make sure that the URB sizes are not multiple of the
> bulkout_size. Drop the check.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

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

> ---
>  drivers/net/wireless/realtek/rtw88/usb.c | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 1a09c9288198a..2a8336b1847a5 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -414,24 +414,11 @@ static int rtw_usb_write_data_rsvd_page(struct rtw_dev *rtwdev, u8 *buf,
>  					u32 size)
>  {
>  	const struct rtw_chip_info *chip = rtwdev->chip;
> -	struct rtw_usb *rtwusb;
>  	struct rtw_tx_pkt_info pkt_info = {0};
> -	u32 len, desclen;
> -
> -	rtwusb = rtw_get_usb_priv(rtwdev);
> 
>  	pkt_info.tx_pkt_size = size;
>  	pkt_info.qsel = TX_DESC_QSEL_BEACON;
> -
> -	desclen = chip->tx_pkt_desc_sz;
> -	len = desclen + size;
> -	if (len % rtwusb->bulkout_size == 0) {
> -		len += RTW_USB_PACKET_OFFSET_SZ;
> -		pkt_info.offset = desclen + RTW_USB_PACKET_OFFSET_SZ;
> -		pkt_info.pkt_offset = 1;
> -	} else {
> -		pkt_info.offset = desclen;
> -	}
> +	pkt_info.offset = chip->tx_pkt_desc_sz;
> 
>  	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
>  }
> --
> 2.30.2

Re: [PATCH v2 0/3] wifi: rtw88: USB fixes

[Andreas Henriksson]

Hello Sacha Hauer,

Thanks alot for fixing this!

On Fri, Feb 10, 2023 at 12:16:29PM +0100, Sascha Hauer wrote:
> This series addresses issues for the recently added RTW88 USB support
> reported by Andreas Henriksson and also our customer.
> 
> The hardware can't handle urbs that have a size of multiple of the
> bulkout_size (usually 512 bytes). The symptom is that the hardware
> stalls completely. The issue can be reproduced by sending a suitably
> sized ping packet from the device:
> 
> ping -s 394 <somehost>
> 
> (It's 394 bytes here on a RTL8822CU and RTL8821CU, the actual size may
> differ on other chips, it was 402 bytes on a RTL8723DU)

I can confirm that with these patches applied that my LM842 dongle
now works reliably on my imx6sx board. On the same board the traffic
would previously usually stall after 80-130MB when downloading.

With patches applied I succesfully completed:
wget -O /dev/null http://speedtest.tele2.net/10GB.zip

Uploading did not seem to trigger the problem before but I still
tested and uploading a gigabyte was no problem using:
curl -T /dev/urandom http://speedtest.tele2.net/upload.php  -O /dev/null

Did not attempt the suggested ping method of reproducing on the old
system, but on the new kernel I could do
$ for a in $(seq 128 512); do ping -n -c 3 -s $a ping.sunet.se ; done
without any stalls.


Feel free to add either or both of:

Reported-by: Andreas Henriksson <andreas@fatal.se>
Tested-by: Andreas Henriksson <andreas@fatal.se>

> 
> Other than that qsel was not set correctly. The sympton here is that
> only one of multiple bulk endpoints was used to send data.
> 
> Changes since v1:
> - Use URB_ZERO_PACKET to let the USB host controller handle it automatically
>   rather than working around the issue.
> 
> Sascha Hauer (3):
>   wifi: rtw88: usb: Set qsel correctly
>   wifi: rtw88: usb: send Zero length packets if necessary
>   wifi: rtw88: usb: drop now unnecessary URB size check
> 
>  drivers/net/wireless/realtek/rtw88/usb.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> -- 
> 2.30.2
> 

Regards,
Andreas Henriksson

Re: [PATCH v2 0/3] wifi: rtw88: USB fixes

[Sascha Hauer]

On Fri, Feb 10, 2023 at 12:16:29PM +0100, Sascha Hauer wrote:
> This series addresses issues for the recently added RTW88 USB support
> reported by Andreas Henriksson and also our customer.
> 
> The hardware can't handle urbs that have a size of multiple of the
> bulkout_size (usually 512 bytes). The symptom is that the hardware
> stalls completely. The issue can be reproduced by sending a suitably
> sized ping packet from the device:
> 
> ping -s 394 <somehost>
> 
> (It's 394 bytes here on a RTL8822CU and RTL8821CU, the actual size may
> differ on other chips, it was 402 bytes on a RTL8723DU)
> 
> Other than that qsel was not set correctly. The sympton here is that
> only one of multiple bulk endpoints was used to send data.
> 
> Changes since v1:
> - Use URB_ZERO_PACKET to let the USB host controller handle it automatically
>   rather than working around the issue.
> 
> Sascha Hauer (3):
>   wifi: rtw88: usb: Set qsel correctly
>   wifi: rtw88: usb: send Zero length packets if necessary
>   wifi: rtw88: usb: drop now unnecessary URB size check

These patches went in upstream as:

7869b834fb07c wifi: rtw88: usb: Set qsel correctly
07ce9fa6ab0e5 wifi: rtw88: usb: send Zero length packets if necessary
462c8db6a0116 wifi: rtw88: usb: drop now unnecessary URB size check

These patches make the RTW88 USB support much more reliable. Can they be
picked for the current 6.2 stable series please?

Sascha

-- 
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 v2 0/3] wifi: rtw88: USB fixes

[Greg KH]

On Wed, Mar 01, 2023 at 08:11:41AM +0100, Sascha Hauer wrote:
> On Fri, Feb 10, 2023 at 12:16:29PM +0100, Sascha Hauer wrote:
> > This series addresses issues for the recently added RTW88 USB support
> > reported by Andreas Henriksson and also our customer.
> > 
> > The hardware can't handle urbs that have a size of multiple of the
> > bulkout_size (usually 512 bytes). The symptom is that the hardware
> > stalls completely. The issue can be reproduced by sending a suitably
> > sized ping packet from the device:
> > 
> > ping -s 394 <somehost>
> > 
> > (It's 394 bytes here on a RTL8822CU and RTL8821CU, the actual size may
> > differ on other chips, it was 402 bytes on a RTL8723DU)
> > 
> > Other than that qsel was not set correctly. The sympton here is that
> > only one of multiple bulk endpoints was used to send data.
> > 
> > Changes since v1:
> > - Use URB_ZERO_PACKET to let the USB host controller handle it automatically
> >   rather than working around the issue.
> > 
> > Sascha Hauer (3):
> >   wifi: rtw88: usb: Set qsel correctly
> >   wifi: rtw88: usb: send Zero length packets if necessary
> >   wifi: rtw88: usb: drop now unnecessary URB size check
> 
> These patches went in upstream as:
> 
> 7869b834fb07c wifi: rtw88: usb: Set qsel correctly
> 07ce9fa6ab0e5 wifi: rtw88: usb: send Zero length packets if necessary
> 462c8db6a0116 wifi: rtw88: usb: drop now unnecessary URB size check
> 
> These patches make the RTW88 USB support much more reliable. Can they be
> picked for the current 6.2 stable series please?

All now queued up, thanks.

greg k-h