RE: [PATCH v2 1/5] rtw88: 8821c: add cck pd settings

[Tony Chuang <yhchuang@realtek.com>]

> <yhchuang@realtek.com> writes:
> 
> > From: Tzu-En Huang <tehuang@realtek.com>
> >
> > CCK PD can reduce the number of false alarm of the CCK rates.
> > It dynamically adjusts the power threshold and CS ratio.
> > The values are compared to the values of the previous level, if
> > the level is changed, set new values of power threshold and CS
> > ratio.
> >
> > Implement rtw_chip_ops::cck_pd_set() for 8821c.
> >
> > Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> [...]
> 
> > --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> > @@ -102,6 +102,7 @@ static void rtw8821c_phy_set_param(struct rtw_dev
> *rtwdev)
> >  	rtwdev->chip->ch_param[2] = rtw_read32_mask(rtwdev, REG_TXFILTER,
> MASKDWORD);
> >
> >  	rtw_phy_init(rtwdev);
> > +	rtwdev->dm_info.cck_pd_default = rtw_read8(rtwdev, REG_CSRATIO) &
> 0x1f;
> >  }
> >
> >  static int rtw8821c_mac_init(struct rtw_dev *rtwdev)
> > @@ -596,6 +597,29 @@ static void rtw8821c_phy_calibration(struct
> rtw_dev *rtwdev)
> >  	rtw8821c_do_iqk(rtwdev);
> >  }
> >
> > +static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl)
> > +{
> > +	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> > +	u8 pd[CCK_PD_LV_MAX] = {3, 7, 13, 13, 13};
> > +
> > +	if (dm_info->min_rssi > 60) {
> > +		new_lvl = 4;
> > +		pd[4] = 0x1d;
> > +		goto set_cck_pd;
> > +	}
> > +
> > +	if (dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] ==
> new_lvl)
> > +		return;
> > +
> > +	dm_info->cck_fa_avg = CCK_FA_AVG_RESET;
> > +
> > +set_cck_pd:
> > +	dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] = new_lvl;
> > +	rtw_write32_mask(rtwdev, REG_PWRTH, 0x3f0000, pd[new_lvl]);
> > +	rtw_write32_mask(rtwdev, REG_PWRTH2, 0x1f0000,
> > +			 dm_info->cck_pd_default + new_lvl * 2);
> > +}
> 
> I'm starting to see the trend of using magic values in rtw88 on the rise
> again. Please be careful with this, the source code should not be full
> of undocument values. In some special cases (eg calibration data etc)
> using undocumented values is ok, but most of cases should have proper
> defines for documenting what's happening.
> 

I'll take care of that, many thanks.

Yen-Hsuan