On 12/11/2024 22:29, Kees Bakker wrote:
> Op 30-10-2024 om 19:28 schreef Bitterblue Smith:
>> These contain code specific to RTL8821AU.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
>> ---
>> v2:
>> - Patch is new in v2.
>> - All of this used to be in patch 18/20 in v1.
>> - Use "k < 3" instead of "k <= 2" in the IQK code.
>> - Replace some while loops with for loops in the IQK code.
>> - Use rtw_write8 instead of rtw_write8_mask in
>> rtw8821a_coex_cfg_ant_switch. The mask was 0xff.
>> - Constify structs/arrays.
>>
>> v3:
>> - No change.
>>
>> v4:
>> - No change.
>> ---
>> drivers/net/wireless/realtek/rtw88/rtw8821a.c | 1197 +++++++++++++++++
>> drivers/net/wireless/realtek/rtw88/rtw8821a.h | 10 +
>> 2 files changed, 1207 insertions(+)
>> create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8821a.c
>> create mode 100644 drivers/net/wireless/realtek/rtw88/rtw8821a.h
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821a.c b/drivers/net/wireless/realtek/rtw88/rtw8821a.c
>> [...]
>> + for (cal_retry = 0; cal_retry < 10; cal_retry++) {
>> + /* one shot */
>> + rtw_write32(rtwdev, REG_IQK_COM64, 0xfa000000);
>> + rtw_write32(rtwdev, REG_IQK_COM64, 0xf8000000);
>> +
>> + mdelay(10);
>> +
>> + rtw_write32(rtwdev, REG_RFECTL_A, 0x00000000);
>> +
>> + for (delay_count = 0; delay_count < 20; delay_count++) {
>> + iqk_ready = rtw_read32_mask(rtwdev,
>> + REG_IQKA_END,
>> + BIT(10));
>> +
>> + /* Originally: if (~iqk_ready || delay_count > 20)
>> + * that looks like a typo so make it more explicit
>> + */
>> + iqk_ready = true;
> This looks a bit suspicious. Why ignore the result from rtw_read32_mask()?
> What do you mean by "Originally ... so make it more explicit"?
Yes, that's strange. I'm guessing the practice didn't align
with the theory--the hardware is not working as expected.
I tried it, bit 10 of REG_IQKA_END doesn't turn 1 in those
20 iterations.
"Originally" means as seen in the driver released by Realtek:
https://github.com/morrownr/8821au-20210708/blob/0b12ea54b7d6dcbfa4ce94eb403b1447565407f1/hal/phydm/halrf/rtl8821a/halrf_iqk_8821a_ce.c#L328
>> +
>> + if (iqk_ready)
>> + break;
>> +
>> + mdelay(1);
>> + }
>> +
>> + if (delay_count < 20) {
>> + /* ============TXIQK Check============== */
>> + tx_fail = rtw_read32_mask(rtwdev,
>> + REG_IQKA_END,
>> + BIT(12));
>> +
>> + /* Originally: if (~tx_fail) {
>> + * It looks like a typo, so make it more explicit.
>> + */
>> + tx_fail = false;
> This also looks suspicious. Again, why ignore the result of rtw_read32_mask()?
>> +
>> + if (!tx_fail) {
>> + rtw_write32(rtwdev, REG_RFECTL_A,
>> + 0x02000000);
>> + vdf_x[k] = rtw_read32_mask(rtwdev,
>> + REG_IQKA_END,
>> + 0x07ff0000);
>> + vdf_x[k] <<= 21;
>> +
>> + rtw_write32(rtwdev, REG_RFECTL_A,
>> + 0x04000000);
>> + vdf_y[k] = rtw_read32_mask(rtwdev,
>> + REG_IQKA_END,
>> + 0x07ff0000);
>> + vdf_y[k] <<= 21;
>> +
>> + *tx0iqkok = true;
>> + break;
>> + }
>> +
> The next rtw_write32_mask()'s are dead code because of tx_fail = false
Yes, looks that way.
>> + rtw_write32_mask(rtwdev, REG_IQC_Y,
>> + 0x000007ff, 0x0);
>> + rtw_write32_mask(rtwdev, REG_IQC_X,
>> + 0x000007ff, 0x200);
>> + }
>> +
>> + *tx0iqkok = false;
>> + }
>> + }
>> +
>>
