[PATCH] Revert "mvebu: 5.10 fix DVFS caused random boot crashes"
Robert Marko
robert.marko at sartura.hr
Sat Jul 24 09:37:39 PDT 2021
On Sat, Jul 24, 2021 at 6:31 PM Hauke Mehrtens <hauke at hauke-m.de> wrote:
>
> Hi Robert,
>
> Could you plase comment on this?
Hi,
The linked discussion already contains all of the available information
and a good discussion.
Upstream still has no better solution than to blacklist CPUFreq for
the 1.2GHz models.
https://lists.openwrt.org/pipermail/openwrt-devel/2021-June/035702.html
I don't really have anything more to add, I understand the purpose of
this patch and
cant really advocate against it as it causes regression for other users.
Regards,
Robert
>
> 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;
> >
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko at sartura.hr
Web: www.sartura.hr
More information about the openwrt-devel
mailing list