Pali Rohár <[email protected]> writes:

> The original CPU voltage value for load L1 is too low for Armada 37xx SoC
> when base CPU frequency is 1000 or 1200 MHz. It leads to instabilities
> where CPU gets stuck soon after dynamic voltage scaling from load L1 to L0.
>
> Update the CPU voltage value for load L1 accordingly when base frequency is
> 1000 or 1200 MHz. The minimal L1 value for base CPU frequency 1000 MHz is
> updated from the original 1.05V to 1.108V and for 1200 MHz is updated to
> 1.155V. This minimal L1 value is used only in the case when it is lower
> than value for L0.
>
> This change fixes CPU instability issues on 1 GHz and 1.2 GHz variants of
> Espressobin and 1 GHz Turris Mox.
>
> Marvell previously for 1 GHz variant of Espressobin provided a patch [1]
> suitable only for their Marvell Linux kernel 4.4 fork which workarounded
> this issue. Patch forced CPU voltage value to 1.108V in all loads. But
> such change does not fix CPU instability issues on 1.2 GHz variants of
> Armada 3720 SoC.
>
> During testing we come to the conclusion that using 1.108V as minimal
> value for L1 load makes 1 GHz variants of Espressobin and Turris Mox boards
> stable. And similarly 1.155V for 1.2 GHz variant of Espressobin.
>
> These two values 1.108V and 1.155V are documented in Armada 3700 Hardware
> Specifications as typical initial CPU voltage values.
>
> Discussion about this issue is also at the Armbian forum [2].
>
> [1] - 
> https://github.com/MarvellEmbeddedProcessors/linux-marvell/commit/dc33b62c90696afb6adc7dbcc4ebbd48bedec269
> [2] - https://forum.armbian.com/topic/10429-how-to-make-espressobin-v7-stable/
>
> Signed-off-by: Pali Rohár <[email protected]>
> Tested-by: Tomasz Maciej Nowak <[email protected]>
> Tested-by: Anders Trier Olesen <[email protected]>
> Tested-by: Philip Soares <[email protected]>
> Fixes: 1c3528232f4b ("cpufreq: armada-37xx: Add AVS support")
> Cc: [email protected]

Acked-by: Gregory CLEMENT <[email protected]>

Thanks,

Gregory
> ---
>  drivers/cpufreq/armada-37xx-cpufreq.c | 37 +++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c 
> b/drivers/cpufreq/armada-37xx-cpufreq.c
> index b8dc6c849579..c7683d447b11 100644
> --- a/drivers/cpufreq/armada-37xx-cpufreq.c
> +++ b/drivers/cpufreq/armada-37xx-cpufreq.c
> @@ -73,6 +73,8 @@
>  #define LOAD_LEVEL_NR        4
>  
>  #define MIN_VOLT_MV 1000
> +#define MIN_VOLT_MV_FOR_L1_1000MHZ 1108
> +#define MIN_VOLT_MV_FOR_L1_1200MHZ 1155
>  
>  /*  AVS value for the corresponding voltage (in mV) */
>  static int avs_map[] = {
> @@ -208,6 +210,8 @@ static u32 armada_37xx_avs_val_match(int target_vm)
>   * - L2 & L3 voltage should be about 150mv smaller than L0 voltage.
>   * This function calculates L1 & L2 & L3 AVS values dynamically based
>   * on L0 voltage and fill all AVS values to the AVS value table.
> + * When base CPU frequency is 1000 or 1200 MHz then there is additional
> + * minimal avs value for load L1.
>   */
>  static void __init armada37xx_cpufreq_avs_configure(struct regmap *base,
>                                               struct armada_37xx_dvfs *dvfs)
> @@ -239,6 +243,19 @@ static void __init 
> armada37xx_cpufreq_avs_configure(struct regmap *base,
>               for (load_level = 1; load_level < LOAD_LEVEL_NR; load_level++)
>                       dvfs->avs[load_level] = avs_min;
>  
> +             /*
> +              * Set the avs values for load L0 and L1 when base CPU frequency
> +              * is 1000/1200 MHz to its typical initial values according to
> +              * the Armada 3700 Hardware Specifications.
> +              */
> +             if (dvfs->cpu_freq_max >= 1000*1000*1000) {
> +                     if (dvfs->cpu_freq_max >= 1200*1000*1000)
> +                             avs_min = 
> armada_37xx_avs_val_match(MIN_VOLT_MV_FOR_L1_1200MHZ);
> +                     else
> +                             avs_min = 
> armada_37xx_avs_val_match(MIN_VOLT_MV_FOR_L1_1000MHZ);
> +                     dvfs->avs[0] = dvfs->avs[1] = avs_min;
> +             }
> +
>               return;
>       }
>  
> @@ -258,6 +275,26 @@ static void __init 
> armada37xx_cpufreq_avs_configure(struct regmap *base,
>       target_vm = avs_map[l0_vdd_min] - 150;
>       target_vm = target_vm > MIN_VOLT_MV ? target_vm : MIN_VOLT_MV;
>       dvfs->avs[2] = dvfs->avs[3] = armada_37xx_avs_val_match(target_vm);
> +
> +     /*
> +      * Fix the avs value for load L1 when base CPU frequency is 1000/1200 
> MHz,
> +      * otherwise the CPU gets stuck when switching from load L1 to load L0.
> +      * Also ensure that avs value for load L1 is not higher than for L0.
> +      */
> +     if (dvfs->cpu_freq_max >= 1000*1000*1000) {
> +             u32 avs_min_l1;
> +
> +             if (dvfs->cpu_freq_max >= 1200*1000*1000)
> +                     avs_min_l1 = 
> armada_37xx_avs_val_match(MIN_VOLT_MV_FOR_L1_1200MHZ);
> +             else
> +                     avs_min_l1 = 
> armada_37xx_avs_val_match(MIN_VOLT_MV_FOR_L1_1000MHZ);
> +
> +             if (avs_min_l1 > dvfs->avs[0])
> +                     avs_min_l1 = dvfs->avs[0];
> +
> +             if (dvfs->avs[1] < avs_min_l1)
> +                     dvfs->avs[1] = avs_min_l1;
> +     }
>  }
>  
>  static void __init armada37xx_cpufreq_avs_setup(struct regmap *base,
> -- 
> 2.20.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

Reply via email to