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(+)

Group all the related big LITTLE configuration together and sort the other entries in alphabetic order.

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

---
 drivers/cpufreq/Kconfig.arm | 82 ++++++++++++++++++++++-----------------------
 1 file changed, 41 insertions(+), 41 deletions(-)

Keep the driver files alphabetically sorted.

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

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

Since the introduction of this driver, the dev_pm_opp_remove() was added. So stop claiming we can't remove opp and use it in case of failure.

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

---
 drivers/cpufreq/mvebu-cpufreq.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

drivers/cpufreq/mvebu-cpufreq.c +0 -6

-		/*
-		 * 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), and simply cancel the
-		 * registration of the cpufreq device.
-		 */
 		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk), 0);
 		if (ret) {
 			clk_put(clk);

@@ -90,6 +84,11 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 

Hello,

Quoted 11 lines · Gregory CLEMENT

drivers/cpufreq/mvebu-cpufreq.c +0 -6

-		/*
-		 * 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), and simply cancel the
-		 * registration of the cpufreq device.
-		 */
 		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk), 0);
 		if (ret) {
 			clk_put(clk);

@@ -90,6 +84,11 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 

drivers/cpufreq/mvebu-cpufreq.c static int __init armada_xp_pmsu_cpufreq_init(void) · line 94

Quoted 10 lines

drivers/cpufreq/mvebu-cpufreq.c +5 -0

 		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0);
 		if (ret) {
+			/*
+			 * The second opp failed to be added, remove
+			 * the first one before exiting.
+			 */
+			dev_pm_opp_remove(cpu_dev, clk_get_rate(clk));
 			clk_put(clk);
 			return ret;
 		}

This still doesn't fix the failure situation. Indeed, you are only removing the OPP at full rate for the current CPU, but you are not removing the OPPs for the N-1 previous CPUs that have been handled in previous iterations of the loop.

This new driver belongs to the mvebu family, update the MAINTAINER file to document it.

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

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

MAINTAINERS +0 -0

index aa71ab52fd76..98dcee849481 100644

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

+/* 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
+#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
+#define ARMADA_37XX_NB_DYN_MOD	0x24
+#define	 ARMADA_37XX_NB_CLK_SEL_EN	BIT(26)

Quoted 19 lines · Gregory CLEMENT

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

+/* 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
+#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
+#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)

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

Quoted 6 lines

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

+#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

I thought you agreed to using space instead of tab after #define ?

Looks fine otherwise. You can add below after fixing above tab/space thing:

Acked-by Viresh Kumar <[email protected]>

Signature · 2 lines

-- 
viresh

Hi,

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

The main change since the first version was a bug fixed in the "cpufreq: Add DVFS support for Armada 37xx" patch which was preventing to register the opp. An other noticeable change is the 4th patch which is new adding the use of dev_pm_opp_remove and removing an comment became wrong. The other changes are described in the change log.

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 5th 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

Changelog:

v1 -> v2:

- using syscon instead of nb_pm for the binding of the North bridge power management unit: reported by Rob Herring

- fix sorting inside the big LITTLE section for the Kconfig: reported by Viresh Kumar

- fix the bogus freq calculation in armada37xx_cpufreq_driver_init, bug reported by Andre Heider

- use dev_pm_opp_remove() on the previous opp if dev_pm_opp_add() failed, reported by Viresh Kumar

- add the Tested-by flag from Andre Heider on "cpufreq: Add DVFS support for Armada 37xx" patch

Gregory CLEMENT (7):
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
cpufreq: mvebu: Use dev_pm_opp_remove()
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 +++++++++++++++++++++
 drivers/cpufreq/mvebu-cpufreq.c                    |  11 +-
 8 files changed, 327 insertions(+), 51 deletions(-)
 create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

Quoted 70 lines · CET Gregory CLEMENT

Hi,

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

The main change since the first version was a bug fixed in the "cpufreq: Add DVFS support for Armada 37xx" patch which was preventing to register the opp. An other noticeable change is the 4th patch which is new adding the use of dev_pm_opp_remove and removing an comment became wrong. The other changes are described in the change log.

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 5th 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

Changelog:

v1 -> v2:

- using syscon instead of nb_pm for the binding of the North bridge power management unit: reported by Rob Herring

- fix sorting inside the big LITTLE section for the Kconfig: reported by Viresh Kumar

- fix the bogus freq calculation in armada37xx_cpufreq_driver_init, bug reported by Andre Heider

- use dev_pm_opp_remove() on the previous opp if dev_pm_opp_add() failed, reported by Viresh Kumar

- add the Tested-by flag from Andre Heider on "cpufreq: Add DVFS support for Armada 37xx" patch

Gregory CLEMENT (7):
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
cpufreq: mvebu: Use dev_pm_opp_remove()
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

Quoted 10 lines

 .../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 +++++++++++++++++++++
 drivers/cpufreq/mvebu-cpufreq.c                    |  11 +-
 8 files changed, 327 insertions(+), 51 deletions(-)
 create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

I'd like you to split patches [2-3/7] off this series as they aren't exactly related to the other changes in it.

Thanks,
Rafael

Hi Rafael,

Quoted 82 lines · Rafael J. Wysocki

Quoted 70 lines · CET Gregory CLEMENT

Hi,

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

The main change since the first version was a bug fixed in the "cpufreq: Add DVFS support for Armada 37xx" patch which was preventing to register the opp. An other noticeable change is the 4th patch which is new adding the use of dev_pm_opp_remove and removing an comment became wrong. The other changes are described in the change log.

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 5th 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

Changelog:

v1 -> v2:

- using syscon instead of nb_pm for the binding of the North bridge power management unit: reported by Rob Herring

- fix sorting inside the big LITTLE section for the Kconfig: reported by Viresh Kumar

- fix the bogus freq calculation in armada37xx_cpufreq_driver_init, bug reported by Andre Heider

- use dev_pm_opp_remove() on the previous opp if dev_pm_opp_add() failed, reported by Viresh Kumar

- add the Tested-by flag from Andre Heider on "cpufreq: Add DVFS support for Armada 37xx" patch

Gregory CLEMENT (7):
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
cpufreq: mvebu: Use dev_pm_opp_remove()
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

Quoted 10 lines

 .../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 +++++++++++++++++++++
 drivers/cpufreq/mvebu-cpufreq.c                    |  11 +-
 8 files changed, 327 insertions(+), 51 deletions(-)
 create mode 100644 drivers/cpufreq/armada-37xx-cpufreq.c

Quoted 2 lines

I'd like you to split patches [2-3/7] off this series as they aren't exactly related to the other changes in it.

Actually the patch 6 depend on patch 2 and 3. To be more precise they modify the same area inside the files drivers/cpufreq/Kconfig.arm and drivers/cpufreq/Makefile.

So if I move them in an other series then there could be some merge conflicts.