On 11/06/2024 06:26, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> 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.
