回复: [PATCH 1/2] mac80211: rework MT7620 PA/LNA RF calibration

[Daniel Golle]

On Wed, Jul 26, 2023 at 02:57:08PM +0000, 杨 世基 wrote:
> Thanks for your review!
> 
> on July 26, 2023, 1:49 p.m. UTC, Daniel Golle wrote:
> >Hi!
> >
> >Thank you for your contribution.
> >I (probably) found a minor typo, see below:
> >
> >On Wed, Jul 26, 2023 at 09:22:22PM +0800, Shiji Yang wrote:
> >> From: Shiji Yang <yangshiji66 at qq.com>
> >> 
> >> This patch makes some improvements to the MT7620 RF calibration.
> >> 
> >> 1. Move MT7620 PA/LNA calibration code to dedicated functions.
> >> 2. Restore RF and BBP registers before R-Calibration.
> >> 3. Do Rx DCOC calibration again before RXIQ calibration.
> >> 4. Correct MAC_RX_EN mask in rt2800_r_calibration()[1].
> >> 
> >> [1] This change may fix the "BBP/RF register access failed" error:
> >> ieee80211 phy0: rt2800_wait_bbp_rf_ready: Error - BBP/RF register access failed, aborting
> >> 
> >> Signed-off-by: Shiji Yang <yangshiji66 at qq.com>
> >> ---
> >>  ...-rework-MT7620-PA-LNA-RF-calibration.patch | 312 ++++++++++++++++++
> >>  1 file changed, 312 insertions(+)
> >>  create mode 100644 package/kernel/mac80211/patches/rt2x00/998-wifi-rt2x00-rework-MT7620-PA-LNA-RF-calibration.patch
> >> 
> >> diff --git a/package/kernel/mac80211/patches/rt2x00/998-wifi-rt2x00-rework-MT7620-PA-LNA-RF-calibration.patch b/package/kernel/mac80211/patches/rt2x00/998-wifi-rt2x00-rework-MT7620-PA-LNA-RF-calibration.patch
> >> new file mode 100644
> >> index 0000000000..0cf34f3a6c
> >> +@@ -10688,30 +10637,151 @@ static void rt2800_init_rfcsr_6352(struc
> >> +              rt2800_rfcsr_write_dccal(rt2x00dev, 5, 0x00);
> >> +              rt2800_rfcsr_write_dccal(rt2x00dev, 17, 0x7C);
> >> +      }
> >> ++}
> >> + 
> >> +-    rt6352_enable_pa_pin(rt2x00dev, 0);
> >> +-    rt2800_r_calibration(rt2x00dev);
> >> +-    rt2800_rf_self_txdc_cal(rt2x00dev);
> >> +-    rt2800_rxdcoc_calibration(rt2x00dev);
> >> +-    rt2800_bw_filter_calibration(rt2x00dev, true);
> >> +-    rt2800_bw_filter_calibration(rt2x00dev, false);
> >> +-    rt2800_loft_iq_calibration(rt2x00dev);
> >> +-    rt2800_rxiq_calibration(rt2x00dev);
> >> +-    rt6352_enable_pa_pin(rt2x00dev, 1);
> >> ++static void rt6352_inint_ext_palna(struct rt2x00_dev *rt2x00dev)
> >
> >'inint' should probably be 'init' (?)
> 
> 
> Yes, you are right. It should be 'init'.
> 
> 
> >> ++/* MT7620 PA/LNA initialization after switching channels */
> >> ++static void rt6352_init_palna_stage2(struct rt2x00_dev *rt2x00dev)
> >> ++{
> >> ++    rt2800_rf_self_txdc_cal(rt2x00dev);
> >> ++    rt2800_rxdcoc_calibration(rt2x00dev);
> >> ++    rt2800_bw_filter_calibration(rt2x00dev, true);
> >> ++    rt2800_bw_filter_calibration(rt2x00dev, false);
> >> ++    rt2800_loft_iq_calibration(rt2x00dev);
> >> ++
> >> ++    /* missing DPD Calibration for devices using internal PA */
> >> ++
> >> ++    rt2800_rxdcoc_calibration(rt2x00dev);
> >> ++    rt2800_rxiq_calibration(rt2x00dev);
> >> ++
> >> ++    if(rt2x00_has_cap_external_pa(rt2x00dev) ||
> >> ++            rt2x00_has_cap_external_lna_bg(rt2x00dev)) {
> >> ++            rt6352_enable_pa_pin(rt2x00dev, 1);
> >> ++            rt6352_inint_ext_palna(rt2x00dev);
> 
> 
> And same typo error 'inint' here.
> 
> Do I need to send a v2 version to fix it? Or maybe you would like to
> manually edit it to avoid too many emails?

The best would be you'd give it a day for further reviews from others,
and then send v2 with all comments up to this point fixed.

To me this looks good, and I hope you will also send this and your
previous patch for rt2x00 to the linux-wireless mailing list to have
it included upstream.

Thank you again for your work!

> 
> Best Regards,
> Shiji Yang