On Sun, Jul 28, 2024 at 10:42:32PM +0300, Bitterblue Smith wrote:
> The chips can be configured to aggregate several frames into a single
> USB transfer. Modify rtw_usb_rx_handler() to support this case.
>
> RX aggregation improves the RX speed on certain ARM systems, like the
> NanoPi NEO Core2.
>
> Currently none of the chips are configured to aggregate frames.
>
> Tested with RTL8811CU and RTL8723DU.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> ---
> drivers/net/wireless/realtek/rtw88/usb.c | 57 +++++++++++++++---------
> 1 file changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 73948078068f..d61be1029a7b 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -546,11 +546,12 @@ static void rtw_usb_rx_handler(struct work_struct *work)
> struct rtw_usb *rtwusb = container_of(work, struct rtw_usb, rx_work);
> struct rtw_dev *rtwdev = rtwusb->rtwdev;
> const struct rtw_chip_info *chip = rtwdev->chip;
> - struct rtw_rx_pkt_stat pkt_stat;
> + u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> struct ieee80211_rx_status rx_status;
> + u32 pkt_offset, next_pkt, urb_len;
> + struct rtw_rx_pkt_stat pkt_stat;
> + struct sk_buff *next_skb = NULL;
> struct sk_buff *skb;
> - u32 pkt_desc_sz = chip->rx_pkt_desc_sz;
> - u32 pkt_offset;
> u8 *rx_desc;
> int limit;
>
> @@ -559,29 +560,44 @@ static void rtw_usb_rx_handler(struct work_struct *work)
> if (!skb)
> break;
>
> - rx_desc = skb->data;
> - chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat,
> - &rx_status);
> - pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> - pkt_stat.shift;
> -
> - if (pkt_stat.is_c2h) {
> - skb_put(skb, pkt_stat.pkt_len + pkt_offset);
> - rtw_fw_c2h_cmd_rx_irqsafe(rtwdev, pkt_offset, skb);
> - continue;
> - }
> -
> if (skb_queue_len(&rtwusb->rx_queue) >= RTW_USB_MAX_RXQ_LEN) {
> dev_dbg_ratelimited(rtwdev->dev, "failed to get rx_queue, overflow\n");
> dev_kfree_skb_any(skb);
> continue;
> }
>
> - skb_put(skb, pkt_stat.pkt_len);
> - skb_reserve(skb, pkt_offset);
> - rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> - memcpy(skb->cb, &rx_status, sizeof(rx_status));
> - ieee80211_rx_irqsafe(rtwdev->hw, skb);
> + urb_len = skb->len;
> +
> + do {
> + rx_desc = skb->data;
> + chip->ops->query_rx_desc(rtwdev, rx_desc, &pkt_stat,
> + &rx_status);
> + pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
> + pkt_stat.shift;
> +
> + next_pkt = round_up(pkt_stat.pkt_len + pkt_offset, 8);
> +
> + if (urb_len >= next_pkt + pkt_desc_sz)
> + next_skb = skb_clone(skb, GFP_KERNEL);
You could add a:
else
next_skb = NULL;
here and drop the next_skb = NULL from the end of the loop. No
functional change, but easier to read.
> +
> + if (pkt_stat.is_c2h) {
> + skb_trim(skb, pkt_stat.pkt_len + pkt_offset);
> + rtw_fw_c2h_cmd_rx_irqsafe(rtwdev, pkt_offset, skb);
> + } else {
> + skb_pull(skb, pkt_offset);
> + skb_trim(skb, pkt_stat.pkt_len);
> + rtw_rx_stats(rtwdev, pkt_stat.vif, skb);
> + memcpy(skb->cb, &rx_status, sizeof(rx_status));
> + ieee80211_rx_irqsafe(rtwdev->hw, skb);
> + }
> +
> + skb = next_skb;
> + if (skb)
> + skb_pull(next_skb, next_pkt);
You could use skb instead of next_skb here. Both are the same, so no
functional change, just makes it a bit easier to read when you use the
same variable that you just tested for validity.
> +
> + urb_len -= next_pkt;
> + next_skb = NULL;
> + } while (skb && urb_len >= pkt_desc_sz);
You can drop the urb_len >= pkt_desc_sz check. It will be exactly true
when skb is non NULL as well.
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 |
