All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <stf_xl@wp.pl>
To: Shiji Yang <yangshiji66@outlook.com>
Cc: linux-wireless@vger.kernel.org, Kalle Valo <kvalo@kernel.org>,
	Helmut Schaa <helmut.schaa@googlemail.com>
Subject: Re: [PATCH] wifi: rt2x00: limit MT7620 TX power based on eeprom calibration
Date: Sun, 6 Aug 2023 10:27:01 +0200	[thread overview]
Message-ID: <20230806082701.GA361202@wp.pl> (raw)
In-Reply-To: <TYAP286MB0315C7523FE0E2EEA7413BC4BC0EA@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM>

On Sat, Aug 05, 2023 at 05:17:55PM +0800, Shiji Yang wrote:
> In the vendor driver, the current channel power is queried from
> EEPROM_TXPOWER_BG1 and EEPROM_TXPOWER_BG2. And then the mixed value
> will be written into the low half-word of the TX_ALC_CFG_0 register.
> The high half-word of the TX_ALC_CFG_0 is a fixed value 0x2f2f.
> 
> We can't get the accurate TX power. Based on my tests and the new
> MediaTek mt76 driver source code, the real TX power is approximately
> equal to channel_power + (max) rate_power. Usually max rate_power is
> the gain of the OFDM 6M rate, which can be readed from the offset
> EEPROM_TXPOWER_BYRATE +1.
> 
> Based on these eeprom values, this patch adds basic TX power control
> for the MT7620 and limits its maximum TX power. This can avoid the
> link speed decrease caused by chip overheating. rt2800_config_alc()
> function has also been renamed to rt2800_config_alc_rt6352() because
> it's only used by RT6352(MT7620).
> 
> Notice:
> It's still need some work to sync the max channel power to the user
> interface. This part is missing from the rt2x00 driver framework. If
> we set the power exceed the calibration value, it won't take effect.
> 
> Signed-off-by: Shiji Yang <yangshiji66@outlook.com>
> ---
>  .../net/wireless/ralink/rt2x00/rt2800lib.c    | 56 +++++++++++++------
>  1 file changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 1226a883c..ff891353c 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -3865,28 +3865,50 @@ static void rt2800_config_channel_rf7620(struct rt2x00_dev *rt2x00dev,
>  	}
>  }
>  
> -static void rt2800_config_alc(struct rt2x00_dev *rt2x00dev,
> -			      struct ieee80211_channel *chan,
> -			      int power_level) {
> -	u16 eeprom, target_power, max_power;
> +static void rt2800_config_alc_rt6352(struct rt2x00_dev *rt2x00dev,
> +				     struct ieee80211_channel *chan,
> +				     int power_level)
> +{
> +	u16 eeprom, chan_power, rate_power, target_power;
> +	u16 tx_power[2];
> +	s8 *power_group[2];
>  	u32 mac_sys_ctrl;
> -	u32 reg;
> +	u32 cnt, reg;
>  	u8 bbp;
>  
> -	/* hardware unit is 0.5dBm, limited to 23.5dBm */
> -	power_level *= 2;
> -	if (power_level > 0x2f)
> -		power_level = 0x2f;
> +	/* get per chain power, 2 chains in total, unit is 0.5dBm */
> +	power_level = (power_level - 3) * 2;
> +
> +	/*
> +	 * We can't get the accurate TX power. Based on some tests, the real
> +	 * TX power is approximately equal to channel_power + (max)rate_power.
> +	 * Usually max rate_power is the gain of the OFDM 6M rate. The antenna
> +	 * gain and externel PA gain are not included as we are unable to
> +	 * obtain these values.
> +	 */
> +	rate_power = rt2800_eeprom_read_from_array(rt2x00dev,
> +						   EEPROM_TXPOWER_BYRATE, 1);
> +	rate_power &= 0x3f;
> +	power_level -= rate_power;
> +	if (power_level < 1)
> +		power_level = 1;
> +	if (power_level > chan->max_power * 2)
> +		power_level = chan->max_power * 2;
>  
> -	max_power = chan->max_power * 2;
> -	if (max_power > 0x2f)
> -		max_power = 0x2f;
> +	power_group[0] = rt2800_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_BG1);
> +	power_group[1] = rt2800_eeprom_addr(rt2x00dev, EEPROM_TXPOWER_BG2);
> +	for (cnt = 0; cnt < 2; cnt++) {
> +		chan_power = power_group[cnt][rt2x00dev->rf_channel - 1];

I whould add

if (WARN_ON(rt2x00dev->rf_channel not in proper range))
	return;

just to be on safe side. This can be done in separate patch, if you think
it is appropriate.

The change overall looks ok to me.

Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>


  reply	other threads:[~2023-08-06  8:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-05  9:17 [PATCH] wifi: rt2x00: limit MT7620 TX power based on eeprom calibration Shiji Yang
2023-08-06  8:27 ` Stanislaw Gruszka [this message]
2023-08-11  6:47   ` Shiji Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230806082701.GA361202@wp.pl \
    --to=stf_xl@wp.pl \
    --cc=helmut.schaa@googlemail.com \
    --cc=kvalo@kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=yangshiji66@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.