Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <91c58969-c60e-2f41-00ac-737786d435ae@nbd.name>
Date:   Sat, 16 Apr 2022 20:39:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [PATCH] ath9k: fix QCA9561 PA bias
Content-Language: en-US
To:     =?UTF-8?Q?Thibaut_VAR=c3=88NE?= <hacks+kernel@slashdirt.org>,
        =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@toke.dk>,
        Kalle Valo <kvalo@kernel.org>, linux-wireless@vger.kernel.org
Cc:     =?UTF-8?Q?Petr_=c5=a0tetiar?= <ynezz@true.cz>,
        Clemens Hopfer <openwrt@wireloss.net>
References: <20220415104419.22749-1-hacks+kernel@slashdirt.org>
From:   Felix Fietkau <nbd@nbd.name>
In-Reply-To: <20220415104419.22749-1-hacks+kernel@slashdirt.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 15.04.22 12:44, Thibaut VARÈNE wrote:
> ath9k is setting the TX PA DC bias level differently on QCA9561 and
> QCA9565 although they have the same radio IP-core, which results in a
> very low output power and very low throughput as devices are further
> away from the AP (compared to other 2.4GHz APs).
> 
> In real life testing, without this patch the 2.4GHz throughput on
> Yuncore XD3200 is around 10Mbps sitting next to the AP, and close to
> practical maximum with the patch applied.
> 
> Tested-by: Petr Štetiar <ynezz@true.cz>
> Signed-off-by: Clemens Hopfer <openwrt@wireloss.net>
> Signed-off-by: Thibaut VARÈNE <hacks+kernel@slashdirt.org>
> ---
>   drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
> index b0a4ca355..8f8682f25 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
> @@ -3606,9 +3606,10 @@ static void ar9003_hw_xpa_bias_level_apply(struct ath_hw *ah, bool is2ghz)
>   	int bias = ar9003_modal_header(ah, is2ghz)->xpaBiasLvl;
>   
>   	if (AR_SREV_9485(ah) || AR_SREV_9330(ah) || AR_SREV_9340(ah) ||
> -	    AR_SREV_9531(ah) || AR_SREV_9561(ah))
> +	    AR_SREV_9531(ah))
>   		REG_RMW_FIELD(ah, AR_CH0_TOP2, AR_CH0_TOP2_XPABIASLVL, bias);
> -	else if (AR_SREV_9462(ah) || AR_SREV_9550(ah) || AR_SREV_9565(ah))
> +	else if (AR_SREV_9462(ah) || AR_SREV_9550(ah) || AR_SREV_9561(ah) ||
> +		 AR_SREV_9565(ah))
>   		REG_RMW_FIELD(ah, AR_CH0_TOP, AR_CH0_TOP_XPABIASLVL, bias);
The patch looks wrong to me. I'm pretty sure that AR_CH0_TOP2 is the 
correct register, the definition has an explicit check for 9561 as well.
I believe this patch works by accident because it avoids writing a wrong 
value to that register.
The value written to that register is wrong, because while the mask 
definition AR_CH0_TOP2_XPABIASLVL uses a different value for 9561, the 
shift definition AR_CH0_TOP2_XPABIASLVL_S is hardcoded to 12, which is 
wrong for 9561.
Please try adjusting it to this:
#define AR_CH0_TOP2_XPABIASLVL_S (AR_SREV_9561(ah) ? 9 : 12)

- Felix