Received-SPF: pass (google.com: domain of linux-kernel+bounces-203416-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 6 Jun 2024 01:44:47 +0200
From: Andrew Lunn <andrew@lunn.ch>
To: Pali =?iso-8859-1?Q?Roh=E1r?= <pali@kernel.org>
Cc: Benjamin Schneider <bschnei@gmail.com>,
	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: <dce98e50-6b50-4d4e-abe2-8419a675d25e@lunn.ch>
References: <20240603012804.122215-1-ben@bens.haus>
 <20240603012804.122215-2-ben@bens.haus>
 <20240605194422.klxtxgyljrrllkzy@pali>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240605194422.klxtxgyljrrllkzy@pali>

On Wed, Jun 05, 2024 at 09:44:22PM +0200, Pali Roh?r wrote:
> 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>

I defer to your knowledge in this matter.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew