Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
> 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.
Maybe we can have init_burst_pkt_len_{v1, v2, v3, ...}, but...
>
> 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.
It seems to be better to just use rtwusb->udev->speed.
I ask USB team but no clear answer, but they prefer rtwusb->udev->speed as well.
>
> (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.)
Nice work!
