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

2023-07-26 Thread
>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 
>> >>
>> >> 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 
>> >> ---
>> >>  ...-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 00..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

Thanks, I'll definitely send them to Linux mailing lists later. But
before that, I have to confirm that they are fully compatible with
upstream version, as we have a lot of local patches.

By the way, I have sent the patch[2/2] to the upstream:
https://lore.kernel.org/all/tyap286mb0315a9671b4ba0347e70d9e0bc...@tyap286mb0315.jpnp286.prod.outlook.com/

I will send the v2 version this weekend.

Regards
Shiji Yang
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


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

2023-07-26 Thread
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 
>> 
>> 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 
>> ---
>>  ...-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 00..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?

Best Regards,
Shiji Yang
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] mac80211: limit MT7620 TX power based on eeprom calibration

2023-07-22 Thread
Hi, 
Thank you for your quick review.

On July 22, 2023, 4:36 p.m. UTC, Daniel Golle wrote:

>On Sat, Jul 22, 2023 at 10:52:02PM +0800, Shiji Yang wrote:
>> From: Shiji Yang 
>>
>> This patch adds basic TX power control to the MT7620 and limits its
>> maximum TX power. This can avoid the link speed decrease caused by
>> chip overheating.
>
>Thanks a lot for your patch and analysis of the situation.
>As you add code reading values from the EEPROM, we need to make sure
>that it doesn't break other chips (Rt5xxx most likely, Rt3xxx could
>also be affected). The easiest would be to create a new function
>rt2800_config_alc_mt7620 which is called only for MT7620 while keeping
>the original codepath for all other rt2800 radios (unless we are 100%
>sure that we won't break things).
>
>>
>> Signed-off-by: Shiji Yang 

This rt2800_config_alc() function is only used by RT6352(AKA MT7620),
so these changes are safe. I'll rename it to rt2800_config_alc_rt6352()
on the V2 patch.

ref:
https://elixir.bootlin.com/linux/v6.4.3/A/ident/rt2800_config_alc
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel