Add CPU Frequency scaling support on Armada 37xx

Extend the documentation of the Armada 37xx SoC with the the North Bridge Power Management component.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 .../devicetree/bindings/arm/marvell/armada-37xx.txt   | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt +6 -0

index 51336e5fc761..7ad9830d9177 100644
@@ -14,3 +14,22 @@ following property before the previous one:
 Example:
 
 compatible = "marvell,armada-3720-db", "marvell,armada3720", "marvell,armada3710";
+
+
+Power management
+----------------
+
+For power management (particularly DVFS and AVS), the North Bridge

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt +3 -0

+	compatible = "marvell,armada-3700-nb-pm", "syscon";
+	reg = <0x14000 0x60>;
+}

Signature · 1 line

2.15.0

Quoted 27 lines · Gregory CLEMENT

Extend the documentation of the Armada 37xx SoC with the the North Bridge Power Management component.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 .../devicetree/bindings/arm/marvell/armada-37xx.txt   | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt +11 -0

index 51336e5fc761..7ad9830d9177 100644
@@ -14,3 +14,22 @@ following property before the previous one:
 Example:
 
 compatible = "marvell,armada-3720-db", "marvell,armada3720", "marvell,armada3710";
+
+
+Power management
+----------------
+
+For power management (particularly DVFS and AVS), the North Bridge
+Power Management component is needed:
+
+Required properties:
+- compatible     : should contain "marvell,armada-3700-nb-pm", "syscon";
+- reg            : the register start and length for the North Bridge

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt following property before the previous one: · line 32

Quoted 5 lines

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt +5 -0

+		    Power Management
+
+Example:
+
+nb_pm: nb_pm at 14000 {

Don't use underscore in node or property names. "syscon" is a better choice here.

Quoted 7 lines

Documentation/devicetree/bindings/arm/marvell/armada-37xx.txt +3 -0

+	compatible = "marvell,armada-3700-nb-pm", "syscon";
+	reg = <0x14000 0x60>;
+}

Signature · 1 line

2.15.0

drivers/cpufreq/Kconfig.arm config ARM_BIG_LITTLE_CPUFREQ · line 46

Quoted 8 lines · Gregory CLEMENT

drivers/cpufreq/Kconfig.arm +8 -0

+config ARM_VEXPRESS_SPC_CPUFREQ
+        tristate "Versatile Express SPC based CPUfreq driver"
+	depends on ARM_BIG_LITTLE_CPUFREQ && ARCH_VEXPRESS_SPC
+        help
+          This add the CPUfreq driver support for Versatile Express
+	  big.LITTLE platforms using SPC for power management.
+
+config ARM_SCPI_CPUFREQ

The order of above two must be reversed ?

Signature · 2 lines

-- 
viresh

Keep the driver files alphabetically sorted.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/cpufreq/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/cpufreq/Makefile +0 -0

index 812f9e0d01a3..d762e76887e7 100644

Quoted 14 lines · Gregory CLEMENT

Keep the driver files alphabetically sorted.

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/cpufreq/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/cpufreq/Makefile +0 -0

index 812f9e0d01a3..d762e76887e7 100644
@@ -53,22 +53,24 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)	+= arm_big_little.o
 obj-$(CONFIG_ARM_DT_BL_CPUFREQ)		+= arm_big_little_dt.o
 

drivers/cpufreq/Makefile obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ) += arm_big_little.o · line 56

Quoted 2 lines

drivers/cpufreq/Makefile +1 -0

 obj-$(CONFIG_ARM_BRCMSTB_AVS_CPUFREQ)	+= brcmstb-avs-cpufreq.o
+obj-$(CONFIG_ACPI_CPPC_CPUFREQ)		+= cppc_cpufreq.o

Shouldn't we add them in ascending order of the whole config name and not just CPPC_CPUFREQ ?

Signature · 2 lines

-- 
viresh

This patch adds DVFS support for the Armada 37xx SoCs

There are up to four CPU frequency loads for Armada 37xx controlled by the hardware.

This driver associates the CPU load level to a frequency, then the hardware will switch while selecting a load level.

The hardware also can associate a voltage for each level (AVS support) but it is not yet supported

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/cpufreq/Kconfig.arm           |   7 +
 drivers/cpufreq/Makefile              |   1 +
 drivers/cpufreq/armada-37xx-cpufreq.c | 241 ++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

drivers/cpufreq/Kconfig.arm +7 -0

index 17625115c67f..3018ff0d068f 100644
@@ -19,6 +19,13 @@ config ACPI_CPPC_CPUFREQ
 
 	  If in doubt, say N.
 
+config ARM_ARMADA_37XX_CPUFREQ
+	tristate "Armada 37xx CPUFreq support"
+	depends on ARCH_MVEBU
+	help
+	  This adds the CPUFreq driver support for Marvell Armada 37xx SoCs.
+	  The Armada 37xx PMU supports 4 frequency and VDD levels.
+
 # big LITTLE core layer and glue drivers
 config ARM_BIG_LITTLE_CPUFREQ
 	tristate "Generic ARM big LITTLE CPUfreq driver"

drivers/cpufreq/Makefile +1 -0

index d762e76887e7..e07715ce8844 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)	+= arm_big_little.o
 # LITTLE drivers, so that it is probed last.
 obj-$(CONFIG_ARM_DT_BL_CPUFREQ)		+= arm_big_little_dt.o
 
+obj-$(CONFIG_ARM_ARMADA_37XX_CPUFREQ)	+= armada-37xx-cpufreq.o
 obj-$(CONFIG_ARM_BRCMSTB_AVS_CPUFREQ)	+= brcmstb-avs-cpufreq.o
 obj-$(CONFIG_ACPI_CPPC_CPUFREQ)		+= cppc_cpufreq.o
 obj-$(CONFIG_ARCH_DAVINCI)		+= davinci-cpufreq.o

drivers/cpufreq/armada-37xx-cpufreq.c new file +28 -0

new file mode 100644
index 000000000000..40c9a744cc6e
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * CPU frequency scaling support for Armada 37xx platform.
+ *
+ * Copyright (C) 2017 Marvell
+ *
+ * Gregory CLEMENT <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* Power management in North Bridge register set */
+#define ARMADA_37XX_NB_L0L1	0x18
+#define ARMADA_37XX_NB_L2L3	0x1C

drivers/cpufreq/armada-37xx-cpufreq.c new file +169 -0

+#define ARMADA_37XX_NB_DYN_MOD	0x24
+#define		ARMADA_37XX_NB_CLK_SEL_EN	BIT(26)
+#define		ARMADA_37XX_NB_TBG_EN		BIT(28)
+#define		ARMADA_37XX_NB_DIV_EN		BIT(29)
+#define		ARMADA_37XX_NB_VDD_EN		BIT(30)
+#define		ARMADA_37XX_NB_DFS_EN		BIT(31)
+#define ARMADA_37XX_NB_CPU_LOAD	0x30
+#define		ARMADA_37XX_NB_CPU_LOAD_MASK	0x3
+#define		ARMADA_37XX_DVFS_LOAD_0		0
+#define		ARMADA_37XX_DVFS_LOAD_1		1
+#define		ARMADA_37XX_DVFS_LOAD_2		2
+#define		ARMADA_37XX_DVFS_LOAD_3		3
+
+/*
+ * On Armada 37xx the Power management manages 4 level of CPU load,
+ * each level can be associated with a CPU clock source, a CPU
+ * divider, a VDD level, etc...
+ */
+#define LOAD_LEVEL_NR	4
+
+struct armada_37xx_dvfs {
+	u32 cpu_freq_max;
+	u8 divider[LOAD_LEVEL_NR];
+};
+
+static struct armada_37xx_dvfs armada_37xx_dvfs[] = {
+	{.cpu_freq_max = 1200*1000*1000, .divider = {1, 2, 4, 6} },
+	{.cpu_freq_max = 1000*1000*1000, .divider = {1, 2, 4, 5} },
+	{.cpu_freq_max = 800*1000*1000,  .divider = {1, 2, 3, 4} },
+	{.cpu_freq_max = 600*1000*1000,  .divider = {2, 4, 5, 6} },
+};
+
+static struct armada_37xx_dvfs *armada_37xx_cpu_freq_info_get(u32 freq)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(armada_37xx_dvfs); i++) {
+		if (freq == armada_37xx_dvfs[i].cpu_freq_max)
+			return &armada_37xx_dvfs[i];
+	}
+
+	pr_err("Unsupported CPU frequency %d MHz\n", freq/1000000);
+	return NULL;
+}
+
+/*
+ * Setup the four level managed by the hardware. Once the four level
+ * will be configured then the DVFS will be enabled.
+ */
+static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
+						 struct clk *clk, u8 *divider)
+{
+	int load_level;
+	struct clk *parent;
+
+	for (load_level = 0; load_level < LOAD_LEVEL_NR; load_level++) {
+		unsigned int reg, mask,  val, offset = 0;
+
+		if (load_level <= ARMADA_37XX_DVFS_LOAD_1)
+			reg = ARMADA_37XX_NB_L0L1;
+		else
+			reg = ARMADA_37XX_NB_L2L3;
+
+		if (load_level ==  ARMADA_37XX_DVFS_LOAD_0 ||
+		    load_level ==  ARMADA_37XX_DVFS_LOAD_2)
+			offset += ARMADA_37XX_NB_CONFIG_SHIFT;
+
+		/* Set cpu clock source, for all the level we use TBG */
+		val = ARMADA_37XX_NB_CLK_SEL_TBG << ARMADA_37XX_NB_CLK_SEL_OFF;
+		mask = (ARMADA_37XX_NB_CLK_SEL_MASK
+			<< ARMADA_37XX_NB_CLK_SEL_OFF);
+
+		/*
+		 * Set cpu divider based on the pre-computed array in
+		 * order to have balanced step.
+		 */
+		val |= divider[load_level] << ARMADA_37XX_NB_TBG_DIV_OFF;
+		mask |= (ARMADA_37XX_NB_TBG_DIV_MASK
+			<< ARMADA_37XX_NB_TBG_DIV_OFF);
+
+		/* Set VDD divider which is actually the load level. */
+		val |= load_level << ARMADA_37XX_NB_VDD_SEL_OFF;
+		mask |= (ARMADA_37XX_NB_VDD_SEL_MASK
+			<< ARMADA_37XX_NB_VDD_SEL_OFF);
+
+		val <<= offset;
+		mask <<= offset;
+
+		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);
+}
+
+static void __init armada37xx_cpufreq_disable_dvfs(struct regmap *base)
+{
+	unsigned int reg = ARMADA_37XX_NB_DYN_MOD,
+		mask = ARMADA_37XX_NB_DFS_EN;
+
+	regmap_update_bits(base, reg, mask, 0);
+}
+
+static void __init armada37xx_cpufreq_enable_dvfs(struct regmap *base)
+{
+	unsigned int val, reg = ARMADA_37XX_NB_CPU_LOAD,
+		mask = ARMADA_37XX_NB_CPU_LOAD_MASK;
+
+	/* Start with the highest load (0) */
+	val = ARMADA_37XX_DVFS_LOAD_0;
+	regmap_update_bits(base, reg, mask, val);
+
+	/* Now enable DVFS for the CPUs */
+	reg = ARMADA_37XX_NB_DYN_MOD;
+	mask =	ARMADA_37XX_NB_CLK_SEL_EN | ARMADA_37XX_NB_TBG_EN |
+		ARMADA_37XX_NB_DIV_EN | ARMADA_37XX_NB_VDD_EN |
+		ARMADA_37XX_NB_DFS_EN;
+
+	regmap_update_bits(base, reg, mask, mask);
+}
+
+static int __init armada37xx_cpufreq_driver_init(void)
+{
+	struct armada_37xx_dvfs *dvfs;
+	struct platform_device *pdev;
+	unsigned int cur_frequency;
+	struct regmap *nb_pm_base;
+	struct device *cpu_dev;
+	int load_level, ret;
+	struct clk *clk;
+
+	nb_pm_base =
+		syscon_regmap_lookup_by_compatible("marvell,armada-3700-nb-pm");
+
+	if (IS_ERR(nb_pm_base))
+		return -ENODEV;
+
+	/* Before doing any configuration on the DVFS first, disable it */
+	armada37xx_cpufreq_disable_dvfs(nb_pm_base);
+
+	/*
+	 * On CPU 0 register the operating points supported (which are
+	 * the nominal CPU frequency and full integer divisions of
+	 * it).
+	 */
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		dev_err(cpu_dev, "Cannot get CPU\n");
+		return -ENODEV;
+	}
+
+	clk = clk_get(cpu_dev, 0);
+	if (IS_ERR(clk)) {
+		dev_err(cpu_dev, "Cannot get clock for CPU0\n");
+		return PTR_ERR(clk);
+	}
+
+	/* Get nominal (current) CPU frequency */
+	cur_frequency = clk_get_rate(clk);
+	if (!cur_frequency) {
+		dev_err(cpu_dev, "Failed to get clock rate for CPU\n");
+		return -EINVAL;
+	}
+

and simply cancel the registration of the cpufreq

drivers/cpufreq/armada-37xx-cpufreq.c new file +24 -0

+	 * device.
+	 */
+	for (load_level = ARMADA_37XX_DVFS_LOAD_0; load_level < LOAD_LEVEL_NR;
+	     load_level++) {
+		unsigned long freq = dvfs->divider[load_level];
+
+		ret = dev_pm_opp_add(cpu_dev, freq, 0);
+		if (ret)
+			return ret;
+	}
+
+	/* Now that everything is setup, enable the DVFS at hardware level */
+	armada37xx_cpufreq_enable_dvfs(nb_pm_base);
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+
+	return PTR_ERR_OR_ZERO(pdev);
+}
+/* late_initcall, to guarantee the driver is loaded after A37xx clock driver */
+late_initcall(armada37xx_cpufreq_driver_init);
+
+MODULE_AUTHOR("Gregory CLEMENT <[email protected]>");
+MODULE_DESCRIPTION("Armada 37xx cpufreq driver");
+MODULE_LICENSE("GPL");

Signature · 1 line

2.15.0

Quoted 88 lines · Gregory CLEMENT

This patch adds DVFS support for the Armada 37xx SoCs

There are up to four CPU frequency loads for Armada 37xx controlled by the hardware.

This driver associates the CPU load level to a frequency, then the hardware will switch while selecting a load level.

The hardware also can associate a voltage for each level (AVS support) but it is not yet supported

Signed-off-by Gregory CLEMENT <[email protected]>

---
 drivers/cpufreq/Kconfig.arm           |   7 +
 drivers/cpufreq/Makefile              |   1 +
 drivers/cpufreq/armada-37xx-cpufreq.c | 241 ++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

drivers/cpufreq/Kconfig.arm +7 -0

index 17625115c67f..3018ff0d068f 100644
@@ -19,6 +19,13 @@ config ACPI_CPPC_CPUFREQ
 
 	  If in doubt, say N.
 
+config ARM_ARMADA_37XX_CPUFREQ
+	tristate "Armada 37xx CPUFreq support"
+	depends on ARCH_MVEBU
+	help
+	  This adds the CPUFreq driver support for Marvell Armada 37xx SoCs.
+	  The Armada 37xx PMU supports 4 frequency and VDD levels.
+
 # big LITTLE core layer and glue drivers
 config ARM_BIG_LITTLE_CPUFREQ
 	tristate "Generic ARM big LITTLE CPUfreq driver"

drivers/cpufreq/Makefile +1 -0

index d762e76887e7..e07715ce8844 100644
@@ -52,6 +52,7 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)	+= arm_big_little.o
 # LITTLE drivers, so that it is probed last.
 obj-$(CONFIG_ARM_DT_BL_CPUFREQ)		+= arm_big_little_dt.o
 
+obj-$(CONFIG_ARM_ARMADA_37XX_CPUFREQ)	+= armada-37xx-cpufreq.o
 obj-$(CONFIG_ARM_BRCMSTB_AVS_CPUFREQ)	+= brcmstb-avs-cpufreq.o
 obj-$(CONFIG_ACPI_CPPC_CPUFREQ)		+= cppc_cpufreq.o
 obj-$(CONFIG_ARCH_DAVINCI)		+= davinci-cpufreq.o

drivers/cpufreq/armada-37xx-cpufreq.c new file +33 -0

new file mode 100644
index 000000000000..40c9a744cc6e
@@ -0,0 +1,241 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * CPU frequency scaling support for Armada 37xx platform.
+ *
+ * Copyright (C) 2017 Marvell
+ *
+ * Gregory CLEMENT <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/* Power management in North Bridge register set */
+#define ARMADA_37XX_NB_L0L1	0x18
+#define ARMADA_37XX_NB_L2L3	0x1C
+#define		ARMADA_37XX_NB_TBG_DIV_OFF	13
+#define		ARMADA_37XX_NB_TBG_DIV_MASK	0x7
+#define		ARMADA_37XX_NB_CLK_SEL_OFF	11
+#define		ARMADA_37XX_NB_CLK_SEL_MASK	0x1
+#define		ARMADA_37XX_NB_CLK_SEL_TBG      0x1

drivers/cpufreq/armada-37xx-cpufreq.c line 38

Quoted 5 lines

drivers/cpufreq/armada-37xx-cpufreq.c new file +5 -0

+#define		ARMADA_37XX_NB_TBG_SEL_OFF	9
+#define		ARMADA_37XX_NB_TBG_SEL_MASK	0x3
+#define		ARMADA_37XX_NB_VDD_SEL_OFF	6
+#define		ARMADA_37XX_NB_VDD_SEL_MASK	0x3
+#define		ARMADA_37XX_NB_CONFIG_SHIFT	16

Looks like you have added tabs after #define as well? Perhaps a space is good enough there.

Quoted 175 lines

drivers/cpufreq/armada-37xx-cpufreq.c new file +174 -0

+#define ARMADA_37XX_NB_DYN_MOD	0x24
+#define		ARMADA_37XX_NB_CLK_SEL_EN	BIT(26)
+#define		ARMADA_37XX_NB_TBG_EN		BIT(28)
+#define		ARMADA_37XX_NB_DIV_EN		BIT(29)
+#define		ARMADA_37XX_NB_VDD_EN		BIT(30)
+#define		ARMADA_37XX_NB_DFS_EN		BIT(31)
+#define ARMADA_37XX_NB_CPU_LOAD	0x30
+#define		ARMADA_37XX_NB_CPU_LOAD_MASK	0x3
+#define		ARMADA_37XX_DVFS_LOAD_0		0
+#define		ARMADA_37XX_DVFS_LOAD_1		1
+#define		ARMADA_37XX_DVFS_LOAD_2		2
+#define		ARMADA_37XX_DVFS_LOAD_3		3
+
+/*
+ * On Armada 37xx the Power management manages 4 level of CPU load,
+ * each level can be associated with a CPU clock source, a CPU
+ * divider, a VDD level, etc...
+ */
+#define LOAD_LEVEL_NR	4
+
+struct armada_37xx_dvfs {
+	u32 cpu_freq_max;
+	u8 divider[LOAD_LEVEL_NR];
+};
+
+static struct armada_37xx_dvfs armada_37xx_dvfs[] = {
+	{.cpu_freq_max = 1200*1000*1000, .divider = {1, 2, 4, 6} },
+	{.cpu_freq_max = 1000*1000*1000, .divider = {1, 2, 4, 5} },
+	{.cpu_freq_max = 800*1000*1000,  .divider = {1, 2, 3, 4} },
+	{.cpu_freq_max = 600*1000*1000,  .divider = {2, 4, 5, 6} },
+};
+
+static struct armada_37xx_dvfs *armada_37xx_cpu_freq_info_get(u32 freq)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(armada_37xx_dvfs); i++) {
+		if (freq == armada_37xx_dvfs[i].cpu_freq_max)
+			return &armada_37xx_dvfs[i];
+	}
+
+	pr_err("Unsupported CPU frequency %d MHz\n", freq/1000000);
+	return NULL;
+}
+
+/*
+ * Setup the four level managed by the hardware. Once the four level
+ * will be configured then the DVFS will be enabled.
+ */
+static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
+						 struct clk *clk, u8 *divider)
+{
+	int load_level;
+	struct clk *parent;
+
+	for (load_level = 0; load_level < LOAD_LEVEL_NR; load_level++) {
+		unsigned int reg, mask,  val, offset = 0;
+
+		if (load_level <= ARMADA_37XX_DVFS_LOAD_1)
+			reg = ARMADA_37XX_NB_L0L1;
+		else
+			reg = ARMADA_37XX_NB_L2L3;
+
+		if (load_level ==  ARMADA_37XX_DVFS_LOAD_0 ||
+		    load_level ==  ARMADA_37XX_DVFS_LOAD_2)
+			offset += ARMADA_37XX_NB_CONFIG_SHIFT;
+
+		/* Set cpu clock source, for all the level we use TBG */
+		val = ARMADA_37XX_NB_CLK_SEL_TBG << ARMADA_37XX_NB_CLK_SEL_OFF;
+		mask = (ARMADA_37XX_NB_CLK_SEL_MASK
+			<< ARMADA_37XX_NB_CLK_SEL_OFF);
+
+		/*
+		 * Set cpu divider based on the pre-computed array in
+		 * order to have balanced step.
+		 */
+		val |= divider[load_level] << ARMADA_37XX_NB_TBG_DIV_OFF;
+		mask |= (ARMADA_37XX_NB_TBG_DIV_MASK
+			<< ARMADA_37XX_NB_TBG_DIV_OFF);
+
+		/* Set VDD divider which is actually the load level. */
+		val |= load_level << ARMADA_37XX_NB_VDD_SEL_OFF;
+		mask |= (ARMADA_37XX_NB_VDD_SEL_MASK
+			<< ARMADA_37XX_NB_VDD_SEL_OFF);
+
+		val <<= offset;
+		mask <<= offset;
+
+		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);
+}
+
+static void __init armada37xx_cpufreq_disable_dvfs(struct regmap *base)
+{
+	unsigned int reg = ARMADA_37XX_NB_DYN_MOD,
+		mask = ARMADA_37XX_NB_DFS_EN;
+
+	regmap_update_bits(base, reg, mask, 0);
+}
+
+static void __init armada37xx_cpufreq_enable_dvfs(struct regmap *base)
+{
+	unsigned int val, reg = ARMADA_37XX_NB_CPU_LOAD,
+		mask = ARMADA_37XX_NB_CPU_LOAD_MASK;
+
+	/* Start with the highest load (0) */
+	val = ARMADA_37XX_DVFS_LOAD_0;
+	regmap_update_bits(base, reg, mask, val);
+
+	/* Now enable DVFS for the CPUs */
+	reg = ARMADA_37XX_NB_DYN_MOD;
+	mask =	ARMADA_37XX_NB_CLK_SEL_EN | ARMADA_37XX_NB_TBG_EN |
+		ARMADA_37XX_NB_DIV_EN | ARMADA_37XX_NB_VDD_EN |
+		ARMADA_37XX_NB_DFS_EN;
+
+	regmap_update_bits(base, reg, mask, mask);
+}
+
+static int __init armada37xx_cpufreq_driver_init(void)
+{
+	struct armada_37xx_dvfs *dvfs;
+	struct platform_device *pdev;
+	unsigned int cur_frequency;
+	struct regmap *nb_pm_base;
+	struct device *cpu_dev;
+	int load_level, ret;
+	struct clk *clk;
+
+	nb_pm_base =
+		syscon_regmap_lookup_by_compatible("marvell,armada-3700-nb-pm");
+
+	if (IS_ERR(nb_pm_base))
+		return -ENODEV;
+
+	/* Before doing any configuration on the DVFS first, disable it */
+	armada37xx_cpufreq_disable_dvfs(nb_pm_base);
+
+	/*
+	 * On CPU 0 register the operating points supported (which are
+	 * the nominal CPU frequency and full integer divisions of
+	 * it).
+	 */
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev) {
+		dev_err(cpu_dev, "Cannot get CPU\n");
+		return -ENODEV;
+	}
+
+	clk = clk_get(cpu_dev, 0);
+	if (IS_ERR(clk)) {
+		dev_err(cpu_dev, "Cannot get clock for CPU0\n");
+		return PTR_ERR(clk);
+	}
+
+	/* Get nominal (current) CPU frequency */
+	cur_frequency = clk_get_rate(clk);
+	if (!cur_frequency) {
+		dev_err(cpu_dev, "Failed to get clock rate for CPU\n");
+		return -EINVAL;
+	}
+
+	dvfs = armada_37xx_cpu_freq_info_get(cur_frequency);
+	if (!dvfs)
+		return -EINVAL;
+
+	armada37xx_cpufreq_dvfs_setup(nb_pm_base, clk, dvfs->divider);

drivers/cpufreq/armada-37xx-cpufreq.c

Quoted 5 lines

drivers/cpufreq/armada-37xx-cpufreq.c new file +5 -0

+
+	/*
+	 * In case of a failure of dev_pm_opp_add(), we don't bother
+	 * with cleaning up the registered OPP (there's no function to
+	 * do so),

What do you mean by the comment within ()? We do have dev_pm_opp_remove() helper.

Quoted 28 lines

and simply cancel the registration of the cpufreq

drivers/cpufreq/armada-37xx-cpufreq.c new file +24 -0

+	 * device.
+	 */
+	for (load_level = ARMADA_37XX_DVFS_LOAD_0; load_level < LOAD_LEVEL_NR;
+	     load_level++) {
+		unsigned long freq = dvfs->divider[load_level];
+
+		ret = dev_pm_opp_add(cpu_dev, freq, 0);
+		if (ret)
+			return ret;
+	}
+
+	/* Now that everything is setup, enable the DVFS at hardware level */
+	armada37xx_cpufreq_enable_dvfs(nb_pm_base);
+
+	pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+
+	return PTR_ERR_OR_ZERO(pdev);
+}
+/* late_initcall, to guarantee the driver is loaded after A37xx clock driver */
+late_initcall(armada37xx_cpufreq_driver_init);
+
+MODULE_AUTHOR("Gregory CLEMENT <[email protected]>");
+MODULE_DESCRIPTION("Armada 37xx cpufreq driver");
+MODULE_LICENSE("GPL");

Signature · 1 line

2.15.0

Signature · 1 line

viresh

Hi,

This series adds the CPU Frequency support on Armada 37xx using DVFS. It is based on the initial work of Evan Wang and Victor Gu.

DVFS control is done by a set of registers from the North Bridge Power Management block. The binding for this block is documented in patch 1.

While adding a new cpufreq driver I found that the Kconfig and Makefile were no more in order, so it is fixed by patch 2 and 3.

The 4th patch is just about updating the MAINTAINERS file with the new driver.

The next patch is the real purpose of the series. The main goal of this driver is to setup the CPU load level in the hardware to associate them to CPU frequencies and register a standard cpufreq driver. Note that the hardware also capable of doing AVS (Adaptive Voltage Scaling), by associating a voltage on each level beside the CPU frequency. However, this support is not yet ready, so it is not part of this series.

Finally, the last patch is for arm-soc the arm-soc subsystem through mvebu and update the device tree to support the CPU frequency scaling.

An update on the CPU clock driver is needed in order to take into account the DVFS setting. It's the purpose of an other series already sent, but is no dependencies between the series (for building or at runtime).

Thanks,

Gregory

Gregory CLEMENT (6):
dt-bindings: marvell: Add documentation for the North Bridge PM on
Armada 37xx
cpufreq: ARM: sort the Kconfig menu
cpufreq: sort the drivers in ARM part
MAINTAINERS: add new entries for Armada 37xx cpufreq driver
cpufreq: Add DVFS support for Armada 37xx
arm64: dts: marvell: armada-37xx: add nodes allowing cpufreq support

  .../bindings/arm/marvell/armada-37xx.txt           |  19 ++
  MAINTAINERS                                        |   1 +
  arch/arm64/boot/dts/marvell/armada-372x.dtsi       |   1 +
  arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   7 +
  drivers/cpufreq/Kconfig.arm                        |  89 ++++----
  drivers/cpufreq/Makefile                           |   9 +-
  drivers/cpufreq/armada-37xx-cpufreq.c              | 241 +++++++++++++++++++++
  7 files changed, 322 insertions(+), 45 deletions(-)

create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

Hi Gregory,

I applied this and the dvfs patch set ([PATCH 0/3] Add DVFS support on CPU clock for Armada 37xx) on top of 4.14.3 and gave it a try on espressobin.

Upon modprobe I get:

[   66.231652] freq_table: Duplicate freq-table entries: 0
[   66.236967] cpu cpu0: cpufreq_init: invalid frequency table: -22
[   66.243534] freq_table: Duplicate freq-table entries: 0
[   66.248575] cpu cpu1: cpufreq_init: invalid frequency table: -22

Is this supposed to work already? Do I miss something?

Oh, and on that scenario it's not possible to rmmod the driver again, EBUSY.

Thanks,
Andre

Quoted 53 lines · Gregory CLEMENT

Hi,

This series adds the CPU Frequency support on Armada 37xx using DVFS. It is based on the initial work of Evan Wang and Victor Gu.

DVFS control is done by a set of registers from the North Bridge Power Management block. The binding for this block is documented in patch 1.

While adding a new cpufreq driver I found that the Kconfig and Makefile were no more in order, so it is fixed by patch 2 and 3.

The 4th patch is just about updating the MAINTAINERS file with the new driver.

The next patch is the real purpose of the series. The main goal of this driver is to setup the CPU load level in the hardware to associate them to CPU frequencies and register a standard cpufreq driver. Note that the hardware also capable of doing AVS (Adaptive Voltage Scaling), by associating a voltage on each level beside the CPU frequency. However, this support is not yet ready, so it is not part of this series.

Finally, the last patch is for arm-soc the arm-soc subsystem through mvebu and update the device tree to support the CPU frequency scaling.

An update on the CPU clock driver is needed in order to take into account the DVFS setting. It's the purpose of an other series already sent, but is no dependencies between the series (for building or at runtime).

Thanks,

Gregory

Gregory CLEMENT (6):
dt-bindings: marvell: Add documentation for the North Bridge PM on
Armada 37xx
cpufreq: ARM: sort the Kconfig menu
cpufreq: sort the drivers in ARM part
MAINTAINERS: add new entries for Armada 37xx cpufreq driver
cpufreq: Add DVFS support for Armada 37xx
arm64: dts: marvell: armada-37xx: add nodes allowing cpufreq support

  .../bindings/arm/marvell/armada-37xx.txt           |  19 ++
  MAINTAINERS                                        |   1 +
  arch/arm64/boot/dts/marvell/armada-372x.dtsi       |   1 +
  arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   7 +
  drivers/cpufreq/Kconfig.arm                        |  89 ++++----
  drivers/cpufreq/Makefile                           |   9 +-
  drivers/cpufreq/armada-37xx-cpufreq.c              | 241 +++++++++++++++++++++
  7 files changed, 322 insertions(+), 45 deletions(-)

create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

I did a last rebase before sending the series to remove the avs part not working yet. And during this rebase I introduced an bug.

I will send a v2 soon if you are intersected by testing it right now, here it is the fix:

iff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c index 40c9a744cc6e..96c2600009b5 100644

Quoted 11 lines · Gregory CLEMENT

I did a last rebase before sending the series to remove the avs part not working yet. And during this rebase I introduced an bug.

I will send a v2 soon if you are intersected by testing it right now, here it is the fix:

iff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c index 40c9a744cc6e..96c2600009b5 100644

drivers/cpufreq/armada-37xx-cpufreq.c static int __init armada37xx_cpufreq_driver_init(void) · line 225

Quoted 8 lines

drivers/cpufreq/armada-37xx-cpufreq.c +2 -1

@@ -219,7 +219,8 @@ static int __init armada37xx_cpufreq_driver_init(void)
          */
         for (load_level = ARMADA_37XX_DVFS_LOAD_0; load_level < LOAD_LEVEL_NR;
              load_level++) {
-               unsigned long freq = dvfs->divider[load_level];
+               unsigned long freq = cur_frequency /
+                       dvfs->divider[load_level];
                   ret = dev_pm_opp_add(cpu_dev, freq, 0);
                 if (ret)

Much better, that seems to work so far, thanks!

My espressobin now gets these frequencies: 200 MHz, 250 MHz, 500 MHz and 1000 MHz.

With the schedutil governor and watch -n 0.2 cpufreq-info -f -m it jumps over all of those depending on the load, nice:

Tested-by Andre Heider <[email protected]>

The measured power usage doesn't drop though, I guess that requires the AVS part you mentioned? Looking forward to it ;)

On a related note: Do you know if power usage can be lowered by disabling eth phys? Is that possible on mainline?

Regards,
Andre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Andre,

Quoted 26 lines · Andre Heider

Quoted 11 lines · Gregory CLEMENT

I did a last rebase before sending the series to remove the avs part not working yet. And during this rebase I introduced an bug.

I will send a v2 soon if you are intersected by testing it right now, here it is the fix:

iff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c index 40c9a744cc6e..96c2600009b5 100644

drivers/cpufreq/armada-37xx-cpufreq.c static int __init armada37xx_cpufreq_driver_init(void) · line 225

Quoted 8 lines

drivers/cpufreq/armada-37xx-cpufreq.c +2 -1

@@ -219,7 +219,8 @@ static int __init armada37xx_cpufreq_driver_init(void)
           */
          for (load_level = ARMADA_37XX_DVFS_LOAD_0; load_level < LOAD_LEVEL_NR;
               load_level++) {
-               unsigned long freq = dvfs->divider[load_level];
+               unsigned long freq = cur_frequency /
+                       dvfs->divider[load_level];
                    ret = dev_pm_opp_add(cpu_dev, freq, 0);
                  if (ret)

Much better, that seems to work so far, thanks!

My espressobin now gets these frequencies: 200 MHz, 250 MHz, 500 MHz and 1000 MHz.

Quoted 7 lines

With the schedutil governor and watch -n 0.2 cpufreq-info -f -m it jumps over all of those depending on the load, nice:

Tested-by Andre Heider <[email protected]>

The measured power usage doesn't drop though, I guess that requires the AVS part you mentioned? Looking forward to it ;)

Well I did some measurement and i saw some drop, according to my notes:

@ 250MHz: 222mA at 12V => 2.66W
@ 1000MHz: 238mA at 12CV => 2.87W

Not something huge, but only the CPUs which are concerned so it's not so bad.

Quoted 2 lines

On a related note: Do you know if power usage can be lowered by disabling eth phys? Is that possible on mainline?

For the test I have done just by removing an Ethernet cable we save a lot of power, so it helps. I'm sure it is doable on mainline, but maybe
currently, there is something still missing.