On Wed, Oct 16, 2019 at 8:33 PM <yhchuang@xxxxxxxxxxx> wrote:
>
> From: Tzu-En Huang <tehuang@xxxxxxxxxxx>
>
> The temperature of the chip can affect the output power
> of the RF components. Hence driver requires to compensate
> the power by adjusting the power index recorded in the
> power swing table.
>
> And if the difference of current thermal value to the
> default thermal value exceeds a threshold, the RF IQK
> should be triggered to re-calibrate the characteristics
> of the RF components, to keep the output IQ vectors of
> the RF components orthogonal enough.
>
> Signed-off-by: Tzu-En Huang <tehuang@xxxxxxxxxxx>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx>
> ---
>
> v1 -> v2
> * Use macros to check current band
> * Some coding style refinement
> * Not casting "const" pointers
>
> @@ -1220,7 +1265,9 @@ struct rtw_efuse {
> u8 country_code[2];
> u8 rf_board_option;
> u8 rfe_option;
> - u8 thermal_meter;
> + u8 power_track_type;
> + u8 thermal_meter[2];
The '2' is related to RTW_RF_PATH_MAX or something else? Please have a
descriptive macro for this.
> + u8 thermal_meter_k;
> u8 crystal_cap;
> u8 ant_div_cfg;
> u8 ant_div_type;
> +
> +bool rtw_phy_pwrtrack_thermal_changed(struct rtw_dev *rtwdev, u8 thermal,
> + u8 path)
> +{
> + struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> + u8 avg = ewma_thermal_read(&dm_info->avg_thermal[path]);
> +
> + if (avg == thermal)
> + return false;
> +
> + return true;
> +}
Why not simplify this as pure if-else? It's a bit confusing.
> +bool rtw_phy_pwrtrack_need_iqk(struct rtw_dev *rtwdev)
> +{
> + struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> + u8 delta_iqk;
> +
> + delta_iqk = abs(dm_info->thermal_avg[0] - dm_info->thermal_meter_k);
> + if (delta_iqk >= rtwdev->chip->iqk_threshold) {
> + dm_info->thermal_meter_k = dm_info->thermal_avg[0];
> + return true;
> + }
> + return false;
> +}
Is 'thermal_avg[0]' for RF_PATH_A? Or it means something else?
And it's also good to simplify it as a simple if-else.
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822b.c b/drivers/net/wireless/realtek/rtw88/rtw8822b.c
> index baf5091fa253..8a018aefbe16 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8822b.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8822b.c
> @@ -43,6 +43,8 @@ static int rtw8822b_read_efuse(struct rtw_dev *rtwdev, u8 *log_map)
> efuse->country_code[1] = map->country_code[1];
> efuse->bt_setting = map->rf_bt_setting;
> efuse->regd = map->rf_board_option & 0x7;
> + efuse->thermal_meter[0] = map->thermal_meter;
> + efuse->thermal_meter_k = map->thermal_meter;
>
> for (i = 0; i < 4; i++)
> efuse->txpwr_idx_table[i] = map->txpwr_idx_table[i];
> @@ -75,6 +77,49 @@ static void rtw8822b_phy_rfe_init(struct rtw_dev *rtwdev)
> rtw_write32_mask(rtwdev, 0x974, (BIT(11) | BIT(10)), 0x3);
> }
>
Is 'thermal_meter[0]' for RF_PATH_A, 1 for PATH_B?
Dose the '4' refer to RTW_RF_PATH_MAX or totally irrelevant? Same
question for the rtw8822c.c
- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
@@ -40,6 +40,11 @@ static int rtw8822c_read_efuse(struct rtw_dev
*rtwdev, u8 *log_map)
efuse->country_code[1] = map->country_code[1];
efuse->bt_setting = map->rf_bt_setting;
efuse->regd = map->rf_board_option & 0x7;
+ efuse->thermal_meter[0] = map->path_a_thermal;
+ efuse->thermal_meter[1] = map->path_b_thermal;
+ efuse->thermal_meter_k =
+ (map->path_a_thermal + map->path_b_thermal) >> 1;
+ efuse->power_track_type = (map->tx_pwr_calibrate_rate >> 4) & 0xf;
for (i = 0; i < 4; i++)
efuse->txpwr_idx_table[i] = map->txpwr_idx_table[i];
Same as above.
> +
> 2.17.1
>
