linux-kernel - Re: [PATCH 1/2] clk: mvebu: armada-37xx-periph: Fix switching CPU rate from 300Mhz to 1.2GHz

Open Source and information security mailing list archives

Message-ID: <87tvpl4pql.fsf@bootlin.com>
Date:   Fri, 29 Jun 2018 16:44:02 +0200
From:   Gregory CLEMENT <gregory.clement@...tlin.com>
To:     Stephen Boyd <sboyd@...eaurora.org>,
        Mike Turquette <mturquette@...libre.com>
Cc:     linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Jason Cooper <jason@...edaemon.net>,
        Hua Jing <jinghua@...vell.com>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        stable@...r.kernel.org,
        Maxime Chevallier <maxime.chevallier@...tlin.com>,
        Nadav Haklai <nadavh@...vell.com>, Ken Ma <make@...vell.com>,
        Victor Gu <xigu@...vell.com>,
        Neta Zur Hershkovits <neta@...vell.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Miquèl Raynal <miquel.raynal@...tlin.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Wilson Ding <dingwei@...vell.com>,
        linux-arm-kernel@...ts.infradead.org,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH 1/2] clk: mvebu: armada-37xx-periph: Fix switching CPU rate from 300Mhz to 1.2GHz

Hi,

 On mar., juin 19 2018, Gregory CLEMENT <gregory.clement@...tlin.com> wrote:

> Switching the CPU from the L2 or L3 frequencies (300 and 200 Mhz
> respectively) to L0 frequency (1.2 Ghz) requires a significant amount
> of time to let VDD stabilize to the appropriate voltage. This amount of
> time is large enough that it cannot be covered by the hardware
> countdown register. Due to this, the CPU might start operating at L0
> before the voltage is stabilized, leading to CPU stalls.
>
> To work around this problem, we prevent switching directly from the
> L2/L3 frequencies to the L0 frequency, and instead switch to the L1
> frequency in-between. The sequence therefore becomes:
>
> 1. First switch from L2/L3(200/300MHz) to L1(600MHZ)
> 2. Sleep 20ms for stabling VDD voltage
> 3. Then switch from L1(600MHZ) to L0(1200Mhz).

Do you have any comment on this fix?

Gregory

>
> It is based on the work done by Ken Ma <make@...vell.com>
>
> Cc: stable@...r.kernel.org
> Fixes: 2089dc33ea0e ("clk: mvebu: armada-37xx-periph: add DVFS support for cpu clocks")
> Signed-off-by: Gregory CLEMENT <gregory.clement@...tlin.com>
> ---
>  drivers/clk/mvebu/armada-37xx-periph.c | 38 ++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> index 6860bd5a37c5..44e4e27eddad 100644
> --- a/drivers/clk/mvebu/armada-37xx-periph.c
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -35,6 +35,7 @@
>  #define CLK_SEL		0x10
>  #define CLK_DIS		0x14
>  
> +#define  ARMADA_37XX_DVFS_LOAD_1 1
>  #define LOAD_LEVEL_NR	4
>  
>  #define ARMADA_37XX_NB_L0L1	0x18
> @@ -507,6 +508,40 @@ static long clk_pm_cpu_round_rate(struct clk_hw *hw, unsigned long rate,
>  	return -EINVAL;
>  }
>  
> +/*
> + * Switching the CPU from the L2 or L3 frequencies (300 and 200 Mhz
> + * respectively) to L0 frequency (1.2 Ghz) requires a significant
> + * amount of time to let VDD stabilize to the appropriate
> + * voltage. This amount of time is large enough that it cannot be
> + * covered by the hardware countdown register. Due to this, the CPU
> + * might start operating at L0 before the voltage is stabilized,
> + * leading to CPU stalls.
> + *
> + * To work around this problem, we prevent switching directly from the
> + * L2/L3 frequencies to the L0 frequency, and instead switch to the L1
> + * frequency in-between. The sequence therefore becomes:
> + * 1. First switch from L2/L3(200/300MHz) to L1(600MHZ)
> + * 2. Sleep 20ms for stabling VDD voltage
> + * 3. Then switch from L1(600MHZ) to L0(1200Mhz).
> + */
> +static void clk_pm_cpu_set_rate_wa(unsigned long rate, struct regmap *base)
> +{
> +	unsigned int cur_level;
> +
> +	if (rate != 1200 * 1000 * 1000)
> +		return;
> +
> +	regmap_read(base, ARMADA_37XX_NB_CPU_LOAD, &cur_level);
> +	cur_level &= ARMADA_37XX_NB_CPU_LOAD_MASK;
> +	if (cur_level <= ARMADA_37XX_DVFS_LOAD_1)
> +		return;
> +
> +	regmap_update_bits(base, ARMADA_37XX_NB_CPU_LOAD,
> +			   ARMADA_37XX_NB_CPU_LOAD_MASK,
> +			   ARMADA_37XX_DVFS_LOAD_1);
> +	msleep(20);
> +}
> +
>  static int clk_pm_cpu_set_rate(struct clk_hw *hw, unsigned long rate,
>  			       unsigned long parent_rate)
>  {
> @@ -537,6 +572,9 @@ static int clk_pm_cpu_set_rate(struct clk_hw *hw, unsigned long rate,
>  			 */
>  			reg = ARMADA_37XX_NB_CPU_LOAD;
>  			mask = ARMADA_37XX_NB_CPU_LOAD_MASK;
> +
> +			clk_pm_cpu_set_rate_wa(rate, base);
> +
>  			regmap_update_bits(base, reg, mask, load_level);
>  
>  			return rate;
> -- 
> 2.17.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives