RE: [PATCH rtw-next v6 2/3] wifi: rtw88: rtw8822c: Convert DAC IQ calibration path to signed math
From: Ping-Ke Shih
Date: Tue Aug 25 2026 - 03:43:59 EST
Arsenii Pashchenko <ulijg308@xxxxxxxxx> wrote:
> Convert the temporary stack arrays 'iv' and 'qv' from u32 to s32.
> Update all internal function signatures to accept s32 pointers and
> handle signed values natively.
>
> Use sign_extend32() combined with FIELD_GET() to properly interpret
> the 10-bit hardware values as signed integers.
Forgot to update commit message as FIELD_GET_SIGNED()...
> This allows flattening
> the complex nested unsigned boundary conditions in
> rtw8822c_dac_iq_check() into a simple amplitude boundary check. Ensure
> rtw8822c_dac_iq_offset() properly maps the signed average back to a
> 10-bit unsigned format expected by the hardware registers.
>
> Signed-off-by: Arsenii Pashchenko <ulijg308@xxxxxxxxx>
> ---
[...]
> @@ -207,30 +172,16 @@ static void rtw8822c_dac_iq_sort(struct rtw_dev *rtwdev, u32 *iv, u32 *qv)
> }
> }
>
> -static void rtw8822c_dac_iq_offset(struct rtw_dev *rtwdev, u32 *vec, u32 *val)
> +static u32 rtw8822c_dac_iq_offset(struct rtw_dev *rtwdev, s32 *vec)
> {
> - u32 p, m, t, i;
> + s32 sum = 0;
>
> - m = 0;
> - p = 0;
> - for (i = 10; i < DACK_SN_8822C - 10; i++) {
> - if (vec[i] > 0x200)
> - m = (0x400 - vec[i]) + m;
> - else
> - p = vec[i] + p;
> - }
> + for (u32 i = 10; i < DACK_SN_8822C - 10; i++)
> + sum += vec[i];
>
> - if (p > m) {
> - t = p - m;
> - t = t / (DACK_SN_8822C - 20);
> - } else {
> - t = m - p;
> - t = t / (DACK_SN_8822C - 20);
> - if (t != 0x0)
> - t = 0x400 - t;
> - }
> + s32 avg = sum / (DACK_SN_8822C - 20);
No declaration in the middle.
>
> - *val = t;
> + return avg >= 0 ? (u32)avg : (u32)(0x400 + avg);
Maybe, we can give an example to understand easier, like
avg = -1, return 0x3FF. (signed 10 bits value)
> }
>
> static u32 rtw8822c_get_path_write_addr(u8 path)
> @@ -271,20 +222,18 @@ static u32 rtw8822c_get_path_read_addr(u8 path)
> return base_addr;
> }
>
> -static bool rtw8822c_dac_iq_check(struct rtw_dev *rtwdev, u32 value)
> +static bool rtw8822c_dac_iq_check(struct rtw_dev *rtwdev, s32 value)
> {
> - bool ret = true;
>
> - if ((value >= 0x200 && (0x400 - value) > 0x64) ||
> - (value < 0x200 && value > 0x64)) {
> - ret = false;
> + if (value > 100 || value < -100) {
> rtw_dbg(rtwdev, RTW_DBG_RFK, "[DACK] Error overflow\n");
> + return false;
> }
>
> - return ret;
> + return true;
> }
>
> -static void rtw8822c_dac_cal_iq_sample(struct rtw_dev *rtwdev, u32 *iv, u32 *qv)
> +static void rtw8822c_dac_cal_iq_sample(struct rtw_dev *rtwdev, s32 *iv, s32 *qv)
> {
> u32 temp;
> int i = 0, cnt = 0;
> @@ -292,8 +241,8 @@ static void rtw8822c_dac_cal_iq_sample(struct rtw_dev *rtwdev, u32 *iv, u32 *qv)
> while (i < DACK_SN_8822C && cnt < 10000) {
> cnt++;
> temp = rtw_read32(rtwdev, 0x2dbc);
> - iv[i] = FIELD_GET(RTW8822C_DAC_IV_MASK, temp);
> - qv[i] = FIELD_GET(RTW8822C_DAC_QV_MASK, temp);
> + iv[i] = FIELD_GET_SIGNED(RTW8822C_DAC_IV_MASK, temp);
> + qv[i] = FIELD_GET_SIGNED(RTW8822C_DAC_QV_MASK, temp);
>
> if (rtw8822c_dac_iq_check(rtwdev, iv[i]) &&
> rtw8822c_dac_iq_check(rtwdev, qv[i]))
> @@ -302,67 +251,63 @@ static void rtw8822c_dac_cal_iq_sample(struct rtw_dev *rtwdev, u32 *iv, u32 *qv)
> }
>
> static void rtw8822c_dac_cal_iq_search(struct rtw_dev *rtwdev,
> - u32 *iv, u32 *qv,
> + s32 *iv, s32 *qv,
> u32 *i_value, u32 *q_value)
> {
> - u32 i_max = 0, q_max = 0, i_min = 0, q_min = 0;
> + s32 i_max = 0, q_max = 0, i_min = 0, q_min = 0;
> u32 i_delta, q_delta;
> u32 temp;
> - int i, cnt = 0;
> + u32 i, cnt = 0;
Why did you need changing 'int' to 'u32'?
>
> do {
> i_min = iv[0];
> i_max = iv[0];
> q_min = qv[0];
> q_max = qv[0];
> - for (i = 0; i < DACK_SN_8822C; i++) {
> - rtw8822c_rf_minmax_cmp(rtwdev, iv[i], &i_min, &i_max);
> - rtw8822c_rf_minmax_cmp(rtwdev, qv[i], &q_min, &q_max);
> + for (i = 1; i < DACK_SN_8822C; i++) {
> + if (iv[i] < i_min)
> + i_min = iv[i];
> + if (iv[i] > i_max)
> + i_max = iv[i];
> + if (qv[i] < q_min)
> + q_min = qv[i];
> + if (qv[i] > q_max)
> + q_max = qv[i];
> }
>
> - if (i_max < 0x200 && i_min < 0x200)
> - i_delta = i_max - i_min;
> - else if (i_max >= 0x200 && i_min >= 0x200)
> - i_delta = i_max - i_min;
> - else
> - i_delta = i_max + (0x400 - i_min);
> + i_delta = i_max - i_min;
> + q_delta = q_max - q_min;
>
> - if (q_max < 0x200 && q_min < 0x200)
> - q_delta = q_max - q_min;
> - else if (q_max >= 0x200 && q_min >= 0x200)
> - q_delta = q_max - q_min;
> - else
> - q_delta = q_max + (0x400 - q_min);
>
> rtw_dbg(rtwdev, RTW_DBG_RFK,
> - "[DACK] i: min=0x%08x, max=0x%08x, delta=0x%08x\n",
> + "[DACK] i: min=%d, max=%d, delta=%d\n",
> i_min, i_max, i_delta);
> rtw_dbg(rtwdev, RTW_DBG_RFK,
> - "[DACK] q: min=0x%08x, max=0x%08x, delta=0x%08x\n",
> + "[DACK] q: min=%d, max=%d, delta=%d",
> q_min, q_max, q_delta);
>
> rtw8822c_dac_iq_sort(rtwdev, iv, qv);
>
> if (i_delta > 5 || q_delta > 5) {
> temp = rtw_read32(rtwdev, 0x2dbc);
> - iv[0] = FIELD_GET(RTW8822C_DAC_IV_MASK, temp);
> - qv[0] = FIELD_GET(RTW8822C_DAC_QV_MASK, temp);
> + iv[0] = FIELD_GET_SIGNED(RTW8822C_DAC_IV_MASK, temp);
> + qv[0] = FIELD_GET_SIGNED(RTW8822C_DAC_QV_MASK, temp);
> temp = rtw_read32(rtwdev, 0x2dbc);
> - iv[DACK_SN_8822C - 1] = FIELD_GET(RTW8822C_DAC_IV_MASK, temp);
> - qv[DACK_SN_8822C - 1] = FIELD_GET(RTW8822C_DAC_QV_MASK, temp);
> + iv[DACK_SN_8822C - 1] = FIELD_GET_SIGNED(RTW8822C_DAC_IV_MASK, temp);
> + qv[DACK_SN_8822C - 1] = FIELD_GET_SIGNED(RTW8822C_DAC_QV_MASK, temp);
> } else {
> break;
> }
> } while (cnt++ < 100);
>
> - rtw8822c_dac_iq_offset(rtwdev, iv, i_value);
> - rtw8822c_dac_iq_offset(rtwdev, qv, q_value);
> + *i_value = rtw8822c_dac_iq_offset(rtwdev, iv);
> + *q_value = rtw8822c_dac_iq_offset(rtwdev, qv);
> }
>
> static void rtw8822c_dac_cal_rf_mode(struct rtw_dev *rtwdev,
> u32 *i_value, u32 *q_value)
> {
> - u32 iv[DACK_SN_8822C], qv[DACK_SN_8822C];
> + s32 iv[DACK_SN_8822C], qv[DACK_SN_8822C];
> u32 rf_a, rf_b;
>
> rf_a = rtw_read_rf(rtwdev, RF_PATH_A, 0x0, RFREG_MASK);
> --
> 2.55.0