diff mbox series

mvebu: 5.10: fix DVFS caused random boot crashes

Message ID 20210518170341.2953450-1-robert.marko@sartura.hr
State Superseded
Headers show
Series mvebu: 5.10: fix DVFS caused random boot crashes | expand

Commit Message

Robert Marko May 18, 2021, 5:03 p.m. UTC
5.10.37 introduced a lot of DVFS changes for Armada 37xx from 5.13 kernel.

Unfortunately commit:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/cpufreq/armada-37xx-cpufreq.c?h=v5.10.37&id=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.

Fixes: d337731 ("kernel: bump 5.10 to 5.10.37")
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
---
 ...rmada-37xx-Fix-setting-TBG-parent-fo.patch | 101 ++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch

Comments

Hauke Mehrtens May 18, 2021, 6:36 p.m. UTC | #1
On 5/18/21 7:03 PM, Robert Marko wrote:
> 5.10.37 introduced a lot of DVFS changes for Armada 37xx from 5.13 kernel.
> 
> Unfortunately commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/cpufreq/armada-37xx-cpufreq.c?h=v5.10.37&id=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.
> 
> Fixes: d337731 ("kernel: bump 5.10 to 5.10.37")
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>

This commit was also backported to v5.4.119 and is now in OpenWrt 21.02 
branch, we should probably also revbert it there.

@Robert Are you working with upstream Linux to fix this problem upstream 
too? For now this revert should be fine.

hauke
Robert Marko May 18, 2021, 6:41 p.m. UTC | #2
On Tue, May 18, 2021 at 8:36 PM Hauke Mehrtens <hauke@hauke-m.de> wrote:
>
> On 5/18/21 7:03 PM, Robert Marko wrote:
> > 5.10.37 introduced a lot of DVFS changes for Armada 37xx from 5.13 kernel.
> >
> > Unfortunately commit:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/cpufreq/armada-37xx-cpufreq.c?h=v5.10.37&id=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.
> >
> > Fixes: d337731 ("kernel: bump 5.10 to 5.10.37")
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>
> This commit was also backported to v5.4.119 and is now in OpenWrt 21.02
> branch, we should probably also revbert it there.

Ok, I can make a patch for that as well as part of v2.
>
> @Robert Are you working with upstream Linux to fix this problem upstream
> too? For now this revert should be fine.

Yes, I was talking to Pali who did most of the CPUFreq changes to see
what can be done.
Essentially, it seems like Marvell is not cooperating at all, their
docs state incorrect recommended voltage, etc.
I was only able to get my board stable under 30m stress test with 1.2V
which is listed as max voltage but in the boards
OTP 1.26V is hardcoded as the starting voltage, so it's currently a mess.

Robert
>
> hauke
diff mbox series

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
new file mode 100644
index 0000000000..1a44d89ac9
--- /dev/null
+++ b/target/linux/mvebu/patches-5.10/803-Revert-cpufreq-armada-37xx-Fix-setting-TBG-parent-fo.patch
@@ -0,0 +1,101 @@ 
+From 2dd652976b099f4234fcb8812a2d504bb78f5a24 Mon Sep 17 00:00:00 2001
+From: Robert Marko <robert.marko@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 commit broke Espressobin Ultra boards with weird and random
+kernel crashes during booting.
+
+Signed-off-by: Robert Marko <robert.marko@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;