[PATCH] Revert "mvebu: 5.10 fix DVFS caused random boot crashes"
Hauke Mehrtens
hauke at hauke-m.de
Sat Jul 24 09:31:29 PDT 2021
Hi Robert,
Could you plase comment on this?
Hauke
On 7/19/21 1:46 PM, Josef Schlehofer wrote:
> Based on the discussion on the mailing list [1], the patch which was
> reverted, it reverts only one patch without the subsequent ones.
>
> This leads to the SoC scaling issue not using a CPU parent clock, but
> it uses DDR clock. This is done for all variants, and it's wrong because
> commits (hacks) that were using the DDR clock are no longer in the mainline kernel.
>
> If someone has stability issues on 1.2 GHz, it should not affect all
> routers (1 GHz, 800 MHz) and it should be rather consulted with guys, who are trying to
> improve the situation in the kernel and not making the situation worse.
>
> There are two solutions in cases of instability:
> a) disable cpufreq
> b) underclock it up to 1 GHz
>
> This reverts commit 080a0b74e39d159eecf69c468debec42f28bf4d8.
>
> [1] https://lists.openwrt.org/pipermail/openwrt-devel/2021-June/035702.html
>
> CC: Pali Rohár <pali at kernel.org>
> Signed-off-by: Josef Schlehofer <pepe.schlehofer at gmail.com>
> ---
> ...rmada-37xx-Fix-setting-TBG-parent-fo.patch | 107 ------------------
> ...rmada-37xx-Fix-setting-TBG-parent-fo.patch | 107 ------------------
> 2 files changed, 214 deletions(-)
> delete mode 100644 target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
> delete mode 100644 target/linux/mvebu/patches-5.4/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
>
> diff --git a/target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch b/target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
> deleted file mode 100644
> index 65184df584..0000000000
> --- a/target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
> +++ /dev/null
> @@ -1,107 +0,0 @@
> -From 35639bac13927d1476398b740b11cbed0ee3ddb2 Mon Sep 17 00:00:00 2001
> -From: Robert Marko <robert.marko at sartura.hr>
> -Date: Tue, 18 May 2021 13:24:30 +0200
> -Subject: [PATCH] Revert "cpufreq: armada-37xx: Fix setting TBG parent for load
> - levels"
> -
> -This reverts commit a13b110e7c9e0dc2edcc7a19d4255fc88abd83cc.
> -
> -This patch actually corrects the things so that 1 or 1.2GHz models would
> -actually get scaled to their native frequency.
> -
> -However, due to a AVS setting voltages too low this will cause random
> -crashes on 1.2GHz models.
> -
> -So, until a new safe for everybody voltage is agreed on
> -lets revert the patch.
> -
> -Signed-off-by: Robert Marko <robert.marko at sartura.hr>
> ----
> - drivers/cpufreq/armada-37xx-cpufreq.c | 35 +++++++++------------------
> - 1 file changed, 12 insertions(+), 23 deletions(-)
> -
> ---- a/drivers/cpufreq/armada-37xx-cpufreq.c
> -+++ b/drivers/cpufreq/armada-37xx-cpufreq.c
> -@@ -25,10 +25,6 @@
> -
> - #include "cpufreq-dt.h"
> -
> --/* Clk register set */
> --#define ARMADA_37XX_CLK_TBG_SEL 0
> --#define ARMADA_37XX_CLK_TBG_SEL_CPU_OFF 22
> --
> - /* Power management in North Bridge register set */
> - #define ARMADA_37XX_NB_L0L1 0x18
> - #define ARMADA_37XX_NB_L2L3 0x1C
> -@@ -126,15 +122,10 @@ static struct armada_37xx_dvfs *armada_3
> - * will be configured then the DVFS will be enabled.
> - */
> - static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
> -- struct regmap *clk_base, u8 *divider)
> -+ struct clk *clk, u8 *divider)
> - {
> -- u32 cpu_tbg_sel;
> - int load_lvl;
> --
> -- /* Determine to which TBG clock is CPU connected */
> -- regmap_read(clk_base, ARMADA_37XX_CLK_TBG_SEL, &cpu_tbg_sel);
> -- cpu_tbg_sel >>= ARMADA_37XX_CLK_TBG_SEL_CPU_OFF;
> -- cpu_tbg_sel &= ARMADA_37XX_NB_TBG_SEL_MASK;
> -+ struct clk *parent;
> -
> - for (load_lvl = 0; load_lvl < LOAD_LEVEL_NR; load_lvl++) {
> - unsigned int reg, mask, val, offset = 0;
> -@@ -153,11 +144,6 @@ static void __init armada37xx_cpufreq_dv
> - mask = (ARMADA_37XX_NB_CLK_SEL_MASK
> - << ARMADA_37XX_NB_CLK_SEL_OFF);
> -
> -- /* Set TBG index, for all levels we use the same TBG */
> -- val = cpu_tbg_sel << ARMADA_37XX_NB_TBG_SEL_OFF;
> -- mask = (ARMADA_37XX_NB_TBG_SEL_MASK
> -- << ARMADA_37XX_NB_TBG_SEL_OFF);
> --
> - /*
> - * Set cpu divider based on the pre-computed array in
> - * order to have balanced step.
> -@@ -176,6 +162,14 @@ static void __init armada37xx_cpufreq_dv
> -
> - regmap_update_bits(base, reg, mask, val);
> - }
> -+
> -+ /*
> -+ * Set cpu clock source, for all the level we keep the same
> -+ * clock source that the one already configured. For this one
> -+ * we need to use the clock framework
> -+ */
> -+ parent = clk_get_parent(clk);
> -+ clk_set_parent(clk, parent);
> - }
> -
> - /*
> -@@ -401,16 +395,11 @@ static int __init armada37xx_cpufreq_dri
> - struct platform_device *pdev;
> - unsigned long freq;
> - unsigned int cur_frequency, base_frequency;
> -- struct regmap *nb_clk_base, *nb_pm_base, *avs_base;
> -+ struct regmap *nb_pm_base, *avs_base;
> - struct device *cpu_dev;
> - int load_lvl, ret;
> - struct clk *clk, *parent;
> -
> -- nb_clk_base =
> -- syscon_regmap_lookup_by_compatible("marvell,armada-3700-periph-clock-nb");
> -- if (IS_ERR(nb_clk_base))
> -- return -ENODEV;
> --
> - nb_pm_base =
> - syscon_regmap_lookup_by_compatible("marvell,armada-3700-nb-pm");
> -
> -@@ -487,7 +476,7 @@ static int __init armada37xx_cpufreq_dri
> - armada37xx_cpufreq_avs_configure(avs_base, dvfs);
> - armada37xx_cpufreq_avs_setup(avs_base, dvfs);
> -
> -- armada37xx_cpufreq_dvfs_setup(nb_pm_base, nb_clk_base, dvfs->divider);
> -+ armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);
> - clk_put(clk);
> -
> - for (load_lvl = ARMADA_37XX_DVFS_LOAD_0; load_lvl < LOAD_LEVEL_NR;
> diff --git a/target/linux/mvebu/patches-5.4/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch b/target/linux/mvebu/patches-5.4/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
> deleted file mode 100644
> index 65184df584..0000000000
> --- a/target/linux/mvebu/patches-5.4/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
> +++ /dev/null
> @@ -1,107 +0,0 @@
> -From 35639bac13927d1476398b740b11cbed0ee3ddb2 Mon Sep 17 00:00:00 2001
> -From: Robert Marko <robert.marko at sartura.hr>
> -Date: Tue, 18 May 2021 13:24:30 +0200
> -Subject: [PATCH] Revert "cpufreq: armada-37xx: Fix setting TBG parent for load
> - levels"
> -
> -This reverts commit a13b110e7c9e0dc2edcc7a19d4255fc88abd83cc.
> -
> -This patch actually corrects the things so that 1 or 1.2GHz models would
> -actually get scaled to their native frequency.
> -
> -However, due to a AVS setting voltages too low this will cause random
> -crashes on 1.2GHz models.
> -
> -So, until a new safe for everybody voltage is agreed on
> -lets revert the patch.
> -
> -Signed-off-by: Robert Marko <robert.marko at sartura.hr>
> ----
> - drivers/cpufreq/armada-37xx-cpufreq.c | 35 +++++++++------------------
> - 1 file changed, 12 insertions(+), 23 deletions(-)
> -
> ---- a/drivers/cpufreq/armada-37xx-cpufreq.c
> -+++ b/drivers/cpufreq/armada-37xx-cpufreq.c
> -@@ -25,10 +25,6 @@
> -
> - #include "cpufreq-dt.h"
> -
> --/* Clk register set */
> --#define ARMADA_37XX_CLK_TBG_SEL 0
> --#define ARMADA_37XX_CLK_TBG_SEL_CPU_OFF 22
> --
> - /* Power management in North Bridge register set */
> - #define ARMADA_37XX_NB_L0L1 0x18
> - #define ARMADA_37XX_NB_L2L3 0x1C
> -@@ -126,15 +122,10 @@ static struct armada_37xx_dvfs *armada_3
> - * will be configured then the DVFS will be enabled.
> - */
> - static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
> -- struct regmap *clk_base, u8 *divider)
> -+ struct clk *clk, u8 *divider)
> - {
> -- u32 cpu_tbg_sel;
> - int load_lvl;
> --
> -- /* Determine to which TBG clock is CPU connected */
> -- regmap_read(clk_base, ARMADA_37XX_CLK_TBG_SEL, &cpu_tbg_sel);
> -- cpu_tbg_sel >>= ARMADA_37XX_CLK_TBG_SEL_CPU_OFF;
> -- cpu_tbg_sel &= ARMADA_37XX_NB_TBG_SEL_MASK;
> -+ struct clk *parent;
> -
> - for (load_lvl = 0; load_lvl < LOAD_LEVEL_NR; load_lvl++) {
> - unsigned int reg, mask, val, offset = 0;
> -@@ -153,11 +144,6 @@ static void __init armada37xx_cpufreq_dv
> - mask = (ARMADA_37XX_NB_CLK_SEL_MASK
> - << ARMADA_37XX_NB_CLK_SEL_OFF);
> -
> -- /* Set TBG index, for all levels we use the same TBG */
> -- val = cpu_tbg_sel << ARMADA_37XX_NB_TBG_SEL_OFF;
> -- mask = (ARMADA_37XX_NB_TBG_SEL_MASK
> -- << ARMADA_37XX_NB_TBG_SEL_OFF);
> --
> - /*
> - * Set cpu divider based on the pre-computed array in
> - * order to have balanced step.
> -@@ -176,6 +162,14 @@ static void __init armada37xx_cpufreq_dv
> -
> - regmap_update_bits(base, reg, mask, val);
> - }
> -+
> -+ /*
> -+ * Set cpu clock source, for all the level we keep the same
> -+ * clock source that the one already configured. For this one
> -+ * we need to use the clock framework
> -+ */
> -+ parent = clk_get_parent(clk);
> -+ clk_set_parent(clk, parent);
> - }
> -
> - /*
> -@@ -401,16 +395,11 @@ static int __init armada37xx_cpufreq_dri
> - struct platform_device *pdev;
> - unsigned long freq;
> - unsigned int cur_frequency, base_frequency;
> -- struct regmap *nb_clk_base, *nb_pm_base, *avs_base;
> -+ struct regmap *nb_pm_base, *avs_base;
> - struct device *cpu_dev;
> - int load_lvl, ret;
> - struct clk *clk, *parent;
> -
> -- nb_clk_base =
> -- syscon_regmap_lookup_by_compatible("marvell,armada-3700-periph-clock-nb");
> -- if (IS_ERR(nb_clk_base))
> -- return -ENODEV;
> --
> - nb_pm_base =
> - syscon_regmap_lookup_by_compatible("marvell,armada-3700-nb-pm");
> -
> -@@ -487,7 +476,7 @@ static int __init armada37xx_cpufreq_dri
> - armada37xx_cpufreq_avs_configure(avs_base, dvfs);
> - armada37xx_cpufreq_avs_setup(avs_base, dvfs);
> -
> -- armada37xx_cpufreq_dvfs_setup(nb_pm_base, nb_clk_base, dvfs->divider);
> -+ armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);
> - clk_put(clk);
> -
> - for (load_lvl = ARMADA_37XX_DVFS_LOAD_0; load_lvl < LOAD_LEVEL_NR;
>
More information about the openwrt-devel
mailing list