[PATCHv3 0/7] cpufreq support for Marvell Armada XP

[PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Thomas Petazzoni]

Mike, Viresh, Rafael, Jason, Gregory, Andrew, Sebastian,

Here is the third version of the patches adding cpufreq support for
the Marvell Armada XP processor.

Changes since v2
================

 - As suggested by Stephen Boyd, instead of using a new clock notifier
   that somewhat "hides" the dependency of the clk-cpu clock driver on
   the PMSU, use a direct call from the clk-cpu driver to the PMSU
   driver.

 - Add a comment that explains why the OPP are not removed on failure
   in the PMSU code initializing the cpufreq logic, in answer to the
   review from Ezequiel Garcia.

Changes since v1
================

 - Rework the patch series to use the generalized cpufreq-cpu0
   (renamed cpufreq-generic) driver instead of having an Armada XP
   specific cpufreq driver. This was suggested by Viresh
   Kumar. Basically, it only involved adding a "clock-latency"
   property in the DT, and changing the PMSU code to register the two
   OPPs supported by each CPU, and registering the "cpufreq-generic"
   platform device instead of the "armadaxp-cpufreq" one.

Jason, this patch series is based on 3.16-rc3, but it applies fine
even with mvebu/fixes and mvebu/soc merged (which contain some PMSU
changes), so I haven't based the patch series on those branches. To
_work_, the code needs the new cpufreq-generic driver which is pending
in Viresh Kumar's tree for 3.17, but there is no build dependency.

Thanks,

Thomas

Thomas Petazzoni (7):
  ARM: mvebu: ensure CPU clocks are enabled
  ARM: mvebu: extend PMSU code to support dynamic frequency scaling
  clk: mvebu: extend clk-cpu for dynamic frequency scaling
  ARM: mvebu: update Armada XP DT for dynamic frequency scaling
  ARM: mvebu: allow enabling of cpufreq on Armada XP
  ARM: mvebu: update mvebu_v7_defconfig with cpufreq support
  ARM: configs: add cpufreq-generic in multi_v7_defconfig

 .../devicetree/bindings/clock/mvebu-cpu-clock.txt  |   5 +-
 arch/arm/boot/dts/armada-xp-mv78230.dtsi           |   2 +
 arch/arm/boot/dts/armada-xp-mv78260.dtsi           |   2 +
 arch/arm/boot/dts/armada-xp-mv78460.dtsi           |   4 +
 arch/arm/boot/dts/armada-xp.dtsi                   |   2 +-
 arch/arm/configs/multi_v7_defconfig                |   1 +
 arch/arm/configs/mvebu_v7_defconfig                |   2 +
 arch/arm/mach-mvebu/Kconfig                        |   1 +
 arch/arm/mach-mvebu/platsmp.c                      |   1 +
 arch/arm/mach-mvebu/pmsu.c                         | 162 +++++++++++++++++++++
 drivers/clk/mvebu/clk-cpu.c                        |  80 +++++++++-
 include/linux/mvebu-pmsu.h                         |  20 +++
 12 files changed, 274 insertions(+), 8 deletions(-)
 create mode 100644 include/linux/mvebu-pmsu.h

-- 
2.0.0

[PATCHv3 1/7] ARM: mvebu: ensure CPU clocks are enabled

[Thomas Petazzoni]

In the Armada XP SMP support code, we are reading the clock frequency
of the booting CPU, and use that to assign the same frequency to the
other CPUs, and we do this while the clocks are disabled.

However, the CPU clocks are in fact never prepared/enabled, and to
support cpufreq, we now have two code paths to change the frequency of
the CPU clocks in the CPU clock driver: one when the clock is enabled
(dynamic frequency scaling), one when the clock is disabled (adjusting
the CPU frequency before starting the CPU). In order for this to work,
the CPU clocks now have to be prepared and enabled after the initial
synchronization of the clock frequencies is done, so that all future
rate changes of the CPU clocks will trigger a dynamic frequency
scaling transition.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/platsmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/platsmp.c b/arch/arm/mach-mvebu/platsmp.c
index 88b976b3..4880b0f 100644
--- a/arch/arm/mach-mvebu/platsmp.c
+++ b/arch/arm/mach-mvebu/platsmp.c
@@ -67,6 +67,7 @@ static void __init set_secondary_cpus_clock(void)
 		if (!cpu_clk)
 			return;
 		clk_set_rate(cpu_clk, rate);
+		clk_prepare_enable(cpu_clk);
 	}
 }
 
-- 
2.0.0

Re: [PATCHv3 1/7] ARM: mvebu: ensure CPU clocks are enabled

[Jason Cooper]

On Wed, Jul 09, 2014 at 05:45:09PM +0200, Thomas Petazzoni wrote:
> In the Armada XP SMP support code, we are reading the clock frequency
> of the booting CPU, and use that to assign the same frequency to the
> other CPUs, and we do this while the clocks are disabled.
> 
> However, the CPU clocks are in fact never prepared/enabled, and to
> support cpufreq, we now have two code paths to change the frequency of
> the CPU clocks in the CPU clock driver: one when the clock is enabled
> (dynamic frequency scaling), one when the clock is disabled (adjusting
> the CPU frequency before starting the CPU). In order for this to work,
> the CPU clocks now have to be prepared and enabled after the initial
> synchronization of the clock frequencies is done, so that all future
> rate changes of the CPU clocks will trigger a dynamic frequency
> scaling transition.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/platsmp.c | 1 +
>  1 file changed, 1 insertion(+)

Tentatively applied patches 1-3, and 5 to mvebu/soc-cpufreq to get some
coverage in -next.

thx,

Jason.

[PATCHv3 2/7] ARM: mvebu: extend PMSU code to support dynamic frequency scaling

[Thomas Petazzoni]

This commit adds the necessary code in the Marvell EBU PMSU driver to
support dynamic frequency scaling. In essence, what this new code does
is that it:

 * registers the frequency operating points supported by the CPU;

 * registers a clock notifier of the CPU clocks. The notifier function
   listens to the newly introduced APPLY_RATE_CHANGE event, and uses
   that to finalize the frequency transition by doing the part of the
   procedure that involves the PMSU;

 * registers a platform device for the cpufreq-generic driver, which
   will take care of the CPU frequency transitions.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/pmsu.c | 162 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mvebu-pmsu.h |  20 ++++++
 2 files changed, 182 insertions(+)
 create mode 100644 include/linux/mvebu-pmsu.h

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index 53a55c8..db7d9ab 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -18,20 +18,26 @@
 
 #define pr_fmt(fmt) "mvebu-pmsu: " fmt
 
+#include <linux/clk.h>
 #include <linux/cpu_pm.h>
+#include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/smp.h>
 #include <linux/resource.h>
+#include <linux/slab.h>
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 #include <asm/tlbflush.h>
 #include "common.h"
+#include "armada-370-xp.h"
 
 static void __iomem *pmsu_mp_base;
 
@@ -57,6 +63,10 @@ static void __iomem *pmsu_mp_base;
 #define PMSU_STATUS_AND_MASK_IRQ_MASK		BIT(24)
 #define PMSU_STATUS_AND_MASK_FIQ_MASK		BIT(25)
 
+#define PMSU_EVENT_STATUS_AND_MASK(cpu)     ((cpu * 0x100) + 0x120)
+#define PMSU_EVENT_STATUS_AND_MASK_DFS_DONE        BIT(1)
+#define PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK   BIT(17)
+
 #define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu) ((cpu * 0x100) + 0x124)
 
 /* PMSU fabric registers */
@@ -296,3 +306,155 @@ int __init armada_370_xp_cpu_pm_init(void)
 
 arch_initcall(armada_370_xp_cpu_pm_init);
 early_initcall(armada_370_xp_pmsu_init);
+
+static void mvebu_pmsu_dfs_request_local(void *data)
+{
+	u32 reg;
+	u32 cpu = smp_processor_id();
+	unsigned long flags;
+
+	local_irq_save(flags);
+
+	/* Prepare to enter idle */
+	reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
+	reg |= PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT |
+	       PMSU_STATUS_AND_MASK_IRQ_MASK     |
+	       PMSU_STATUS_AND_MASK_FIQ_MASK;
+	writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
+
+	/* Request the DFS transition */
+	reg = readl(pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(cpu));
+	reg |= PMSU_CONTROL_AND_CONFIG_DFS_REQ;
+	writel(reg, pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(cpu));
+
+	/* The fact of entering idle will trigger the DFS transition */
+	wfi();
+
+	/*
+	 * We're back from idle, the DFS transition has completed,
+	 * clear the idle wait indication.
+	 */
+	reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
+	reg &= ~PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT;
+	writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
+
+	local_irq_restore(flags);
+}
+
+int mvebu_pmsu_dfs_request(int cpu)
+{
+	unsigned long timeout;
+	int hwcpu = cpu_logical_map(cpu);
+	u32 reg;
+
+	/* Clear any previous DFS DONE event */
+	reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+	reg &= ~PMSU_EVENT_STATUS_AND_MASK_DFS_DONE;
+	writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+
+	/* Mask the DFS done interrupt, since we are going to poll */
+	reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+	reg |= PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK;
+	writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+
+	/* Trigger the DFS on the appropriate CPU */
+	smp_call_function_single(cpu, mvebu_pmsu_dfs_request_local,
+				 NULL, false);
+
+	/* Poll until the DFS done event is generated */
+	timeout = jiffies + HZ;
+	while (time_before(jiffies, timeout)) {
+		reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+		if (reg & PMSU_EVENT_STATUS_AND_MASK_DFS_DONE)
+			break;
+		udelay(10);
+	}
+
+	if (time_after(jiffies, timeout))
+		return -ETIME;
+
+	/* Restore the DFS mask to its original state */
+	reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+	reg &= ~PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK;
+	writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
+
+	return 0;
+}
+
+static int __init armada_xp_pmsu_cpufreq_init(void)
+{
+	struct device_node *np;
+	struct resource res;
+	int ret, cpu;
+
+	if (!of_machine_is_compatible("marvell,armadaxp"))
+		return 0;
+
+	/*
+	 * In order to have proper cpufreq handling, we need to ensure
+	 * that the Device Tree description of the CPU clock includes
+	 * the definition of the PMU DFS registers. If not, we do not
+	 * register the clock notifier and the cpufreq driver. This
+	 * piece of code is only for compatibility with old Device
+	 * Trees.
+	 */
+	np = of_find_compatible_node(NULL, NULL, "marvell,armada-xp-cpu-clock");
+	if (!np)
+		return 0;
+
+	ret = of_address_to_resource(np, 1, &res);
+	if (ret) {
+		pr_warn(FW_WARN "not enabling cpufreq, deprecated armada-xp-cpu-clock binding\n");
+		of_node_put(np);
+		return 0;
+	}
+
+	of_node_put(np);
+
+	/*
+	 * For each CPU, this loop registers the operating points
+	 * supported (which are the nominal CPU frequency and half of
+	 * it), and registers the clock notifier that will take care
+	 * of doing the PMSU part of a frequency transition.
+	 */
+	for_each_possible_cpu(cpu) {
+		struct device *cpu_dev;
+		struct clk *clk;
+		int ret;
+
+		cpu_dev = get_cpu_device(cpu);
+		if (!cpu_dev) {
+			pr_err("Cannot get CPU %d\n", cpu);
+			continue;
+		}
+
+		clk = clk_get(cpu_dev, 0);
+		if (!clk) {
+			pr_err("Cannot get clock for CPU %d\n", cpu);
+			return -ENODEV;
+		}
+
+		/*
+		 * 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);
+			return ret;
+		}
+
+		ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0);
+		if (ret) {
+			clk_put(clk);
+			return ret;
+		}
+	}
+
+	platform_device_register_simple("cpufreq-generic", -1, NULL, 0);
+	return 0;
+}
+
+device_initcall(armada_xp_pmsu_cpufreq_init);
diff --git a/include/linux/mvebu-pmsu.h b/include/linux/mvebu-pmsu.h
new file mode 100644
index 0000000..b918d07
--- /dev/null
+++ b/include/linux/mvebu-pmsu.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __MVEBU_PMSU_H__
+#define __MVEBU_PMSU_H__
+
+#ifdef CONFIG_MACH_MVEBU_V7
+int mvebu_pmsu_dfs_request(int cpu);
+#else
+static inline int mvebu_pmsu_dfs_request(int cpu) { return -ENODEV; }
+#endif
+
+#endif /* __MVEBU_PMSU_H__ */
-- 
2.0.0

Re: [PATCHv3 2/7] ARM: mvebu: extend PMSU code to support dynamic frequency scaling

[Mike Turquette]

Quoting Thomas Petazzoni (2014-07-09 08:45:10)
> This commit adds the necessary code in the Marvell EBU PMSU driver to
> support dynamic frequency scaling. In essence, what this new code does
> is that it:
> 
>  * registers the frequency operating points supported by the CPU;
> 
>  * registers a clock notifier of the CPU clocks. The notifier function

Where does this code register a clock notifier callback?

>    listens to the newly introduced APPLY_RATE_CHANGE event, and uses

I don't see APPLY_RATE_CHANGE referenced.

>    that to finalize the frequency transition by doing the part of the
>    procedure that involves the PMSU;

Thanks,
Mike

> 
>  * registers a platform device for the cpufreq-generic driver, which
>    will take care of the CPU frequency transitions.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/pmsu.c | 162 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mvebu-pmsu.h |  20 ++++++
>  2 files changed, 182 insertions(+)
>  create mode 100644 include/linux/mvebu-pmsu.h
> 
> diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
> index 53a55c8..db7d9ab 100644
> --- a/arch/arm/mach-mvebu/pmsu.c
> +++ b/arch/arm/mach-mvebu/pmsu.c
> @@ -18,20 +18,26 @@
>  
>  #define pr_fmt(fmt) "mvebu-pmsu: " fmt
>  
> +#include <linux/clk.h>
>  #include <linux/cpu_pm.h>
> +#include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/of_address.h>
> +#include <linux/of_device.h>
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
>  #include <linux/smp.h>
>  #include <linux/resource.h>
> +#include <linux/slab.h>
>  #include <asm/cacheflush.h>
>  #include <asm/cp15.h>
>  #include <asm/smp_plat.h>
>  #include <asm/suspend.h>
>  #include <asm/tlbflush.h>
>  #include "common.h"
> +#include "armada-370-xp.h"
>  
>  static void __iomem *pmsu_mp_base;
>  
> @@ -57,6 +63,10 @@ static void __iomem *pmsu_mp_base;
>  #define PMSU_STATUS_AND_MASK_IRQ_MASK          BIT(24)
>  #define PMSU_STATUS_AND_MASK_FIQ_MASK          BIT(25)
>  
> +#define PMSU_EVENT_STATUS_AND_MASK(cpu)     ((cpu * 0x100) + 0x120)
> +#define PMSU_EVENT_STATUS_AND_MASK_DFS_DONE        BIT(1)
> +#define PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK   BIT(17)
> +
>  #define PMSU_BOOT_ADDR_REDIRECT_OFFSET(cpu) ((cpu * 0x100) + 0x124)
>  
>  /* PMSU fabric registers */
> @@ -296,3 +306,155 @@ int __init armada_370_xp_cpu_pm_init(void)
>  
>  arch_initcall(armada_370_xp_cpu_pm_init);
>  early_initcall(armada_370_xp_pmsu_init);
> +
> +static void mvebu_pmsu_dfs_request_local(void *data)
> +{
> +       u32 reg;
> +       u32 cpu = smp_processor_id();
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +
> +       /* Prepare to enter idle */
> +       reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
> +       reg |= PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT |
> +              PMSU_STATUS_AND_MASK_IRQ_MASK     |
> +              PMSU_STATUS_AND_MASK_FIQ_MASK;
> +       writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
> +
> +       /* Request the DFS transition */
> +       reg = readl(pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(cpu));
> +       reg |= PMSU_CONTROL_AND_CONFIG_DFS_REQ;
> +       writel(reg, pmsu_mp_base + PMSU_CONTROL_AND_CONFIG(cpu));
> +
> +       /* The fact of entering idle will trigger the DFS transition */
> +       wfi();
> +
> +       /*
> +        * We're back from idle, the DFS transition has completed,
> +        * clear the idle wait indication.
> +        */
> +       reg = readl(pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
> +       reg &= ~PMSU_STATUS_AND_MASK_CPU_IDLE_WAIT;
> +       writel(reg, pmsu_mp_base + PMSU_STATUS_AND_MASK(cpu));
> +
> +       local_irq_restore(flags);
> +}
> +
> +int mvebu_pmsu_dfs_request(int cpu)
> +{
> +       unsigned long timeout;
> +       int hwcpu = cpu_logical_map(cpu);
> +       u32 reg;
> +
> +       /* Clear any previous DFS DONE event */
> +       reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +       reg &= ~PMSU_EVENT_STATUS_AND_MASK_DFS_DONE;
> +       writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +
> +       /* Mask the DFS done interrupt, since we are going to poll */
> +       reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +       reg |= PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK;
> +       writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +
> +       /* Trigger the DFS on the appropriate CPU */
> +       smp_call_function_single(cpu, mvebu_pmsu_dfs_request_local,
> +                                NULL, false);
> +
> +       /* Poll until the DFS done event is generated */
> +       timeout = jiffies + HZ;
> +       while (time_before(jiffies, timeout)) {
> +               reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +               if (reg & PMSU_EVENT_STATUS_AND_MASK_DFS_DONE)
> +                       break;
> +               udelay(10);
> +       }
> +
> +       if (time_after(jiffies, timeout))
> +               return -ETIME;
> +
> +       /* Restore the DFS mask to its original state */
> +       reg = readl(pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +       reg &= ~PMSU_EVENT_STATUS_AND_MASK_DFS_DONE_MASK;
> +       writel(reg, pmsu_mp_base + PMSU_EVENT_STATUS_AND_MASK(hwcpu));
> +
> +       return 0;
> +}
> +
> +static int __init armada_xp_pmsu_cpufreq_init(void)
> +{
> +       struct device_node *np;
> +       struct resource res;
> +       int ret, cpu;
> +
> +       if (!of_machine_is_compatible("marvell,armadaxp"))
> +               return 0;
> +
> +       /*
> +        * In order to have proper cpufreq handling, we need to ensure
> +        * that the Device Tree description of the CPU clock includes
> +        * the definition of the PMU DFS registers. If not, we do not
> +        * register the clock notifier and the cpufreq driver. This
> +        * piece of code is only for compatibility with old Device
> +        * Trees.
> +        */
> +       np = of_find_compatible_node(NULL, NULL, "marvell,armada-xp-cpu-clock");
> +       if (!np)
> +               return 0;
> +
> +       ret = of_address_to_resource(np, 1, &res);
> +       if (ret) {
> +               pr_warn(FW_WARN "not enabling cpufreq, deprecated armada-xp-cpu-clock binding\n");
> +               of_node_put(np);
> +               return 0;
> +       }
> +
> +       of_node_put(np);
> +
> +       /*
> +        * For each CPU, this loop registers the operating points
> +        * supported (which are the nominal CPU frequency and half of
> +        * it), and registers the clock notifier that will take care
> +        * of doing the PMSU part of a frequency transition.
> +        */
> +       for_each_possible_cpu(cpu) {
> +               struct device *cpu_dev;
> +               struct clk *clk;
> +               int ret;
> +
> +               cpu_dev = get_cpu_device(cpu);
> +               if (!cpu_dev) {
> +                       pr_err("Cannot get CPU %d\n", cpu);
> +                       continue;
> +               }
> +
> +               clk = clk_get(cpu_dev, 0);
> +               if (!clk) {
> +                       pr_err("Cannot get clock for CPU %d\n", cpu);
> +                       return -ENODEV;
> +               }
> +
> +               /*
> +                * 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);
> +                       return ret;
> +               }
> +
> +               ret = dev_pm_opp_add(cpu_dev, clk_get_rate(clk) / 2, 0);
> +               if (ret) {
> +                       clk_put(clk);
> +                       return ret;
> +               }
> +       }
> +
> +       platform_device_register_simple("cpufreq-generic", -1, NULL, 0);
> +       return 0;
> +}
> +
> +device_initcall(armada_xp_pmsu_cpufreq_init);
> diff --git a/include/linux/mvebu-pmsu.h b/include/linux/mvebu-pmsu.h
> new file mode 100644
> index 0000000..b918d07
> --- /dev/null
> +++ b/include/linux/mvebu-pmsu.h
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (C) 2012 Marvell
> + *
> + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef __MVEBU_PMSU_H__
> +#define __MVEBU_PMSU_H__
> +
> +#ifdef CONFIG_MACH_MVEBU_V7
> +int mvebu_pmsu_dfs_request(int cpu);
> +#else
> +static inline int mvebu_pmsu_dfs_request(int cpu) { return -ENODEV; }
> +#endif
> +
> +#endif /* __MVEBU_PMSU_H__ */
> -- 
> 2.0.0
> 

Re: [PATCHv3 2/7] ARM: mvebu: extend PMSU code to support dynamic frequency scaling

[Thomas Petazzoni]

Dear Mike Turquette,

On Wed, 23 Jul 2014 16:50:26 -0700, Mike Turquette wrote:

> >  * registers the frequency operating points supported by the CPU;
> > 
> >  * registers a clock notifier of the CPU clocks. The notifier function
> 
> Where does this code register a clock notifier callback?
> 
> >    listens to the newly introduced APPLY_RATE_CHANGE event, and uses
> 
> I don't see APPLY_RATE_CHANGE referenced.

Yes, this is a mistake of the commit log, due to remains of the v2 of
the patch series. Back in the v2, there was indeed a new clock notifier
being used. But Stephen Boyd argued against that, and instead suggested
to use a direct function call, which this v3 implements, as stated in
the cover letter:

 - As suggested by Stephen Boyd, instead of using a new clock notifier
   that somewhat "hides" the dependency of the clk-cpu clock driver on
   the PMSU, use a direct call from the clk-cpu driver to the PMSU
   driver.

The commit log of this commit was not adjusted consequently, and this
is my fault. Jason, is it still time to change this commit log?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv3 2/7] ARM: mvebu: extend PMSU code to support dynamic frequency scaling

[Jason Cooper]

On Thu, Jul 24, 2014 at 08:29:02AM +0200, Thomas Petazzoni wrote:
> Dear Mike Turquette,
> 
> On Wed, 23 Jul 2014 16:50:26 -0700, Mike Turquette wrote:
> 
> > >  * registers the frequency operating points supported by the CPU;
> > > 
> > >  * registers a clock notifier of the CPU clocks. The notifier function
> > 
> > Where does this code register a clock notifier callback?
> > 
> > >    listens to the newly introduced APPLY_RATE_CHANGE event, and uses
> > 
> > I don't see APPLY_RATE_CHANGE referenced.
> 
> Yes, this is a mistake of the commit log, due to remains of the v2 of
> the patch series. Back in the v2, there was indeed a new clock notifier
> being used. But Stephen Boyd argued against that, and instead suggested
> to use a direct function call, which this v3 implements, as stated in
> the cover letter:
> 
>  - As suggested by Stephen Boyd, instead of using a new clock notifier
>    that somewhat "hides" the dependency of the clk-cpu clock driver on
>    the PMSU, use a direct call from the clk-cpu driver to the PMSU
>    driver.
> 
> The commit log of this commit was not adjusted consequently, and this
> is my fault. Jason, is it still time to change this commit log?

If there are no code changes, I'd prefer not to.  We're rather late in
the game.

Even though it's not ideal, the commit in question does have a Link: tag
pointing at the patch email on which this conversation is based.  So a
frustrated future developer won't be frustrated long. :)

thx,

Jason.

[PATCHv3 3/7] clk: mvebu: extend clk-cpu for dynamic frequency scaling

[Thomas Petazzoni]

This commit extends the existing clk-cpu driver used on Marvell Armada
XP platforms to support the dynamic frequency scaling of the CPU
clock. Non-dynamic frequency change was already supported (and used
before secondary CPUs are started), but the dynamic frequency change
requires a completely different procedure.

In order to achieve this, the clk_cpu_set_rate() function is reworked
to handle two separate cases:

 - The case where the clock is enabled, which is the new dynamic
   frequency change code, implemented in clk_cpu_on_set_rate(). This
   part will be used for cpufreq activities.

 - The case where the clock is disabled, which is the existing
   frequency change code, moved in clk_cpu_off_set_rate(). This part
   is already used to set the clock frequency of the secondary CPUs
   before starting them.

In order to implement the dynamic frequency change function, we need
to access the PMU DFS registers, which are outside the currently
mapped "Clock Complex" registers, so a new area of registers is now
mapped. This affects the Device Tree binding, but we are careful to do
it in a backward-compatible way (by allowing the second pair of
registers to be non-existent, and in this case, ensuring
clk_cpu_on_set_rate() returns an error).

Note that technically speaking, the clk_cpu_on_set_rate() does not do
the entire procedure needed to change the frequency dynamically, as it
involves touching a number of PMSU registers. This is done through a
clock notifier registered by the PMSU driver in followup commits.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/clock/mvebu-cpu-clock.txt  |  5 +-
 drivers/clk/mvebu/clk-cpu.c                        | 80 ++++++++++++++++++++--
 2 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt
index feb8301..99c2146 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-cpu-clock.txt
@@ -3,14 +3,15 @@ Device Tree Clock bindings for cpu clock of Marvell EBU platforms
 Required properties:
 - compatible : shall be one of the following:
 	"marvell,armada-xp-cpu-clock" - cpu clocks for Armada XP
-- reg : Address and length of the clock complex register set
+- reg : Address and length of the clock complex register set, followed
+        by address and length of the PMU DFS registers
 - #clock-cells : should be set to 1.
 - clocks : shall be the input parent clock phandle for the clock.
 
 cpuclk: clock-complex@d0018700 {
 	#clock-cells = <1>;
 	compatible = "marvell,armada-xp-cpu-clock";
-	reg = <0xd0018700 0xA0>;
+	reg = <0xd0018700 0xA0>, <0x1c054 0x10>;
 	clocks = <&coreclk 1>;
 }
 
diff --git a/drivers/clk/mvebu/clk-cpu.c b/drivers/clk/mvebu/clk-cpu.c
index 8ebf757..3821a88 100644
--- a/drivers/clk/mvebu/clk-cpu.c
+++ b/drivers/clk/mvebu/clk-cpu.c
@@ -16,10 +16,19 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/delay.h>
+#include <linux/mvebu-pmsu.h>
+#include <asm/smp_plat.h>
 
-#define SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET    0x0
-#define SYS_CTRL_CLK_DIVIDER_VALUE_OFFSET   0xC
-#define SYS_CTRL_CLK_DIVIDER_MASK	    0x3F
+#define SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET               0x0
+#define   SYS_CTRL_CLK_DIVIDER_CTRL_RESET_ALL          0xff
+#define   SYS_CTRL_CLK_DIVIDER_CTRL_RESET_SHIFT        8
+#define SYS_CTRL_CLK_DIVIDER_CTRL2_OFFSET              0x8
+#define   SYS_CTRL_CLK_DIVIDER_CTRL2_NBCLK_RATIO_SHIFT 16
+#define SYS_CTRL_CLK_DIVIDER_VALUE_OFFSET              0xC
+#define SYS_CTRL_CLK_DIVIDER_MASK                      0x3F
+
+#define PMU_DFS_RATIO_SHIFT 16
+#define PMU_DFS_RATIO_MASK  0x3F
 
 #define MAX_CPU	    4
 struct cpu_clk {
@@ -28,6 +37,7 @@ struct cpu_clk {
 	const char *clk_name;
 	const char *parent_name;
 	void __iomem *reg_base;
+	void __iomem *pmu_dfs;
 };
 
 static struct clk **clks;
@@ -62,8 +72,9 @@ static long clk_cpu_round_rate(struct clk_hw *hwclk, unsigned long rate,
 	return *parent_rate / div;
 }
 
-static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
-			    unsigned long parent_rate)
+static int clk_cpu_off_set_rate(struct clk_hw *hwclk, unsigned long rate,
+				unsigned long parent_rate)
+
 {
 	struct cpu_clk *cpuclk = to_cpu_clk(hwclk);
 	u32 reg, div;
@@ -95,6 +106,58 @@ static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
 	return 0;
 }
 
+static int clk_cpu_on_set_rate(struct clk_hw *hwclk, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	u32 reg;
+	unsigned long fabric_div, target_div, cur_rate;
+	struct cpu_clk *cpuclk = to_cpu_clk(hwclk);
+
+	/*
+	 * PMU DFS registers are not mapped, Device Tree does not
+	 * describes them. We cannot change the frequency dynamically.
+	 */
+	if (!cpuclk->pmu_dfs)
+		return -ENODEV;
+
+	cur_rate = __clk_get_rate(hwclk->clk);
+
+	reg = readl(cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL2_OFFSET);
+	fabric_div = (reg >> SYS_CTRL_CLK_DIVIDER_CTRL2_NBCLK_RATIO_SHIFT) &
+		SYS_CTRL_CLK_DIVIDER_MASK;
+
+	/* Frequency is going up */
+	if (rate == 2 * cur_rate)
+		target_div = fabric_div / 2;
+	/* Frequency is going down */
+	else
+		target_div = fabric_div;
+
+	if (target_div == 0)
+		target_div = 1;
+
+	reg = readl(cpuclk->pmu_dfs);
+	reg &= ~(PMU_DFS_RATIO_MASK << PMU_DFS_RATIO_SHIFT);
+	reg |= (target_div << PMU_DFS_RATIO_SHIFT);
+	writel(reg, cpuclk->pmu_dfs);
+
+	reg = readl(cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET);
+	reg |= (SYS_CTRL_CLK_DIVIDER_CTRL_RESET_ALL <<
+		SYS_CTRL_CLK_DIVIDER_CTRL_RESET_SHIFT);
+	writel(reg, cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET);
+
+	return mvebu_pmsu_dfs_request(cpuclk->cpu);
+}
+
+static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
+			    unsigned long parent_rate)
+{
+	if (__clk_is_enabled(hwclk->clk))
+		return clk_cpu_on_set_rate(hwclk, rate, parent_rate);
+	else
+		return clk_cpu_off_set_rate(hwclk, rate, parent_rate);
+}
+
 static const struct clk_ops cpu_ops = {
 	.recalc_rate = clk_cpu_recalc_rate,
 	.round_rate = clk_cpu_round_rate,
@@ -105,6 +168,7 @@ static void __init of_cpu_clk_setup(struct device_node *node)
 {
 	struct cpu_clk *cpuclk;
 	void __iomem *clock_complex_base = of_iomap(node, 0);
+	void __iomem *pmu_dfs_base = of_iomap(node, 1);
 	int ncpus = 0;
 	struct device_node *dn;
 
@@ -114,6 +178,10 @@ static void __init of_cpu_clk_setup(struct device_node *node)
 		return;
 	}
 
+	if (pmu_dfs_base == NULL)
+		pr_warn("%s: pmu-dfs base register not set, dynamic frequency scaling not available\n",
+			__func__);
+
 	for_each_node_by_type(dn, "cpu")
 		ncpus++;
 
@@ -146,6 +214,8 @@ static void __init of_cpu_clk_setup(struct device_node *node)
 		cpuclk[cpu].clk_name = clk_name;
 		cpuclk[cpu].cpu = cpu;
 		cpuclk[cpu].reg_base = clock_complex_base;
+		if (pmu_dfs_base)
+			cpuclk[cpu].pmu_dfs = pmu_dfs_base + 4 * cpu;
 		cpuclk[cpu].hw.init = &init;
 
 		init.name = cpuclk[cpu].clk_name;
-- 
2.0.0

[PATCHv3 4/7] ARM: mvebu: update Armada XP DT for dynamic frequency scaling

[Thomas Petazzoni]

In order to support dynamic frequency scaling:

 * the cpuclk Device Tree node needs to be updated to describe a
   second set of registers describing the PMU DFS registers.

 * the clock-latency property of the CPUs must be filled, otherwise
   the ondemand and conservative cpufreq governors refuse to work. The
   latency is high because the cost of a frequency transition is quite
   high on those CPUs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-mv78230.dtsi | 2 ++
 arch/arm/boot/dts/armada-xp-mv78260.dtsi | 2 ++
 arch/arm/boot/dts/armada-xp-mv78460.dtsi | 4 ++++
 arch/arm/boot/dts/armada-xp.dtsi         | 2 +-
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-xp-mv78230.dtsi b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
index 1257ff1..2592e1c 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -34,6 +34,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <0>;
 			clocks = <&cpuclk 0>;
+			clock-latency = <1000000>;
 		};
 
 		cpu@1 {
@@ -41,6 +42,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <1>;
 			clocks = <&cpuclk 1>;
+			clock-latency = <1000000>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/armada-xp-mv78260.dtsi b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
index 3396b25..480e237 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -36,6 +36,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <0>;
 			clocks = <&cpuclk 0>;
+			clock-latency = <1000000>;
 		};
 
 		cpu@1 {
@@ -43,6 +44,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <1>;
 			clocks = <&cpuclk 1>;
+			clock-latency = <1000000>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/armada-xp-mv78460.dtsi b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
index 6da84bf..2c7b1fe 100644
--- a/arch/arm/boot/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
@@ -37,6 +37,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <0>;
 			clocks = <&cpuclk 0>;
+			clock-latency = <1000000>;
 		};
 
 		cpu@1 {
@@ -44,6 +45,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <1>;
 			clocks = <&cpuclk 1>;
+			clock-latency = <1000000>;
 		};
 
 		cpu@2 {
@@ -51,6 +53,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <2>;
 			clocks = <&cpuclk 2>;
+			clock-latency = <1000000>;
 		};
 
 		cpu@3 {
@@ -58,6 +61,7 @@
 			compatible = "marvell,sheeva-v7";
 			reg = <3>;
 			clocks = <&cpuclk 3>;
+			clock-latency = <1000000>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 5902e83..bff9f6c 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -99,7 +99,7 @@
 			cpuclk: clock-complex@18700 {
 				#clock-cells = <1>;
 				compatible = "marvell,armada-xp-cpu-clock";
-				reg = <0x18700 0xA0>;
+				reg = <0x18700 0xA0>, <0x1c054 0x10>;
 				clocks = <&coreclk 1>;
 			};
 
-- 
2.0.0

Re: [PATCHv3 4/7] ARM: mvebu: update Armada XP DT for dynamic frequency scaling

[Jason Cooper]

On Wed, Jul 09, 2014 at 05:45:12PM +0200, Thomas Petazzoni wrote:
> In order to support dynamic frequency scaling:
> 
>  * the cpuclk Device Tree node needs to be updated to describe a
>    second set of registers describing the PMU DFS registers.
> 
>  * the clock-latency property of the CPUs must be filled, otherwise
>    the ondemand and conservative cpufreq governors refuse to work. The
>    latency is high because the cost of a frequency transition is quite
>    high on those CPUs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-xp-mv78230.dtsi | 2 ++
>  arch/arm/boot/dts/armada-xp-mv78260.dtsi | 2 ++
>  arch/arm/boot/dts/armada-xp-mv78460.dtsi | 4 ++++
>  arch/arm/boot/dts/armada-xp.dtsi         | 2 +-
>  4 files changed, 9 insertions(+), 1 deletion(-)

Applied to mvebu/dt

thx,

Jason.

[PATCHv3 5/7] ARM: mvebu: allow enabling of cpufreq on Armada XP

[Thomas Petazzoni]

Now that all the pieces to enable cpufreq on Armada XP are in place,
we can make MACH_ARMADA_XP select ARCH_HAS_CPUFREQ to allow the usage
of dynamic frequency scaling on this platform.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index b9bc599..1ff60ef 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -66,6 +66,7 @@ config MACH_ARMADA_XP
 	select CPU_PJ4B
 	select MACH_MVEBU_V7
 	select PINCTRL_ARMADA_XP
+	select ARCH_HAS_CPUFREQ
 	help
 	  Say 'Y' here if you want your kernel to support boards based
 	  on the Marvell Armada XP SoC with device tree.
-- 
2.0.0

[PATCHv3 6/7] ARM: mvebu: update mvebu_v7_defconfig with cpufreq support

[Thomas Petazzoni]

Now that the Armada XP supports dynamic CPU frequency scaling, it
makes sense to enable the cpufreq subsystem in mvebu_v7_defconfig, as
well as the cpufreq-generic driver used on Armada XP.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/configs/mvebu_v7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index b0bfefa..be2b5bc 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -29,6 +29,8 @@ CONFIG_ZBOOT_ROM_TEXT=0x0
 CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_ARM_APPENDED_DTB=y
 CONFIG_ARM_ATAG_DTB_COMPAT=y
+CONFIG_CPU_FREQ=y
+CONFIG_CPUFREQ_GENERIC=y
 CONFIG_VFP=y
 CONFIG_NET=y
 CONFIG_INET=y
-- 
2.0.0

Re: [PATCHv3 6/7] ARM: mvebu: update mvebu_v7_defconfig with cpufreq support

[Jason Cooper]

On Wed, Jul 09, 2014 at 05:45:14PM +0200, Thomas Petazzoni wrote:
> Now that the Armada XP supports dynamic CPU frequency scaling, it
> makes sense to enable the cpufreq subsystem in mvebu_v7_defconfig, as
> well as the cpufreq-generic driver used on Armada XP.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/configs/mvebu_v7_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Applied to mvebu/defconfig

thx,

Jason.

[PATCHv3 7/7] ARM: configs: add cpufreq-generic in multi_v7_defconfig

[Thomas Petazzoni]

The cpufreq-generic driver is going to be used by more and more
platforms to support dynamic frequency scaling of the CPU. This commit
enables this driver by default in multi_v7_defconfig.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index be1a345..80c1f60 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -82,6 +82,7 @@ CONFIG_KEXEC=y
 CONFIG_CPU_FREQ=y
 CONFIG_CPU_FREQ_STAT_DETAILS=y
 CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
+CONFIG_CPUFREQ_GENERIC=y
 CONFIG_CPU_IDLE=y
 CONFIG_NET=y
 CONFIG_PACKET=y
-- 
2.0.0

Re: [PATCHv3 7/7] ARM: configs: add cpufreq-generic in multi_v7_defconfig

[Jason Cooper]

On Wed, Jul 09, 2014 at 05:45:15PM +0200, Thomas Petazzoni wrote:
> The cpufreq-generic driver is going to be used by more and more
> platforms to support dynamic frequency scaling of the CPU. This commit
> enables this driver by default in multi_v7_defconfig.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/configs/multi_v7_defconfig | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Jason Cooper <jason@lakedaemon.net>

Please re-send to arm-soc maintainers for them to apply directly.

thx,

Jason.

Re: [PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Jason Cooper]

Mike,

On Wed, Jul 09, 2014 at 05:45:08PM +0200, Thomas Petazzoni wrote:
> Mike, Viresh, Rafael, Jason, Gregory, Andrew, Sebastian,
> 
> Here is the third version of the patches adding cpufreq support for
> the Marvell Armada XP processor.
> 
> Changes since v2
> ================
> 
>  - As suggested by Stephen Boyd, instead of using a new clock notifier
>    that somewhat "hides" the dependency of the clk-cpu clock driver on
>    the PMSU, use a direct call from the clk-cpu driver to the PMSU
>    driver.
> 
>  - Add a comment that explains why the OPP are not removed on failure
>    in the PMSU code initializing the cpufreq logic, in answer to the
>    review from Ezequiel Garcia.
> 
> Changes since v1
> ================
> 
>  - Rework the patch series to use the generalized cpufreq-cpu0
>    (renamed cpufreq-generic) driver instead of having an Armada XP
>    specific cpufreq driver. This was suggested by Viresh
>    Kumar. Basically, it only involved adding a "clock-latency"
>    property in the DT, and changing the PMSU code to register the two
>    OPPs supported by each CPU, and registering the "cpufreq-generic"
>    platform device instead of the "armadaxp-cpufreq" one.
> 
> Jason, this patch series is based on 3.16-rc3, but it applies fine
> even with mvebu/fixes and mvebu/soc merged (which contain some PMSU
> changes), so I haven't based the patch series on those branches. To
> _work_, the code needs the new cpufreq-generic driver which is pending
> in Viresh Kumar's tree for 3.17, but there is no build dependency.
> 
> Thanks,
> 
> Thomas
> 
> Thomas Petazzoni (7):
>   ARM: mvebu: ensure CPU clocks are enabled
>   ARM: mvebu: extend PMSU code to support dynamic frequency scaling
>   clk: mvebu: extend clk-cpu for dynamic frequency scaling
>   ARM: mvebu: update Armada XP DT for dynamic frequency scaling
>   ARM: mvebu: allow enabling of cpufreq on Armada XP
>   ARM: mvebu: update mvebu_v7_defconfig with cpufreq support
>   ARM: configs: add cpufreq-generic in multi_v7_defconfig
> 
>  .../devicetree/bindings/clock/mvebu-cpu-clock.txt  |   5 +-
>  arch/arm/boot/dts/armada-xp-mv78230.dtsi           |   2 +
>  arch/arm/boot/dts/armada-xp-mv78260.dtsi           |   2 +
>  arch/arm/boot/dts/armada-xp-mv78460.dtsi           |   4 +
>  arch/arm/boot/dts/armada-xp.dtsi                   |   2 +-
>  arch/arm/configs/multi_v7_defconfig                |   1 +
>  arch/arm/configs/mvebu_v7_defconfig                |   2 +
>  arch/arm/mach-mvebu/Kconfig                        |   1 +
>  arch/arm/mach-mvebu/platsmp.c                      |   1 +
>  arch/arm/mach-mvebu/pmsu.c                         | 162 +++++++++++++++++++++
>  drivers/clk/mvebu/clk-cpu.c                        |  80 +++++++++-
>  include/linux/mvebu-pmsu.h                         |  20 +++
>  12 files changed, 274 insertions(+), 8 deletions(-)
>  create mode 100644 include/linux/mvebu-pmsu.h

Creating a clock branch for you to pull makes this series an unwieldy
knot of dependencies. :(  Mind giving me an Ack to take it through the
mvebu tree?

thx,

Jason.

Re: [PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Thomas Petazzoni]

Viresh, Jason,

So, what do we do with this patch series, which depends on the
cpufreq-generic driver? Has there been any solution found for 3.17 ?

Jason, in any case, I'd like the following patches to be merged for
3.17, regardless of what happens with the cpufreq driver:

 ARM: mvebu: ensure CPU clocks are enabled
 ARM: mvebu: extend PMSU code to support dynamic frequency scaling
 clk: mvebu: extend clk-cpu for dynamic frequency scaling

One patch should be split:

 ARM: mvebu: update Armada XP DT for dynamic frequency scaling

 -> In this patch, the addition of clock-latency is related to the
    cpufreq generic DT binding, so I think we shouldn't merge that. But
    on the other hand, this patch also adds the new registers for the
    Armada XP CPU clock, which is used by "clk: mvebu: extend clk-cpu
    for dynamic frequency scaling".

The patch:

 ARM: mvebu: allow enabling of cpufreq on Armada XP

can be dropped, since ARCH_HAS_CPUFREQ has been removed.

The other patches are defconfig changes, which are meaningless without
the cpufreq-generic driver.

Jason, what do you think about me sending a new version of the patch
series, which will have two clearly separated set of patches:

 1/ A first set of patches that can be applied regardless of what
    happens on the cpufreq driver side. Getting it merged will not
    bring cpufreq support, but it will add the foundations needed to
    support it.

 2/ A second set of patches that use the cpufreq-generic driver, which
    might get applied of the cpufreq maintainers find a solution in
    time for 3.17. If not, then I'll re-adapt them for 3.18.

What do you think?

Thomas


On Wed,  9 Jul 2014 17:45:08 +0200, Thomas Petazzoni wrote:
> Mike, Viresh, Rafael, Jason, Gregory, Andrew, Sebastian,
> 
> Here is the third version of the patches adding cpufreq support for
> the Marvell Armada XP processor.
> 
> Changes since v2
> ================
> 
>  - As suggested by Stephen Boyd, instead of using a new clock notifier
>    that somewhat "hides" the dependency of the clk-cpu clock driver on
>    the PMSU, use a direct call from the clk-cpu driver to the PMSU
>    driver.
> 
>  - Add a comment that explains why the OPP are not removed on failure
>    in the PMSU code initializing the cpufreq logic, in answer to the
>    review from Ezequiel Garcia.
> 
> Changes since v1
> ================
> 
>  - Rework the patch series to use the generalized cpufreq-cpu0
>    (renamed cpufreq-generic) driver instead of having an Armada XP
>    specific cpufreq driver. This was suggested by Viresh
>    Kumar. Basically, it only involved adding a "clock-latency"
>    property in the DT, and changing the PMSU code to register the two
>    OPPs supported by each CPU, and registering the "cpufreq-generic"
>    platform device instead of the "armadaxp-cpufreq" one.
> 
> Jason, this patch series is based on 3.16-rc3, but it applies fine
> even with mvebu/fixes and mvebu/soc merged (which contain some PMSU
> changes), so I haven't based the patch series on those branches. To
> _work_, the code needs the new cpufreq-generic driver which is pending
> in Viresh Kumar's tree for 3.17, but there is no build dependency.
> 
> Thanks,
> 
> Thomas
> 
> Thomas Petazzoni (7):
>   ARM: mvebu: ensure CPU clocks are enabled
>   ARM: mvebu: extend PMSU code to support dynamic frequency scaling
>   clk: mvebu: extend clk-cpu for dynamic frequency scaling
>   ARM: mvebu: update Armada XP DT for dynamic frequency scaling
>   ARM: mvebu: allow enabling of cpufreq on Armada XP
>   ARM: mvebu: update mvebu_v7_defconfig with cpufreq support
>   ARM: configs: add cpufreq-generic in multi_v7_defconfig
> 
>  .../devicetree/bindings/clock/mvebu-cpu-clock.txt  |   5 +-
>  arch/arm/boot/dts/armada-xp-mv78230.dtsi           |   2 +
>  arch/arm/boot/dts/armada-xp-mv78260.dtsi           |   2 +
>  arch/arm/boot/dts/armada-xp-mv78460.dtsi           |   4 +
>  arch/arm/boot/dts/armada-xp.dtsi                   |   2 +-
>  arch/arm/configs/multi_v7_defconfig                |   1 +
>  arch/arm/configs/mvebu_v7_defconfig                |   2 +
>  arch/arm/mach-mvebu/Kconfig                        |   1 +
>  arch/arm/mach-mvebu/platsmp.c                      |   1 +
>  arch/arm/mach-mvebu/pmsu.c                         | 162 +++++++++++++++++++++
>  drivers/clk/mvebu/clk-cpu.c                        |  80 +++++++++-
>  include/linux/mvebu-pmsu.h                         |  20 +++
>  12 files changed, 274 insertions(+), 8 deletions(-)
>  create mode 100644 include/linux/mvebu-pmsu.h
> 



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Jason Cooper]

Thomas,

On Wed, Jul 23, 2014 at 01:19:30PM +0200, Thomas Petazzoni wrote:
> Viresh, Jason,
> 
> So, what do we do with this patch series, which depends on the
> cpufreq-generic driver? Has there been any solution found for 3.17 ?
> 
> Jason, in any case, I'd like the following patches to be merged for
> 3.17, regardless of what happens with the cpufreq driver:
> 
>  ARM: mvebu: ensure CPU clocks are enabled
>  ARM: mvebu: extend PMSU code to support dynamic frequency scaling
>  clk: mvebu: extend clk-cpu for dynamic frequency scaling

I just sent the pull for these three yesterday.

> One patch should be split:
> 
>  ARM: mvebu: update Armada XP DT for dynamic frequency scaling
> 
>  -> In this patch, the addition of clock-latency is related to the
>     cpufreq generic DT binding, so I think we shouldn't merge that. But
>     on the other hand, this patch also adds the new registers for the
>     Armada XP CPU clock, which is used by "clk: mvebu: extend clk-cpu
>     for dynamic frequency scaling".

This was a part of one of the previous DT pull requests and is already
in arm-soc.

> The patch:
> 
>  ARM: mvebu: allow enabling of cpufreq on Armada XP
> 
> can be dropped, since ARCH_HAS_CPUFREQ has been removed.

Yup, did that when Paul raised the issue.

> The other patches are defconfig changes, which are meaningless without
> the cpufreq-generic driver.

Already pushed to arm-soc.

> Jason, what do you think about me sending a new version of the patch
> series, which will have two clearly separated set of patches:
> 
>  1/ A first set of patches that can be applied regardless of what
>     happens on the cpufreq driver side. Getting it merged will not
>     bring cpufreq support, but it will add the foundations needed to
>     support it.
> 
>  2/ A second set of patches that use the cpufreq-generic driver, which
>     might get applied of the cpufreq maintainers find a solution in
>     time for 3.17. If not, then I'll re-adapt them for 3.18.

It sounds like the only patch in group 2 would be the DT change, which
has already been taken.

> What do you think?

Let's wait and see what -rc1 looks like and take action based on that.

thx,

Jason.

Re: [PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Thomas Petazzoni]

Dear Jason Cooper,

On Wed, 23 Jul 2014 07:39:45 -0400, Jason Cooper wrote:

> > What do you think?
> 
> Let's wait and see what -rc1 looks like and take action based on that.

Sure. Thanks a lot for your summary. I agree with your proposal.

I'm working on respining the cpuidle series based on the discussion
with Daniel Lezcano and Arnd Bergmann. If all goes well, you can expect
a new version today.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv3 0/7] cpufreq support for Marvell Armada XP

[Viresh Kumar]

On 23 July 2014 16:49, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> So, what do we do with this patch series, which depends on the
> cpufreq-generic driver? Has there been any solution found for 3.17 ?

I have been trying hard, nothing conclusive yet :(

[PATCHv3 3/7] clk: mvebu: extend clk-cpu for dynamic frequency scaling

[Thomas Petazzoni]

+static int clk_cpu_on_set_rate(struct clk_hw *hwclk, unsigned long rate,
+                              unsigned long parent_rate)
+{
+       u32 reg;
+       unsigned long fabric_div, target_div, cur_rate;
+       struct cpu_clk *cpuclk = to_cpu_clk(hwclk);
+
+       /*
+        * PMU DFS registers are not mapped, Device Tree does not
+        * describes them. We cannot change the frequency dynamically.
+        */
+       if (!cpuclk->pmu_dfs)
+               return -ENODEV;
+
+       cur_rate = __clk_get_rate(hwclk->clk);
+
+       reg = readl(cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL2_OFFSET);
+       fabric_div = (reg >> SYS_CTRL_CLK_DIVIDER_CTRL2_NBCLK_RATIO_SHIFT) &
+               SYS_CTRL_CLK_DIVIDER_MASK;
+
+       /* Frequency is going up */
+       if (rate == 2 * cur_rate)
+               target_div = fabric_div / 2;
+       /* Frequency is going down */
+       else
+               target_div = fabric_div;
+
+       if (target_div == 0)
+               target_div = 1;
+
+       reg = readl(cpuclk->pmu_dfs);
+       reg &= ~(PMU_DFS_RATIO_MASK << PMU_DFS_RATIO_SHIFT);
+       reg |= (target_div << PMU_DFS_RATIO_SHIFT);
+       writel(reg, cpuclk->pmu_dfs);
+
+       reg = readl(cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET);
+       reg |= (SYS_CTRL_CLK_DIVIDER_CTRL_RESET_ALL <<
+               SYS_CTRL_CLK_DIVIDER_CTRL_RESET_SHIFT);
+       writel(reg, cpuclk->reg_base + SYS_CTRL_CLK_DIVIDER_CTRL_OFFSET);
+
+       return mvebu_pmsu_dfs_request(cpuclk->cpu);
+}
+
+static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
+                           unsigned long parent_rate)
+{
+       if (__clk_is_enabled(hwclk->clk))
+               return clk_cpu_on_set_rate(hwclk, rate, parent_rate);
+       else
+               return clk_cpu_off_set_rate(hwclk, rate, parent_rate);

This is racy. You don't hold the clk_enable lock so it could be enable
between the conditional check and executing clk_cpu_on_set_rate.

How do you ensure that secondary CPU clocks are not enabled/disabled
when changing rates?

Regards,
Mike

Re: [PATCHv3 3/7] clk: mvebu: extend clk-cpu for dynamic frequency scaling

[Thomas Petazzoni]

Hello,

(Not sure why this e-mail comes with me as the From:, but anyway.)

On Wed, 23 Jul 2014 16:53:58 -0700, Thomas Petazzoni wrote:

> +static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
> +                           unsigned long parent_rate)
> +{
> +       if (__clk_is_enabled(hwclk->clk))
> +               return clk_cpu_on_set_rate(hwclk, rate, parent_rate);
> +       else
> +               return clk_cpu_off_set_rate(hwclk, rate, parent_rate);
> 
> This is racy. You don't hold the clk_enable lock so it could be enable
> between the conditional check and executing clk_cpu_on_set_rate.

Right.

> How do you ensure that secondary CPU clocks are not enabled/disabled
> when changing rates?

In practice, this currently cannot happen: we enable the secondary CPU
clocks before starting the secondary CPUs, and we never ever disable or
re-enable again those clocks. So with the present code, I believe there
is no problem. Even when we do CPU hotplug, we don't turn off the CPU
clocks, simply because they cannot be turned off: the enable/disable
state is only used here as an indication so that the clock driver knows
which frequency change strategy it should apply.

But you're anyway right, I'll send a followup patch adding the lock.
Would that be OK for you?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv3 3/7] clk: mvebu: extend clk-cpu for dynamic frequency scaling

[Mike Turquette]

Quoting Thomas Petazzoni (2014-07-23 23:33:25)
> Hello,
> 
> (Not sure why this e-mail comes with me as the From:, but anyway.)
> 
> On Wed, 23 Jul 2014 16:53:58 -0700, Thomas Petazzoni wrote:
> 
> > +static int clk_cpu_set_rate(struct clk_hw *hwclk, unsigned long rate,
> > +                           unsigned long parent_rate)
> > +{
> > +       if (__clk_is_enabled(hwclk->clk))
> > +               return clk_cpu_on_set_rate(hwclk, rate, parent_rate);
> > +       else
> > +               return clk_cpu_off_set_rate(hwclk, rate, parent_rate);
> > 
> > This is racy. You don't hold the clk_enable lock so it could be enable
> > between the conditional check and executing clk_cpu_on_set_rate.
> 
> Right.
> 
> > How do you ensure that secondary CPU clocks are not enabled/disabled
> > when changing rates?
> 
> In practice, this currently cannot happen: we enable the secondary CPU
> clocks before starting the secondary CPUs, and we never ever disable or
> re-enable again those clocks. So with the present code, I believe there
> is no problem. Even when we do CPU hotplug, we don't turn off the CPU
> clocks, simply because they cannot be turned off: the enable/disable
> state is only used here as an indication so that the clock driver knows
> which frequency change strategy it should apply.
> 
> But you're anyway right, I'll send a followup patch adding the lock.
> Would that be OK for you?

Sounds good. Can you also fix up the changelog in patch #2? After that
I am happy with this series. I guess Jason will take it in and send it
for his PR?

Thanks,
Mike

> 
> Thanks,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: [PATCHv3 3/7] clk: mvebu: extend clk-cpu for dynamic frequency scaling

[Thomas Petazzoni]

Dear Mike Turquette,

On Thu, 24 Jul 2014 10:52:51 -0700, Mike Turquette wrote:

> > > This is racy. You don't hold the clk_enable lock so it could be enable
> > > between the conditional check and executing clk_cpu_on_set_rate.
> > 
> > Right.
> > 
> > > How do you ensure that secondary CPU clocks are not enabled/disabled
> > > when changing rates?
> > 
> > In practice, this currently cannot happen: we enable the secondary CPU
> > clocks before starting the secondary CPUs, and we never ever disable or
> > re-enable again those clocks. So with the present code, I believe there
> > is no problem. Even when we do CPU hotplug, we don't turn off the CPU
> > clocks, simply because they cannot be turned off: the enable/disable
> > state is only used here as an indication so that the clock driver knows
> > which frequency change strategy it should apply.
> > 
> > But you're anyway right, I'll send a followup patch adding the lock.
> > Would that be OK for you?
> 
> Sounds good. Can you also fix up the changelog in patch #2? After that
> I am happy with this series. I guess Jason will take it in and send it
> for his PR?

The fixup for the commit log in patch #2 cannot be done, because the
commit has already been merged in arm-soc, if I understood correctly.
Jason said:

"""
> The commit log of this commit was not adjusted consequently, and this
> is my fault. Jason, is it still time to change this commit log?  

If there are no code changes, I'd prefer not to.  We're rather late in
the game.

Even though it's not ideal, the commit in question does have a Link: tag
pointing at the patch email on which this conversation is based.  So a
frustrated future developer won't be frustrated long. :)
"""

I'll do the lock patch.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com