Re: [PATCH v2] wifi: rtw88: usb: Further limit the TX aggregation

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 11/06/2024 06:26, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> @@ -2065,25 +2066,26 @@ const struct rtw_chip_info rtw8703b_hw_spec = {
>>         .bt_rssi_type = COEX_BTRSSI_RATIO,
>>         .ant_isolation = 15,
>>         .rssi_tolerance = 2,
>> -       .bt_rssi_step = bt_rssi_step_8703b,
>> -       .wl_rssi_step = wl_rssi_step_8703b,
>>         /* sant -> shared antenna, nsant -> non-shared antenna
>>          * Not sure if 8703b versions with non-shard antenna even exist.
>>          */
>>         .table_sant_num = ARRAY_SIZE(table_sant_8703b),
>> -       .table_sant = table_sant_8703b,
> 
> Not sure why you move array and array size apart. 
> No need to churn this as well as other similar stuffs. 
> 

I thought you wanted the members initialised in the same order
they appear in struct rtw_chip_info in main.h. Maybe I misunderstood.

> This patch can only change things required by what subject mention. Align these
> fields by another patch, or do it later. Both are fine to me. 
> 

Ah, it should be done in a different patch, okay.

I'm still not sure what you meant here:

>> diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
>> index 49894331f7b4..49a3fd4fb7dc 100644
>> --- a/drivers/net/wireless/realtek/rtw88/main.h
>> +++ b/drivers/net/wireless/realtek/rtw88/main.h
>> @@ -1197,6 +1197,8 @@ struct rtw_chip_info {
>>         u16 fw_fifo_addr[RTW_FW_FIFO_MAX];
>>         const struct rtw_fwcd_segs *fwcd_segs;
>>
>> +       u8 usb_tx_agg_desc_num;
>> +
>
> Please keep order of field and instance declaration, like rtw8703b_hw_spec.

and here:

>>  #define REG_DWBCN0_CTRL                0x0208
>>  #define BIT_BCN_VALID          BIT(16)
>>  #define REG_TXDMA_OFFSET_CHK   0x020C
>> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.c
>> b/drivers/net/wireless/realtek/rtw88/rtw8703b.c
>> index 8919f9e11f03..222608de33cd 100644
>> --- a/drivers/net/wireless/realtek/rtw88/rtw8703b.c
>> +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.c
>> @@ -2013,6 +2013,7 @@ const struct rtw_chip_info rtw8703b_hw_spec = {
>>         .tx_stbc = false,
>>         .max_power_index = 0x3f,
>>         .ampdu_density = IEEE80211_HT_MPDU_DENSITY_16,
>> +       .usb_tx_agg_desc_num = 1, /* Not sure if this chip has USB interface */
>
> The position to declare this field is very different from others. 
> 
> Not sure when we messed up the order, but please don't feel free to add one,
> keep the order.