Re: [PATCH wireless-next v2 6/7] wifi: rt2800usb: enable RX bulk aggregation

[Stanislaw Gruszka <stf_xl@wp.pl>]

On Sun, Nov 01, 2026 at 12:51:13AM +0800, Ho Jie Feng wrote:
> Enable RX bulk aggregation on high speed USB, to receive more
> frames per URB.
> 
> RX buffers are now 24KB, so reduce it to 16 buffers instead of 128,
> so that they take 512 KB in total instead of 4 MB.
> 
> Test results:
>    before after
> TX 30.5   30.5
> RX 67.8   80.9
> 
> This code references the mt7601u driver.
> 
> Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
> ---
>  .../net/wireless/ralink/rt2x00/rt2800usb.c    | 32 +++++--
>  .../net/wireless/ralink/rt2x00/rt2800usb.h    |  6 ++
>  drivers/net/wireless/ralink/rt2x00/rt2x00.h   |  3 +-
>  .../net/wireless/ralink/rt2x00/rt2x00dev.c    | 84 +++++++++++++------
>  .../net/wireless/ralink/rt2x00/rt2x00mmio.c   |  2 +-
>  .../net/wireless/ralink/rt2x00/rt2x00usb.c    |  5 +-
>  6 files changed, 100 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
> index 387c07e2251b7..2e4277b1e5d1b 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
> @@ -301,15 +301,15 @@ static int rt2800usb_enable_radio(struct rt2x00_dev *rt2x00dev)
>  		return -EIO;
>  
>  	rt2x00_set_field32(&reg, USB_DMA_CFG_PHY_CLEAR, 0);
> -	rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_EN, 0);
> +	rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_EN,
> +			   rt2x00dev->rx->usb_maxpacket == 512);

Any idea why this condition ? We can not have aggregation on slower
or faster USB?

>  	rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_TIMEOUT, 128);
>  	/*
>  	 * Total room for RX frames in kilobytes, PBF might still exceed
>  	 * this limit so reduce the number to prevent errors.
>  	 */
>  	rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_AGG_LIMIT,
> -			   ((rt2x00dev->rx->limit * DATA_FRAME_SIZE)
> -			    / 1024) - 3);
> +			   (rt2x00dev->rx->data_size / 1024) - 4);
>  	rt2x00_set_field32(&reg, USB_DMA_CFG_RX_BULK_EN, 1);
>  	rt2x00_set_field32(&reg, USB_DMA_CFG_TX_BULK_EN, 1);
>  	rt2x00usb_register_write(rt2x00dev, USB_DMA_CFG, reg);
> @@ -478,6 +478,27 @@ static void rt2800usb_work_txdone(struct work_struct *work)
>  /*
>   * RX control handlers
>   */
> +static unsigned int rt2800usb_get_rx_seg_len(void *data, unsigned int len)
> +{
> +	unsigned int min_seg_len = RXINFO_DESC_SIZE + RXWI_DESC_SIZE_4WORDS +
> +				   RXD_DESC_SIZE;
> +	unsigned int rx_pkt_len;
> +	u32 word;
> +
> +	if (len < min_seg_len)
> +		return 0;
> +
> +	word = rt2x00_desc_read(data, 0);
> +	rx_pkt_len = rt2x00_get_field32(word, RXINFO_W0_USB_DMA_RX_PKT_LEN);
> +
> +	if (!rx_pkt_len ||
> +	    RXINFO_DESC_SIZE + rx_pkt_len + RXD_DESC_SIZE > len ||
> +	    rx_pkt_len & 0x3)

This is indication of we do something wrong, similar when len < min_seg_len.
Maybe wrap those with WARN_ON_ONCE() ? 

>
> +
> +	return RXINFO_DESC_SIZE + rx_pkt_len + RXD_DESC_SIZE;
> +}
> +
>  static void rt2800usb_fill_rxdone(struct queue_entry *entry,
>  				  struct rxdone_entry_desc *rxdesc)
>  {
> @@ -706,6 +727,7 @@ static const struct rt2x00lib_ops rt2800usb_rt2x00_ops = {
>  	.clear_beacon		= rt2800_clear_beacon,
>  	.get_tx_data_len	= rt2800usb_get_tx_data_len,
>  	.fill_rxdone		= rt2800usb_fill_rxdone,
> +	.get_rx_seg_len		= rt2800usb_get_rx_seg_len,
>  	.config_shared_key	= rt2800_config_shared_key,
>  	.config_pairwise_key	= rt2800_config_pairwise_key,
>  	.config_filter		= rt2800_config_filter,
> @@ -725,8 +747,8 @@ static void rt2800usb_queue_init(struct data_queue *queue)
>  
>  	switch (queue->qid) {
>  	case QID_RX:
> -		queue->limit = 128;
> -		queue->data_size = AGGREGATION_SIZE;
> +		queue->limit = 16;
> +		queue->data_size = MAX_RXBULK_SIZE;

Since we enable aggregation conditionally, this probably should also be
conditional. But the issue is that we do not have 
rt2x00dev->rx->usb_maxpacket value available yet.

We could just enable aggregation unconditionally,
but I'm not sure it will work with some older chips like RT2870?

So perhaps better would be set a capability flag during probe for
chips that know to work with the aggregation and use it here and 
when configuring USB_DMA register? Or base on usb_maxpacket
but check it during probe?

Regards
Stanislaw