RE: [PATCH 25/40] rtw88: 8723d: Add LC calibration

[Tony Chuang <yhchuang@realtek.com>]

> On 2020-04-17 15:46:38 [+0800], yhchuang@realtek.com wrote:
> > index cf897af380c1..94784c7f0743 100644
> > --- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> > @@ -64,6 +64,33 @@ static const struct rtw_hw_reg rtw8723d_txagc[] = {
> >  #define WLAN_LTR_CTRL1		0xCB004010
> >  #define WLAN_LTR_CTRL2		0x01233425
> >
> > +static void rtw8723d_lck(struct rtw_dev *rtwdev)
> > +{
> > +#define BIT_LCK		BIT(15)
> 
> please don't add defines like this within a function.
> 
> > +	u8 val_ctx;
> > +	u32 lc_cal, cnt;
> > +
> > +	val_ctx = rtw_read8(rtwdev, REG_CTX);
> > +	if ((val_ctx & BIT_MASK_CTX_TYPE) != 0)
> > +		rtw_write8(rtwdev, REG_CTX, val_ctx & ~BIT_MASK_CTX_TYPE);
> > +	else
> > +		rtw_write8(rtwdev, REG_TXPAUSE, 0xFF);
> > +	lc_cal = rtw_read_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK);
> > +
> > +	rtw_write_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK, lc_cal |
> BIT_LCK);
> > +	for (cnt = 0; cnt < 100; cnt++) {
> > +		if (rtw_read_rf(rtwdev, RF_PATH_A, RF_CFGCH, BIT_LCK) != 0x1)
> > +			break;
> > +		mdelay(10);
> 
> Do you have any numbers on how long this takes? Like best-case, on average,
> worst case? I'm asking because if the bit does not flip on the first
> read then you busy-loop-delay here for 10ms. If it does not flip at all,
> you busy waited a whole second without any consequence.
> 
> It looks like this context here is not atomic so msleep() would work where.

Indeed, I think read_poll_timeout() will be good for this case. Thanks.

> 
> Sebastian
> 

Yen-Hsuan