Received-SPF: pass (google.com: domain of linux-kernel+bounces-203119-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 5 Jun 2024 21:44:22 +0200
From: Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
To: Benjamin Schneider <bschnei@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Gregory Clement <gregory.clement@bootlin.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	Benjamin Schneider <ben@bens.haus>
Subject: Re: [PATCH] cpufreq: enable 1200Mhz clock speed for armada-37xx
Message-ID: <20240605194422.klxtxgyljrrllkzy@pali>
References: <20240603012804.122215-1-ben@bens.haus>
 <20240603012804.122215-2-ben@bens.haus>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240603012804.122215-2-ben@bens.haus>
User-Agent: NeoMutt/20180716

On Sunday 02 June 2024 18:26:38 Benjamin Schneider wrote:
> This frequency was disabled because of unresolved stability problems.
> However, based on several months of testing, the source of the
> stability problems seems to be the bootloader, not the kernel.
> Marvell has recently merged changes to their bootloader source that
> addresses the stability issues when frequency scaling is enabled at
> all frequencies including 1.2Ghz.
> 
> Signed-off-by: Benjamin Schneider <ben@bens.haus>
> ---
>  drivers/cpufreq/armada-37xx-cpufreq.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c
> index bea41ccab..f28a4435f 100644
> --- a/drivers/cpufreq/armada-37xx-cpufreq.c
> +++ b/drivers/cpufreq/armada-37xx-cpufreq.c
> @@ -102,11 +102,7 @@ struct armada_37xx_dvfs {
>  };
>  
>  static struct armada_37xx_dvfs armada_37xx_dvfs[] = {
> -	/*
> -	 * The cpufreq scaling for 1.2 GHz variant of the SOC is currently
> -	 * unstable because we do not know how to configure it properly.
> -	 */
> -	/* {.cpu_freq_max = 1200*1000*1000, .divider = {1, 2, 4, 6} }, */
> +	{.cpu_freq_max = 1200*1000*1000, .divider = {1, 2, 4, 6} },
>  	{.cpu_freq_max = 1000*1000*1000, .divider = {1, 2, 4, 5} },
>  	{.cpu_freq_max = 800*1000*1000,  .divider = {1, 2, 3, 4} },
>  	{.cpu_freq_max = 600*1000*1000,  .divider = {2, 4, 5, 6} },
> -- 
> 2.45.1

As without the updated firmware on 1.2 GHz variant of the SoC is kernel
already crashing, even with commented line for .cpu_freq_max = 1200,
this change makes sense.

There is no reason to have 1.2 GHz line disabled as it does not solve
any issue (as was originally thought) and just prevent people with
updated firmware to use non-performance governor on that SoC.
(When cpufreq driver is not loaded then CPU frequency of the SoC is
locked at the max speed, which has observed behavior same as performance
cpufreq governor).

So, go ahead with this change with my

Reviewed-by: Pali Rohár <pali@kernel.org>