On 30/07/2024 06:57, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>> Init RX burst length according to the USB speed.
>>
>> This is needed in order to make USB RX aggregation work.
>>
>> Tested with RTL8811CU.
>
> Having a throughput after this change would be better.
>
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>> ---
>> I would mention in the commit message what BIT_DMA_BURST_CNT,
>> BIT_DMA_MODE, and BIT_DROP_DATA_EN are doing, but I don't know.
>
> That will be helpful to other developers. Please put them in second paragraph.
>
> [...]
>
>> +static void rtw8821cu_init_burst_pkt_len(struct rtw_dev *rtwdev)
>> +{
>> + u8 rxdma, burst_size;
>> +
>> + rxdma = BIT_DMA_BURST_CNT | BIT_DMA_MODE;
>> +
>> + if (rtw_read8(rtwdev, REG_SYS_CFG2 + 3) == 0x20)
>> + burst_size = BIT_DMA_BURST_SIZE_1024;
>> + else if ((rtw_read8(rtwdev, REG_USB_USBSTAT) & 0x3) == 0x1)
>> + burst_size = BIT_DMA_BURST_SIZE_512;
>> + else
>> + burst_size = BIT_DMA_BURST_SIZE_64;
>> +
>> + u8p_replace_bits(&rxdma, burst_size, BIT_DMA_BURST_SIZE);
>> +
>> + rtw_write8(rtwdev, REG_RXDMA_MODE, rxdma);
>> + rtw_write16_set(rtwdev, REG_TXDMA_OFFSET_CHK, BIT_DROP_DATA_EN);
>> +}
>> +
>
> All use the same setup.
> Can we move it to usb.c? Maybe rtw_usb_interface_cfg() is a good place?
> (still exclude untested chips.)
>
rtw_usb_interface_cfg() is a good place. I will move it there.
The other chips will complicate it a bit, but that's okay.
RTL8723DU doesn't check for USB 3, of course:
if (rtwusb->udev->speed == USB_SPEED_HIGH)
burst_size = BIT_DMA_BURST_SIZE_512;
else
burst_size = BIT_DMA_BURST_SIZE_64;
RTL8821AU/RTL8812AU:
if (chip->id == RTW_CHIP_TYPE_8821A)
speedvalue = BIT(7);
else
speedvalue = rtw_read8(rtwdev, REG_SYS_CFG2 + 3);
if (speedvalue & BIT(7)) { /* USB 2/1.1 Mode */
temp = rtw_read8(rtwdev, 0xfe17);
if (((temp >> 4) & 0x03) == 0)
burst_size = BIT_DMA_BURST_SIZE_512;
else
burst_size = BIT_DMA_BURST_SIZE_64;
} else { /* USB3 Mode */
burst_size = BIT_DMA_BURST_SIZE_1024;
}
RTL8814AU:
speedvalue = rtw_read8(rtwdev, REG_SYS_CFG2 + 3);
if (speedvalue & BIT(7)) { /* USB 2/1.1 Mode */
if (rtwusb->udev->speed == USB_SPEED_HIGH)
burst_size = BIT_DMA_BURST_SIZE_512;
else
burst_size = BIT_DMA_BURST_SIZE_64;
} else { /* USB3 Mode */
burst_size = BIT_DMA_BURST_SIZE_1024;
}
I don't understand why we can't just check rtwusb->udev->speed
instead of reading various registers. Then they could all use
the same code.
(By the way, RTL8821AU/RTL8812AU is ready now. I will send
the patches after this patch set is sorted out. There are about
16 smaller patches to prepare things, and then the new driver.)
