RE: [PATCH v2 3/8] rtw88: 8723d: Add set_channel

[Tony Chuang <yhchuang@realtek.com>]

Kalle Valo <kvalo@codeaurora.org> writes:

> <yhchuang@realtek.com> writes:
> 
> > From: Ping-Ke Shih <pkshih@realtek.com>
> >
> > Set MAC/BB/RF register according to specified channel. The function
> > rtw_set_channel_mac() is used to set MAC registers, but 8723D only need
> > some of them.
> >
> > For channel 14, we need to set different CCK DFIR values, so restore the
> > values when channel 1 to 13 is selected.
> >
> > Spur calibration is needed in channel 13 and 14, and we do notch if spur
> > is over threshold.
> >
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> [...]
> 
> > --- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> > @@ -287,6 +287,190 @@ static void rtw8723d_query_rx_desc(struct
> rtw_dev *rtwdev, u8 *rx_desc,
> >  	rtw_rx_fill_rx_status(rtwdev, pkt_stat, hdr, rx_status, phy_status);
> >  }
> >
> > +static
> > +bool rtw8723d_check_spur_ov_thres(struct rtw_dev *rtwdev, u8 channel,
> u32 thres)
> > +{
> > +#define DIS_3WIRE	0xccf000c0
> > +#define EN_3WIRE	0xccc000c0
> > +#define START_PSD	0x400000
> > +#define FREQ_CH13	0xFCCD
> > +#define FREQ_CH14	0xFF9A
> 
> Why this style of adding defines within a function? You use it in
> multiple functions in this patch.
> 
> The normal upstream style is to have these kind of values defined in a
> .h file somewhere.
> 

Thought they are used just for the function, but yes, should put them
into rtw8723d.h or something like that :)

Yen-Hsuan