Re: [PATCH rtw-next 3/5] wifi: rtw88: pci: Introduce struct rtw_pci_gen

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 24/10/2026 11:41, Luka Gejak wrote:
> October 24, 2026 at 1:01 AM, "Bitterblue Smith" <rtl8821cerfe2@gmail.com> wrote:
> 
>>
>> Move a few functions into a new struct rtw_pci_gen in order to add
>> support for RTL8821AE and RTL8812AE. That is an older generation of
>> chips which handle the PCI communications differently.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>  [...]
> 
>>       rx_ring->r.head = head;
>> +     rx_ring->r.dma = dma;
>> +     rx_ring->r.len = len;
>> +     rx_ring->r.desc_size = desc_size;
>> +     rx_ring->r.wp = 0;
>> +     rx_ring->r.rp = 0;
> 
> In my opinion the commit message should mention why these were moved
> before the loop.
> 

Right, that's because rtw_pci_old_reset_rx_desc() needs the ring
length too.

> [...]
> 
>>  {
>> +     struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
>> +     const struct rtw_pci_gen *pci_gen = rtwpci->gen;
>> +
>>       u8 q;
> 
> The blank line before u8 q can go, so the declarations stay together.
> 

I didn't notice that.

> [...]
> 
>>  int rtw_pci_probe(struct pci_dev *pdev,
>>                 const struct pci_device_id *id)
>>  {
>> +     struct rtw_pci_info *pci_info = (struct rtw_pci_info *)id->driver_data;
> 
> I think this should be a const pointer, since the pci_info tables are
> const.

Sounds good. Although that line gets very long.

> 
> Best regards,
> Luka Gejak