From: "Pali Rohár" <pali@kernel.org> To: Gregory Clement <gregory.clement@bootlin.com>, Andrew Lunn <andrew@lunn.ch>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org Cc: "Marek Behún" <kabel@kernel.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Tomasz Maciej Nowak" <tmn505@gmail.com>, "Luka Perkov" <luka.perkov@sartura.hr>, "Andre Heider" <a.heider@gmail.com>, "Vladimir Vid" <vladimir.vid@sartura.hr>, "Russell King" <rmk+kernel@armlinux.org.uk>, "Gérald Kerma" <gerald@gk2.net>, "Konstantin Porotchkin" <kostap@marvell.com> Subject: [PATCH mvebu v2 02/10] cpufreq: armada-37xx: Fix setting TBG parent for load levels Date: Thu, 14 Jan 2021 13:40:24 +0100 [thread overview] Message-ID: <20210114124032.12765-3-pali@kernel.org> (raw) In-Reply-To: <20210114124032.12765-1-pali@kernel.org> From: Marek Behún <kabel@kernel.org> With CPU frequency determining software [1] we have discovered that after this driver does one CPU frequency change, the base frequency of the CPU is set to the frequency of TBG-A-P clock, instead of the TBG that is parent to the CPU. This can be reproduced on EspressoBIN and Turris MOX: cd /sys/devices/system/cpu/cpufreq/policy0 echo powersave >scaling_governor echo performance >scaling_governor Running the mhz tool before this driver is loaded reports 1000 MHz, and after loading the driver and executing commands above the tool reports 800 MHz. The change of TBG clock selector is supposed to happen in function armada37xx_cpufreq_dvfs_setup. Before the function returns, it does this: parent = clk_get_parent(clk); clk_set_parent(clk, parent); The armada-37xx-periph clock driver has the .set_parent method implemented correctly for this, so if the method was actually called, this would work. But since the introduction of the common clock framework in commit b2476490ef11 ("clk: introduce the common clock..."), the clk_set_parent function checks whether the parent is actually changing, and if the requested new parent is same as the old parent (which is obviously the case for the code above), the .set_parent method is not called at all. This patch fixes this issue by filling the correct TBG clock selector directly in the armada37xx_cpufreq_dvfs_setup during the filling of other registers at the same address. But the determination of CPU TBG index cannot be done via the common clock framework, therefore we need to access the North Bridge Peripheral Clock registers directly in this driver. [1] https://github.com/wtarreau/mhz Signed-off-by: Marek Behún <kabel@kernel.org> Fixes: 92ce45fb875d ("cpufreq: Add DVFS support for Armada 37xx") Cc: stable@vger.kernel.org Cc: Gregory CLEMENT <gregory.clement@bootlin.com> Cc: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/cpufreq/armada-37xx-cpufreq.c | 35 ++++++++++++++++++--------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c index b4af4094309b..b8dc6c849579 100644 --- a/drivers/cpufreq/armada-37xx-cpufreq.c +++ b/drivers/cpufreq/armada-37xx-cpufreq.c @@ -25,6 +25,10 @@ #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 @@ -120,10 +124,15 @@ static struct armada_37xx_dvfs *armada_37xx_cpu_freq_info_get(u32 freq) * will be configured then the DVFS will be enabled. */ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, - struct clk *clk, u8 *divider) + struct regmap *clk_base, u8 *divider) { + u32 cpu_tbg_sel; int load_lvl; - struct clk *parent; + + /* 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; for (load_lvl = 0; load_lvl < LOAD_LEVEL_NR; load_lvl++) { unsigned int reg, mask, val, offset = 0; @@ -142,6 +151,11 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, 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. @@ -160,14 +174,6 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, 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); } /* @@ -358,11 +364,16 @@ static int __init armada37xx_cpufreq_driver_init(void) struct platform_device *pdev; unsigned long freq; unsigned int cur_frequency, base_frequency; - struct regmap *nb_pm_base, *avs_base; + struct regmap *nb_clk_base, *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"); @@ -439,7 +450,7 @@ static int __init armada37xx_cpufreq_driver_init(void) armada37xx_cpufreq_avs_configure(avs_base, dvfs); armada37xx_cpufreq_avs_setup(avs_base, dvfs); - armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider); + armada37xx_cpufreq_dvfs_setup(nb_pm_base, nb_clk_base, dvfs->divider); clk_put(clk); for (load_lvl = ARMADA_37XX_DVFS_LOAD_0; load_lvl < LOAD_LEVEL_NR; -- 2.20.1
WARNING: multiple messages have this Message-ID (diff)
From: "Pali Rohár" <pali@kernel.org> To: Gregory Clement <gregory.clement@bootlin.com>, Andrew Lunn <andrew@lunn.ch>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org Cc: "Marek Behún" <kabel@kernel.org>, "Tomasz Maciej Nowak" <tmn505@gmail.com>, "Luka Perkov" <luka.perkov@sartura.hr>, "Andre Heider" <a.heider@gmail.com>, "Vladimir Vid" <vladimir.vid@sartura.hr>, "Russell King" <rmk+kernel@armlinux.org.uk>, "Gérald Kerma" <gerald@gk2.net>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Konstantin Porotchkin" <kostap@marvell.com> Subject: [PATCH mvebu v2 02/10] cpufreq: armada-37xx: Fix setting TBG parent for load levels Date: Thu, 14 Jan 2021 13:40:24 +0100 [thread overview] Message-ID: <20210114124032.12765-3-pali@kernel.org> (raw) In-Reply-To: <20210114124032.12765-1-pali@kernel.org> From: Marek Behún <kabel@kernel.org> With CPU frequency determining software [1] we have discovered that after this driver does one CPU frequency change, the base frequency of the CPU is set to the frequency of TBG-A-P clock, instead of the TBG that is parent to the CPU. This can be reproduced on EspressoBIN and Turris MOX: cd /sys/devices/system/cpu/cpufreq/policy0 echo powersave >scaling_governor echo performance >scaling_governor Running the mhz tool before this driver is loaded reports 1000 MHz, and after loading the driver and executing commands above the tool reports 800 MHz. The change of TBG clock selector is supposed to happen in function armada37xx_cpufreq_dvfs_setup. Before the function returns, it does this: parent = clk_get_parent(clk); clk_set_parent(clk, parent); The armada-37xx-periph clock driver has the .set_parent method implemented correctly for this, so if the method was actually called, this would work. But since the introduction of the common clock framework in commit b2476490ef11 ("clk: introduce the common clock..."), the clk_set_parent function checks whether the parent is actually changing, and if the requested new parent is same as the old parent (which is obviously the case for the code above), the .set_parent method is not called at all. This patch fixes this issue by filling the correct TBG clock selector directly in the armada37xx_cpufreq_dvfs_setup during the filling of other registers at the same address. But the determination of CPU TBG index cannot be done via the common clock framework, therefore we need to access the North Bridge Peripheral Clock registers directly in this driver. [1] https://github.com/wtarreau/mhz Signed-off-by: Marek Behún <kabel@kernel.org> Fixes: 92ce45fb875d ("cpufreq: Add DVFS support for Armada 37xx") Cc: stable@vger.kernel.org Cc: Gregory CLEMENT <gregory.clement@bootlin.com> Cc: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/cpufreq/armada-37xx-cpufreq.c | 35 ++++++++++++++++++--------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c index b4af4094309b..b8dc6c849579 100644 --- a/drivers/cpufreq/armada-37xx-cpufreq.c +++ b/drivers/cpufreq/armada-37xx-cpufreq.c @@ -25,6 +25,10 @@ #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 @@ -120,10 +124,15 @@ static struct armada_37xx_dvfs *armada_37xx_cpu_freq_info_get(u32 freq) * will be configured then the DVFS will be enabled. */ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, - struct clk *clk, u8 *divider) + struct regmap *clk_base, u8 *divider) { + u32 cpu_tbg_sel; int load_lvl; - struct clk *parent; + + /* 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; for (load_lvl = 0; load_lvl < LOAD_LEVEL_NR; load_lvl++) { unsigned int reg, mask, val, offset = 0; @@ -142,6 +151,11 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, 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. @@ -160,14 +174,6 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base, 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); } /* @@ -358,11 +364,16 @@ static int __init armada37xx_cpufreq_driver_init(void) struct platform_device *pdev; unsigned long freq; unsigned int cur_frequency, base_frequency; - struct regmap *nb_pm_base, *avs_base; + struct regmap *nb_clk_base, *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"); @@ -439,7 +450,7 @@ static int __init armada37xx_cpufreq_driver_init(void) armada37xx_cpufreq_avs_configure(avs_base, dvfs); armada37xx_cpufreq_avs_setup(avs_base, dvfs); - armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider); + armada37xx_cpufreq_dvfs_setup(nb_pm_base, nb_clk_base, dvfs->divider); clk_put(clk); for (load_lvl = ARMADA_37XX_DVFS_LOAD_0; load_lvl < LOAD_LEVEL_NR; -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-14 12:41 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-14 12:40 [PATCH mvebu v2 00/10] Armada 37xx: Fix cpufreq changing base CPU speed to 800 MHz from 1000 MHz Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-01-14 12:40 ` [PATCH mvebu v2 01/10] arm64: dts: marvell: armada-37xx: add syscon compatible to NB clk node Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-04-02 19:50 ` Gregory CLEMENT 2021-04-02 19:50 ` Gregory CLEMENT 2021-01-14 12:40 ` Pali Rohár [this message] 2021-01-14 12:40 ` [PATCH mvebu v2 02/10] cpufreq: armada-37xx: Fix setting TBG parent for load levels Pali Rohár 2021-01-14 12:40 ` [PATCH mvebu v2 03/10] clk: mvebu: armada-37xx-periph: remove .set_parent method for CPU PM clock Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-02-10 1:58 ` Stephen Boyd 2021-02-10 1:58 ` Stephen Boyd 2021-01-14 12:40 ` [PATCH mvebu v2 04/10] cpufreq: armada-37xx: Fix the AVS value for loads L0 and L1 Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-01-14 12:40 ` [PATCH mvebu v2 05/10] clk: mvebu: armada-37xx-periph: Fix switching CPU freq from 250 Mhz to 1 GHz Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-02-10 1:58 ` Stephen Boyd 2021-02-10 1:58 ` Stephen Boyd 2021-01-14 12:40 ` [PATCH mvebu v2 06/10] clk: mvebu: armada-37xx-periph: Fix workaround for switching from L1 to L0 Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-02-10 1:58 ` Stephen Boyd 2021-02-10 1:58 ` Stephen Boyd 2021-01-14 12:40 ` [PATCH mvebu v2 07/10] cpufreq: armada-37xx: Fix driver cleanup when registration failed Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-01-14 12:40 ` [PATCH mvebu v2 08/10] cpufreq: armada-37xx: Fix determining base CPU frequency Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-01-14 12:40 ` [PATCH mvebu v2 09/10] cpufreq: armada-37xx: Remove cur_frequency variable Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-03-29 15:00 ` Gregory CLEMENT 2021-03-29 15:00 ` Gregory CLEMENT 2021-03-29 21:44 ` Marek Behún 2021-03-29 21:44 ` Marek Behún 2021-01-14 12:40 ` [PATCH mvebu v2 10/10] cpufreq: armada-37xx: Fix module unloading Pali Rohár 2021-01-14 12:40 ` Pali Rohár 2021-02-01 14:35 ` [PATCH mvebu v2 00/10] Armada 37xx: Fix cpufreq changing base CPU speed to 800 MHz from 1000 MHz Tomasz Maciej Nowak 2021-02-01 14:35 ` Tomasz Maciej Nowak 2021-02-03 19:29 ` Anders Trier Olesen 2021-02-03 19:29 ` Anders Trier Olesen 2021-02-22 19:41 ` [PATCH mvebu v3 " Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-02-22 19:41 ` [PATCH mvebu v3 01/10] arm64: dts: marvell: armada-37xx: add syscon compatible to NB clk node Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-02-22 19:41 ` [PATCH mvebu v3 02/10] cpufreq: armada-37xx: Fix setting TBG parent for load levels Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:45 ` Gregory CLEMENT 2021-03-29 14:45 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 03/10] clk: mvebu: armada-37xx-periph: remove .set_parent method for CPU PM clock Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:46 ` Gregory CLEMENT 2021-03-29 14:46 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 04/10] cpufreq: armada-37xx: Fix the AVS value for load L1 Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:47 ` Gregory CLEMENT 2021-03-29 14:47 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 05/10] clk: mvebu: armada-37xx-periph: Fix switching CPU freq from 250 Mhz to 1 GHz Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:48 ` Gregory CLEMENT 2021-03-29 14:48 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 06/10] clk: mvebu: armada-37xx-periph: Fix workaround for switching from L1 to L0 Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:49 ` Gregory CLEMENT 2021-03-29 14:49 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 07/10] cpufreq: armada-37xx: Fix driver cleanup when registration failed Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:58 ` Gregory CLEMENT 2021-03-29 14:58 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 08/10] cpufreq: armada-37xx: Fix determining base CPU frequency Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-29 14:59 ` Gregory CLEMENT 2021-03-29 14:59 ` Gregory CLEMENT 2021-02-22 19:41 ` [PATCH mvebu v3 09/10] cpufreq: armada-37xx: Remove cur_frequency variable Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-02-22 19:41 ` [PATCH mvebu v3 10/10] cpufreq: armada-37xx: Fix module unloading Pali Rohár 2021-02-22 19:41 ` Pali Rohár 2021-03-01 19:20 ` [PATCH mvebu v3 00/10] Armada 37xx: Fix cpufreq changing base CPU speed to 800 MHz from 1000 MHz Pali Rohár 2021-03-01 19:20 ` Pali Rohár 2021-03-12 9:12 ` Gregory CLEMENT 2021-03-12 9:12 ` Gregory CLEMENT 2021-03-12 9:27 ` Marek Behún 2021-03-12 9:27 ` Marek Behún 2021-03-28 11:31 ` Pali Rohár 2021-03-28 11:31 ` Pali Rohár 2021-04-08 0:38 ` Stephen Boyd 2021-04-08 0:38 ` Stephen Boyd
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210114124032.12765-3-pali@kernel.org \ --to=pali@kernel.org \ --cc=a.heider@gmail.com \ --cc=andrew@lunn.ch \ --cc=gerald@gk2.net \ --cc=gregory.clement@bootlin.com \ --cc=kabel@kernel.org \ --cc=kostap@marvell.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luka.perkov@sartura.hr \ --cc=miquel.raynal@bootlin.com \ --cc=mturquette@baylibre.com \ --cc=rmk+kernel@armlinux.org.uk \ --cc=sboyd@kernel.org \ --cc=tmn505@gmail.com \ --cc=vladimir.vid@sartura.hr \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.