RE: [PATCH 1/4] wifi: rtw88: Init RX burst length for 8822cu/8822bu/8821cu

[Ping-Ke Shih <pkshih@realtek.com>]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 30/07/2024 06:57, Ping-Ke Shih wrote:
> > Bitterblue Smith <rtl8821cerfe2@gmail.com> 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@gmail.com>
> >> ---
> >> 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!