On 10/09/2024 05:30, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>> On 16/08/2024 09:06, Ping-Ke Shih wrote:
>>> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> wrote:
>>>> + pkt_stat->phy_status = GET_RX_DESC_PHYST(rx_desc);
>>>> + pkt_stat->icv_err = GET_RX_DESC_ICV_ERR(rx_desc);
>>>> + pkt_stat->crc_err = GET_RX_DESC_CRC32(rx_desc);
>>>> + pkt_stat->decrypted = !GET_RX_DESC_SWDEC(rx_desc) &&
>>>> + GET_RX_DESC_ENC_TYPE(rx_desc) != RX_DESC_ENC_NONE;
>>>> + pkt_stat->is_c2h = GET_RX_DESC_C2H(rx_desc);
>>>> + pkt_stat->pkt_len = GET_RX_DESC_PKT_LEN(rx_desc);
>>>> + pkt_stat->drv_info_sz = GET_RX_DESC_DRV_INFO_SIZE(rx_desc);
>>>> + pkt_stat->shift = GET_RX_DESC_SHIFT(rx_desc);
>>>> + pkt_stat->rate = GET_RX_DESC_RX_RATE(rx_desc);
>>>> + pkt_stat->cam_id = GET_RX_DESC_MACID(rx_desc);
>>>> + pkt_stat->ppdu_cnt = 0;
>>>> + pkt_stat->tsf_low = GET_RX_DESC_TSFL(rx_desc);
>>>> + pkt_stat->bw = GET_RX_DESC_BW(rx_desc);
>>>
>>> More and more chips use these macros. Please add a patch using struct to
>>> access these fields. More, query_rx_desc() are very similar across chips,
>>> please move them to mac.c or phy.c as a common function.
>>>
>>
>> Why not rx.c?
>
> Also fine to me.
>
>>> Can you collect undefined register addresses? I can try to lookup them in one go.
>>>
>>
>> Here are the addresses I could find and a few bits/bit masks:
>>
>> 0x765
>
> #define REG_GNT_BT 0x765
> #define BIT_PTA_SW_CTL GENMASK(4, 3)
>
>> 0x90c
>
> #define REG_DAC_RSTB 0x90c
>
>> 0x978
>> 31, 0x03ff8000, 0x000007ff
>> 0x97c
>> 31
>> 0x980
>> 0x984
>
> #define REG_IQK_COM00 0x978
> #define REG_IQK_COM32 0x97c
> #define REG_IQK_COM64 0x980
> #define REG_IQK_COM96 0x984
>
>> 0xa0a
>
> #define REG_CCK_PD_TH 0xa0a
>
>> 0xc00
>> 0xf
>> 0xe00
>
> #define REG_3WIRE_SWA 0xc00
> #define REG_3WIRE_SWB 0xe00
>
> (0xc00 page for path A, 0xe00 page for path B)
>
Previously, you called 0xe00 REG_HSSI_WRITE_B. Are both names correct?
(I'm not sure why I put 0xc00 and 0xe00 on the list if you already gave
them a name.)
>> 0xc5c
>> 0xe5c
>> GENMASK(26, 24)
>
> #define REG_CK_MONHA 0xc5c
> #define REG_CK_MONHB 0xe5c
>
>> 0xce8
>> 31, 0x3fff0000
>> 0xee8
>
> #define REG_INTPO_SETA 0xce8
> #define REG_INTPO_SETB 0xee8
>
>> 0xd00
>> 10, 11, 12, 0x07ff0000
>
> #define REG_IQKA_END 0xd00
>
>> 0xd40
>> 10, 11, 12
>
> #define REG_IQKB_END 0xd40
>
>> GENMASK(26, 24)
>> 0xe80
>
> #define REG_TXTONEB 0xe80
>
>> 0xe84
>
> #define REG_RXTONEB 0xe84
>
>> 0xe88
>
> #define REG_TXPITMB 0xe88
>
>> 0xe8c
>
> #define REG_RXPITMB 0xe8c
>
>> 0xe90
>
> #define REG_PREDISTB 0xe90
>
I put 0xe90 on the list by mistake. We already have a name for it
in the official driver:
./include/Hal8812PhyReg.h:66:#define rB_LSSIWrite_Jaguar 0xe90 /* RF write addr */
I translated that as REG_LSSI_WRITE_B. Is REG_PREDISTB also a valid
name? Do we need both names?
>> 7
>> 0xe94
>
> #define REG_INIDLYB 0xe94
>
>> 0
>> 0xec4
>> 18, 29
>
>
> #define REG_BPBDB 0xec4
>
>> 0xec8
>> 29
>
> #define REG_PHYTXONB 0xec8
>
Some of these names are very different from their counterparts
from page C. In your previous email you said:
> I think we can reuse existing definitions:
>
> rtw8723x.h:#define REG_OFDM_0_XA_TX_IQ_IMBALANCE 0x0c80
> rtw8703b.h:#define REG_OFDM0_A_TX_AFE 0x0c84
> rtw8723x.h:#define REG_OFDM_0_XB_TX_IQ_IMBALANCE 0x0c88
>
> #define REG_TSSI_TRK_SW 0xc8c
>
> rtw8821a.h:#define REG_TXAGCIDX 0xc94
Can we reuse these definitions?
>> 0xecc
>
> #define REG_IQKYB 0xecc
>
>> 0xed4
>
> #define REG_IQKXB 0xed4
>
>> 0xf008
>> 3, 4
>> 0xf050
>
> I can't find these two, but just follow comments above.
>
> #define REG_USB_MOD 0xf008
> #define REG_USB3_RXITV 0xf050
>
>
>>
>> And some RF registers: 0x8, 0x58, 0x65, 0x8f
>>
>
> #define RF_DTXLOK 0x08 (already existing)
> #define RF_TXMOD 0x58
> #define RF_TXA_PREPAD 0x65
> #define RF_RXBB2 0x8f
>
>
>> [...]
>>
>>>> +
>>>> +const struct rtw_chip_info rtw8812a_hw_spec = {
>>>
>>> Is it possible moving 8812a to individual file?
>>> Since you have rtw8812au.c and rtw8821au.c.
>>>
>>
>> I think it is possible. But most of the code is common to both chips.
>> Only the IQ calibration could be moved.
>
> Yep, depend on how much IQK code echo chip has.
>
The IQ calibration for RTL8812AU is about 700 lines.
>>
>> Another possibility is to move rtw8812au.c into rtw8821au.c and have
>> only one module handle both chips.
>
> Prefer two modules as was. That is clear to people we have two chips.
>
