* [PATCH v2 0/3] Kirkwoode cpufreq driver
@ 2013-01-26 15:43 Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs Andrew Lunn
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Andrew Lunn @ 2013-01-26 15:43 UTC (permalink / raw)
To: linux-arm-kernel
This patchset adds a cpufreq driver for Marvell Kirkwood SoCs.
The changes to kirkwood_defconfig enable it and set the default
governor to ondemand.
Changes since v1:
tabify Kconfig.arm entry
Sort order of include files
Remove some unnecassary include files
Reformat multiline comment to be coding style conform.
Andrew Lunn (3):
cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
arm: kirkwood: Instantiate cpufreq driver
arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig
arch/arm/Kconfig | 1 +
arch/arm/configs/kirkwood_defconfig | 3 +
arch/arm/mach-kirkwood/board-dt.c | 3 +-
arch/arm/mach-kirkwood/common.c | 23 ++
arch/arm/mach-kirkwood/common.h | 2 +
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 +
drivers/clk/mvebu/clk-gating-ctrl.c | 1 +
drivers/cpufreq/Kconfig.arm | 6 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/kirkwood-cpufreq.c | 271 +++++++++++++++++++++
10 files changed, 312 insertions(+), 1 deletion(-)
create mode 100644 drivers/cpufreq/kirkwood-cpufreq.c
--
1.7.10.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
2013-01-26 15:43 [PATCH v2 0/3] Kirkwoode cpufreq driver Andrew Lunn
@ 2013-01-26 15:43 ` Andrew Lunn
2013-01-27 3:51 ` Viresh Kumar
2013-01-26 15:43 ` [PATCH v2 2/3] arm: kirkwood: Instantiate cpufreq driver Andrew Lunn
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2013-01-26 15:43 UTC (permalink / raw)
To: linux-arm-kernel
The Marvell Kirkwood SoCs have simple cpufreq support in hardware. The
CPU can either use the a high speed cpu clock, or the slower DDR
clock. Add a driver to swap between these two clock sources.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/clk/mvebu/clk-gating-ctrl.c | 1 +
drivers/cpufreq/Kconfig.arm | 6 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/kirkwood-cpufreq.c | 271 +++++++++++++++++++++++++++++++++++
4 files changed, 279 insertions(+)
create mode 100644 drivers/cpufreq/kirkwood-cpufreq.c
diff --git a/drivers/clk/mvebu/clk-gating-ctrl.c b/drivers/clk/mvebu/clk-gating-ctrl.c
index 8fa5408..ebf141d 100644
--- a/drivers/clk/mvebu/clk-gating-ctrl.c
+++ b/drivers/clk/mvebu/clk-gating-ctrl.c
@@ -193,6 +193,7 @@ static const struct mvebu_soc_descr __initconst kirkwood_gating_descr[] = {
{ "runit", NULL, 7 },
{ "xor0", NULL, 8 },
{ "audio", NULL, 9 },
+ { "powersave", "cpuclk", 11 },
{ "sata0", NULL, 14 },
{ "sata1", NULL, 15 },
{ "xor1", NULL, 16 },
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index a0b3661..80c8229 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -77,6 +77,12 @@ config ARM_EXYNOS5250_CPUFREQ
This adds the CPUFreq driver for Samsung EXYNOS5250
SoC.
+config ARM_KIRKWOOD_CPUFREQ
+ def_bool ARCH_KIRKWOOD && OF
+ help
+ This adds the CPUFreq driver for Marvell Kirkwood
+ SoCs.
+
config ARM_SPEAR_CPUFREQ
bool "SPEAr CPUFreq support"
depends on PLAT_SPEAR
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index fadc4d4..39a0ffe 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_ARM_EXYNOS_CPUFREQ) += exynos-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ) += exynos4210-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ) += exynos4x12-cpufreq.o
obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ) += exynos5250-cpufreq.o
+obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_SPEAR_CPUFREQ) += spear-cpufreq.o
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
new file mode 100644
index 0000000..300d01d
--- /dev/null
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -0,0 +1,271 @@
+/*
+ * kirkwood_freq.c: cpufreq driver for the Marvell kirkwood
+ *
+ * Copyright (C) 2013 Andrew Lunn <andrew@lunn.ch>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/cpufreq.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <asm/proc-fns.h>
+
+#define CPU_SW_INT_BLK BIT(28)
+
+
+#include <linux/clk-private.h>
+
+static struct priv
+{
+ struct clk *cpu_clk;
+ struct clk *ddr_clk;
+ struct clk *powersave_clk;
+ struct device *dev;
+ void __iomem *base;
+} priv;
+
+#define STATE_CPU_FREQ 0x01
+#define STATE_DDR_FREQ 0x02
+
+/*
+ * Kirkwood can swap the clock to the CPU between two clocks:
+ *
+ * - cpu clk
+ * - ddr clk
+ *
+ * The frequencies are set at runtime before registering this *
+ * table.
+ */
+static struct cpufreq_frequency_table kirkwood_freq_table[] = {
+ {STATE_CPU_FREQ, 0}, /* CPU uses cpuclk */
+ {STATE_DDR_FREQ, 0}, /* CPU uses ddrclk */
+ {0, CPUFREQ_TABLE_END},
+};
+
+static unsigned int kirkwood_cpufreq_get_cpu_frequency(unsigned int cpu)
+{
+ if (__clk_is_enabled(priv.powersave_clk))
+ return kirkwood_freq_table[1].frequency;
+ return kirkwood_freq_table[0].frequency;
+}
+
+static void kirkwood_cpufreq_set_cpu_state(unsigned int index)
+{
+
+ struct cpufreq_freqs freqs;
+ unsigned int state = kirkwood_freq_table[index].index;
+ unsigned long reg;
+
+ freqs.old = kirkwood_cpufreq_get_cpu_frequency(0);
+ freqs.new = kirkwood_freq_table[index].frequency;
+ freqs.cpu = 0; /* Kirkwood is UP */
+
+ cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+ dev_dbg(priv.dev, "Attempting to set frequency to %i KHz\n",
+ kirkwood_freq_table[index].frequency);
+ dev_dbg(priv.dev, "old frequency was %i KHz\n",
+ kirkwood_cpufreq_get_cpu_frequency(0));
+
+ if (freqs.old != freqs.new) {
+ local_irq_disable();
+
+ /* Disable interrupts to the CPU */
+ reg = readl_relaxed(priv.base);
+ reg |= CPU_SW_INT_BLK;
+ writel(reg, priv.base);
+
+ switch (state) {
+ case STATE_CPU_FREQ:
+ clk_disable(priv.powersave_clk);
+ break;
+ case STATE_DDR_FREQ:
+ clk_enable(priv.powersave_clk);
+ break;
+ default:
+ dev_err(priv.dev, "Unexpected cpufreq state");
+ }
+
+ /* Wait-for-Interrupt, which the hardware changes frequency */
+ cpu_do_idle();
+
+ /* Enable interrupts to the CPU */
+ reg = readl_relaxed(priv.base);
+ reg &= ~CPU_SW_INT_BLK;
+ writel(reg, priv.base);
+
+ local_irq_enable();
+ }
+ cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+};
+
+static int kirkwood_cpufreq_verify(struct cpufreq_policy *policy)
+{
+ return cpufreq_frequency_table_verify(policy, &kirkwood_freq_table[0]);
+}
+
+static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,
+ unsigned int target_freq,
+ unsigned int relation)
+{
+ unsigned int index = 0;
+
+ if (cpufreq_frequency_table_target(policy, kirkwood_freq_table,
+ target_freq, relation, &index))
+ return -EINVAL;
+
+ kirkwood_cpufreq_set_cpu_state(index);
+
+ return 0;
+}
+
+/*
+ * Module init and exit code
+ */
+static int kirkwood_cpufreq_cpu_init(struct cpufreq_policy *policy)
+{
+ int result;
+
+ /* cpuinfo and default policy values */
+ policy->cpuinfo.transition_latency = 5000; /* 5uS */
+ policy->cur = kirkwood_cpufreq_get_cpu_frequency(0);
+
+ result = cpufreq_frequency_table_cpuinfo(policy, kirkwood_freq_table);
+ if (result)
+ return result;
+
+ cpufreq_frequency_table_get_attr(kirkwood_freq_table, policy->cpu);
+
+ return 0;
+}
+
+static int kirkwood_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+ cpufreq_frequency_table_put_attr(policy->cpu);
+ return 0;
+}
+
+static struct freq_attr *kirkwood_cpufreq_attr[] = {
+ &cpufreq_freq_attr_scaling_available_freqs,
+ NULL,
+};
+
+
+static struct cpufreq_driver kirkwood_cpufreq_driver = {
+ .get = kirkwood_cpufreq_get_cpu_frequency,
+ .verify = kirkwood_cpufreq_verify,
+ .target = kirkwood_cpufreq_target,
+ .init = kirkwood_cpufreq_cpu_init,
+ .exit = kirkwood_cpufreq_cpu_exit,
+ .name = "kirkwood_freq",
+ .owner = THIS_MODULE,
+ .attr = kirkwood_cpufreq_attr,
+};
+
+static int kirkwood_cpufreq_probe(struct platform_device *pdev)
+{
+ struct device_node *np = of_find_compatible_node(
+ NULL, NULL, "marvell,kirkwood-core-clock");
+
+ struct of_phandle_args clkspec;
+ struct resource *res;
+ int err;
+
+ priv.dev = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(&pdev->dev, "Cannot get memory resource\n");
+ return -ENODEV;
+ }
+ priv.base = devm_request_and_ioremap(&pdev->dev, res);
+ if (!priv.base) {
+ dev_err(&pdev->dev, "Cannot ioremap\n");
+ return -ENOMEM;
+ }
+
+ clkspec.np = np;
+ clkspec.args_count = 1;
+ clkspec.args[0] = 1;
+
+ priv.cpu_clk = of_clk_get_from_provider(&clkspec);
+ if (IS_ERR(priv.cpu_clk)) {
+ dev_err(priv.dev, "Unable to get cpuclk");
+ return PTR_ERR(priv.cpu_clk);
+ }
+
+ clk_prepare_enable(priv.cpu_clk);
+ kirkwood_freq_table[0].frequency = clk_get_rate(priv.cpu_clk) / 1000;
+
+ clkspec.args[0] = 3;
+ priv.ddr_clk = of_clk_get_from_provider(&clkspec);
+ if (IS_ERR(priv.ddr_clk)) {
+ dev_err(priv.dev, "Unable to get ddrclk");
+ err = PTR_ERR(priv.ddr_clk);
+ goto out_cpu;
+ }
+
+ clk_prepare_enable(priv.ddr_clk);
+ kirkwood_freq_table[1].frequency = clk_get_rate(priv.ddr_clk) / 1000;
+
+ np = of_find_compatible_node(NULL, NULL,
+ "marvell,kirkwood-gating-clock");
+ clkspec.np = np;
+ clkspec.args[0] = 11;
+ priv.powersave_clk = of_clk_get_from_provider(&clkspec);
+ if (IS_ERR(priv.powersave_clk)) {
+ dev_err(priv.dev, "Unable to get powersave");
+ err = PTR_ERR(priv.powersave_clk);
+ goto out_ddr;
+ }
+ clk_prepare(priv.powersave_clk);
+
+ err = cpufreq_register_driver(&kirkwood_cpufreq_driver);
+ if (!err)
+ return 0;
+ dev_err(priv.dev, "Failed to register cpufreq driver");
+
+ clk_disable_unprepare(priv.powersave_clk);
+out_ddr:
+ clk_disable_unprepare(priv.ddr_clk);
+out_cpu:
+ clk_disable_unprepare(priv.cpu_clk);
+
+ return err;
+}
+
+
+static int kirkwood_cpufreq_remove(struct platform_device *pdev)
+{
+ cpufreq_unregister_driver(&kirkwood_cpufreq_driver);
+
+ clk_disable_unprepare(priv.powersave_clk);
+ clk_disable_unprepare(priv.ddr_clk);
+ clk_disable_unprepare(priv.cpu_clk);
+
+ return 0;
+}
+
+static struct platform_driver kirkwood_cpufreq_platform_driver = {
+ .probe = kirkwood_cpufreq_probe,
+ .remove = kirkwood_cpufreq_remove,
+ .driver = {
+ .name = "kirkwood-cpufreq",
+ .owner = THIS_MODULE,
+ },
+};
+
+module_platform_driver(kirkwood_cpufreq_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andrew Lunn <andrew@lunn.ch");
+MODULE_DESCRIPTION("cpufreq driver for Marvell's kirkwood CPU");
+MODULE_ALIAS("platform:kirkwood-cpufreq");
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] arm: kirkwood: Instantiate cpufreq driver
2013-01-26 15:43 [PATCH v2 0/3] Kirkwoode cpufreq driver Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs Andrew Lunn
@ 2013-01-26 15:43 ` Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 3/3] arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig Andrew Lunn
2013-01-26 21:52 ` [PATCH v2 0/3] Kirkwoode cpufreq driver Rafael J. Wysocki
3 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2013-01-26 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Register a platform driver structure for the cpufreq driver.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-kirkwood/board-dt.c | 3 ++-
arch/arm/mach-kirkwood/common.c | 23 +++++++++++++++++++++
arch/arm/mach-kirkwood/common.h | 2 ++
arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 ++
5 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 67874b8..830975b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -543,6 +543,7 @@ config ARCH_DOVE
config ARCH_KIRKWOOD
bool "Marvell Kirkwood"
+ select ARCH_HAS_CPUFREQ
select ARCH_REQUIRE_GPIOLIB
select CPU_FEROCEON
select GENERIC_CLOCKEVENTS
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c
index de4fd2b..fab541d 100644
--- a/arch/arm/mach-kirkwood/board-dt.c
+++ b/arch/arm/mach-kirkwood/board-dt.c
@@ -70,7 +70,6 @@ static void __init kirkwood_legacy_clk_init(void)
clkspec.args[0] = CGC_BIT_SDIO;
orion_clkdev_add(NULL, "mvsdio",
of_clk_get_from_provider(&clkspec));
-
}
static void __init kirkwood_of_clk_init(void)
@@ -95,6 +94,8 @@ static void __init kirkwood_dt_init(void)
kirkwood_l2_init();
+ kirkwood_cpufreq_init();
+
/* Setup root of clk tree */
kirkwood_of_clk_init();
diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index bac21a5..a3dc21c 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -584,6 +584,29 @@ void __init kirkwood_audio_init(void)
}
/*****************************************************************************
+ * CPU Frequency
+ ****************************************************************************/
+static struct resource kirkwood_cpufreq_resources[] = {
+ [0] = {
+ .start = CPU_CONTROL_PHYS,
+ .end = CPU_CONTROL_PHYS + 3,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device kirkwood_cpufreq_device = {
+ .name = "kirkwood-cpufreq",
+ .id = -1,
+ .num_resources = ARRAY_SIZE(kirkwood_cpufreq_resources),
+ .resource = kirkwood_cpufreq_resources,
+};
+
+void __init kirkwood_cpufreq_init(void)
+{
+ platform_device_register(&kirkwood_cpufreq_device);
+}
+
+/*****************************************************************************
* General
****************************************************************************/
/*
diff --git a/arch/arm/mach-kirkwood/common.h b/arch/arm/mach-kirkwood/common.h
index 5ffa57f..9ede04b 100644
--- a/arch/arm/mach-kirkwood/common.h
+++ b/arch/arm/mach-kirkwood/common.h
@@ -50,6 +50,8 @@ void kirkwood_nand_init(struct mtd_partition *parts, int nr_parts, int delay);
void kirkwood_nand_init_rnb(struct mtd_partition *parts, int nr_parts,
int (*dev_ready)(struct mtd_info *));
void kirkwood_audio_init(void);
+void kirkwood_cpufreq_init(void);
+
void kirkwood_restart(char, const char *);
void kirkwood_clk_init(void);
diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
index 5c82b7d..d4cbe5e 100644
--- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
+++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
@@ -17,6 +17,7 @@
#define CPU_CONFIG_ERROR_PROP 0x00000004
#define CPU_CONTROL (BRIDGE_VIRT_BASE + 0x0104)
+#define CPU_CONTROL_PHYS (BRIDGE_PHYS_BASE + 0x0104)
#define CPU_RESET 0x00000002
#define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
@@ -69,6 +70,7 @@
#define CGC_RUNIT (1 << 7)
#define CGC_XOR0 (1 << 8)
#define CGC_AUDIO (1 << 9)
+#define CGC_POWERSAVE (1 << 11)
#define CGC_SATA0 (1 << 14)
#define CGC_SATA1 (1 << 15)
#define CGC_XOR1 (1 << 16)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig
2013-01-26 15:43 [PATCH v2 0/3] Kirkwoode cpufreq driver Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 2/3] arm: kirkwood: Instantiate cpufreq driver Andrew Lunn
@ 2013-01-26 15:43 ` Andrew Lunn
2013-01-26 21:52 ` [PATCH v2 0/3] Kirkwoode cpufreq driver Rafael J. Wysocki
3 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2013-01-26 15:43 UTC (permalink / raw)
To: linux-arm-kernel
Now that we have a cpufreq driver for kirkwood, enable it in
kirkwood_defconfig and set the default governer to ondemand.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/configs/kirkwood_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/kirkwood_defconfig b/arch/arm/configs/kirkwood_defconfig
index 93f3794..6ecb7de 100644
--- a/arch/arm/configs/kirkwood_defconfig
+++ b/arch/arm/configs/kirkwood_defconfig
@@ -55,6 +55,9 @@ CONFIG_AEABI=y
# CONFIG_OABI_COMPAT is not set
CONFIG_ZBOOT_ROM_TEXT=0x0
CONFIG_ZBOOT_ROM_BSS=0x0
+CONFIG_CPU_FREQ=y
+CONFIG_CPU_FREQ_STAT_DETAILS=y
+CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
CONFIG_CPU_IDLE=y
CONFIG_NET=y
CONFIG_PACKET=y
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 0/3] Kirkwoode cpufreq driver
2013-01-26 15:43 [PATCH v2 0/3] Kirkwoode cpufreq driver Andrew Lunn
` (2 preceding siblings ...)
2013-01-26 15:43 ` [PATCH v2 3/3] arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig Andrew Lunn
@ 2013-01-26 21:52 ` Rafael J. Wysocki
2013-01-26 23:04 ` Jason Cooper
3 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-01-26 21:52 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday, January 26, 2013 04:43:56 PM Andrew Lunn wrote:
> This patchset adds a cpufreq driver for Marvell Kirkwood SoCs.
>
> The changes to kirkwood_defconfig enable it and set the default
> governor to ondemand.
>
> Changes since v1:
>
> tabify Kconfig.arm entry
> Sort order of include files
> Remove some unnecassary include files
> Reformat multiline comment to be coding style conform.
>
> Andrew Lunn (3):
> cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
> arm: kirkwood: Instantiate cpufreq driver
> arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig
If you want me to take this, it'll need some ACKs from the clk people
and arm-soc maintainers at least.
An ACK from Viresh would be welcome too.
Thanks,
Rafael
> arch/arm/Kconfig | 1 +
> arch/arm/configs/kirkwood_defconfig | 3 +
> arch/arm/mach-kirkwood/board-dt.c | 3 +-
> arch/arm/mach-kirkwood/common.c | 23 ++
> arch/arm/mach-kirkwood/common.h | 2 +
> arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 2 +
> drivers/clk/mvebu/clk-gating-ctrl.c | 1 +
> drivers/cpufreq/Kconfig.arm | 6 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/kirkwood-cpufreq.c | 271 +++++++++++++++++++++
> 10 files changed, 312 insertions(+), 1 deletion(-)
> create mode 100644 drivers/cpufreq/kirkwood-cpufreq.c
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/3] Kirkwoode cpufreq driver
2013-01-26 21:52 ` [PATCH v2 0/3] Kirkwoode cpufreq driver Rafael J. Wysocki
@ 2013-01-26 23:04 ` Jason Cooper
2013-01-27 22:39 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Jason Cooper @ 2013-01-26 23:04 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Jan 26, 2013 at 10:52:42PM +0100, Rafael J. Wysocki wrote:
> On Saturday, January 26, 2013 04:43:56 PM Andrew Lunn wrote:
> > This patchset adds a cpufreq driver for Marvell Kirkwood SoCs.
> >
> > The changes to kirkwood_defconfig enable it and set the default
> > governor to ondemand.
> >
> > Changes since v1:
> >
> > tabify Kconfig.arm entry
> > Sort order of include files
> > Remove some unnecassary include files
> > Reformat multiline comment to be coding style conform.
> >
> > Andrew Lunn (3):
> > cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
> > arm: kirkwood: Instantiate cpufreq driver
> > arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig
>
> If you want me to take this, it'll need some ACKs from the clk people
> and arm-soc maintainers at least.
In the past, we've sent the driver changes (patch #1) through the
appropriate driver maintainer's tree, and the subsequent patches (2 & 3)
through arm-soc with a listed dependency on the driver maintainer's
tree/branch. We've found that causes the fewest conflicts/headaches
that way.
If you're Ok with that, just let me know which tree and branch you pull
it in to.
> An ACK from Viresh would be welcome too.
fwiw:
Acked-by: Jason Cooper <jason@lakedaemon.net>
thx,
Jason.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
2013-01-26 15:43 ` [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs Andrew Lunn
@ 2013-01-27 3:51 ` Viresh Kumar
2013-01-27 8:59 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Viresh Kumar @ 2013-01-27 3:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi Andrew,
Few more comments from my side :(
On 26 January 2013 21:13, Andrew Lunn <andrew@lunn.ch> wrote:
> diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/cpufreq.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <asm/proc-fns.h>
> +
> +#define CPU_SW_INT_BLK BIT(28)
> +
> +
> +#include <linux/clk-private.h>
Any specific reason for keeping it together with other #includes?
> +static void kirkwood_cpufreq_set_cpu_state(unsigned int index)
> +{
> +
Can remove extra blank line.
> + if (freqs.old != freqs.new) {
> + local_irq_disable();
> +
> + /* Disable interrupts to the CPU */
> + reg = readl_relaxed(priv.base);
> + reg |= CPU_SW_INT_BLK;
> + writel(reg, priv.base);
Any specific reason for calling writel() instead of writel_relaxed()?
relaxed one would take less time and would be much more efficient.
same for all other writel's too.
> +};
> +
> +static int kirkwood_cpufreq_verify(struct cpufreq_policy *policy)
> +{
> + return cpufreq_frequency_table_verify(policy, &kirkwood_freq_table[0]);
return cpufreq_frequency_table_verify(policy, kirkwood_freq_table); ??
> +}
> +
> +static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,
> + unsigned int target_freq,
> + unsigned int relation)
> +{
> + unsigned int index = 0;
> +
> + if (cpufreq_frequency_table_target(policy, kirkwood_freq_table,
> + target_freq, relation, &index))
> + return -EINVAL;
> +
> + kirkwood_cpufreq_set_cpu_state(index);
This routine does have error cases, like: wrong value of "state"
variable, so returning int from it might make sense. Though i believe
state can't be anything else then STATE_CPU_FREQ or STATE_DDR_FREQ.
In which case, switch must be modified?
> + return 0;
> +}
> +
> +/*
> + * Module init and exit code
> + */
Can be converted to a single line comment if you want. :)
> +static int kirkwood_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> +{
> + cpufreq_frequency_table_put_attr(policy->cpu);
> + return 0;
> +}
Hmm.. Exit is normally called in two cases. Either cpufreq driver is
unregistered
or cpu is removed. In that case i am not sure if this routine makes sense? As
we have a uniprocessor SoC here.
@Rafael?
> +static struct freq_attr *kirkwood_cpufreq_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + NULL,
> +};
> +
> +
Can remove extra blank line.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
2013-01-27 3:51 ` Viresh Kumar
@ 2013-01-27 8:59 ` Andrew Lunn
2013-01-27 14:43 ` Viresh Kumar
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2013-01-27 8:59 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Jan 27, 2013 at 09:21:42AM +0530, Viresh Kumar wrote:
> Hi Andrew,
>
> Few more comments from my side :(
No problems...
>
> On 26 January 2013 21:13, Andrew Lunn <andrew@lunn.ch> wrote:
> > diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/clk.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <asm/proc-fns.h>
> > +
> > +#define CPU_SW_INT_BLK BIT(28)
> > +
> > +
> > +#include <linux/clk-private.h>
>
> Any specific reason for keeping it together with other #includes?
Ah, yes, there is a good reason. I added this in for debugging
purposes. I needed to look inside the clk cookie, which required this
include file. But because it was for debug only, to be removed later,
i kept it separate, easier to find and remove. I just forgot to remove
it :-(
> > + if (freqs.old != freqs.new) {
> > + local_irq_disable();
> > +
> > + /* Disable interrupts to the CPU */
> > + reg = readl_relaxed(priv.base);
> > + reg |= CPU_SW_INT_BLK;
> > + writel(reg, priv.base);
> > +static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,
> > + unsigned int target_freq,
> > + unsigned int relation)
> > +{
> > + unsigned int index = 0;
> > +
> > + if (cpufreq_frequency_table_target(policy, kirkwood_freq_table,
> > + target_freq, relation, &index))
> > + return -EINVAL;
> > +
> > + kirkwood_cpufreq_set_cpu_state(index);
>
> This routine does have error cases, like: wrong value of "state"
> variable, so returning int from it might make sense. Though i believe
> state can't be anything else then STATE_CPU_FREQ or STATE_DDR_FREQ.
> In which case, switch must be modified?
As you say, state cannot be anything else that the two expected
values. I've just been taught that switch statements should always
have a default clause. I also thought that a BUG() is too strong, no
need to kill the machine. It is better to spam the kernel log so it
gets noticed. I can swap to a WARN_ON(1). I don't really think this is
an error that needs to be reported back to the framework. Its an
implementation problem, not a runtime problem. So i would prefer to
keep to void.
> > +static int kirkwood_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> > +{
> > + cpufreq_frequency_table_put_attr(policy->cpu);
> > + return 0;
> > +}
>
> Hmm.. Exit is normally called in two cases. Either cpufreq driver is
> unregistered
> or cpu is removed. In that case i am not sure if this routine makes sense? As
> we have a uniprocessor SoC here.
The current Kconfig entry does not allow it to be compiled as a
module. But the code does allow for it. With the current trend of
making the ARM kernel multiplatform, its likely that cpufreq drivers
will become modular so that only the appropriate driver gets loaded in
a multiplatform kernel. The question then becomes, is it O.K. not
being able to unload the module?
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
2013-01-27 8:59 ` Andrew Lunn
@ 2013-01-27 14:43 ` Viresh Kumar
0 siblings, 0 replies; 10+ messages in thread
From: Viresh Kumar @ 2013-01-27 14:43 UTC (permalink / raw)
To: linux-arm-kernel
On 27 January 2013 14:29, Andrew Lunn <andrew@lunn.ch> wrote:
> On Sun, Jan 27, 2013 at 09:21:42AM +0530, Viresh Kumar wrote:
>> > + kirkwood_cpufreq_set_cpu_state(index);
>>
>> This routine does have error cases, like: wrong value of "state"
>> variable, so returning int from it might make sense. Though i believe
>> state can't be anything else then STATE_CPU_FREQ or STATE_DDR_FREQ.
>> In which case, switch must be modified?
>
> As you say, state cannot be anything else that the two expected
> values. I've just been taught that switch statements should always
> have a default clause. I also thought that a BUG() is too strong, no
> need to kill the machine. It is better to spam the kernel log so it
> gets noticed. I can swap to a WARN_ON(1). I don't really think this is
> an error that needs to be reported back to the framework. Its an
> implementation problem, not a runtime problem. So i would prefer to
> keep to void.
Hmm. What i meant to say was, you just need an if,else instead of switch
and you really don't have to take care of anything else than these two values.
As that can't happen. Even the WARN_ON(). Its useless. Just believe on
what is returned from cpufreq_frequency_table_target(). You really don't
need to validate index returned from it, but only the return value (which
you are already doing)
>> > +static int kirkwood_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>> > +{
>> > + cpufreq_frequency_table_put_attr(policy->cpu);
>> > + return 0;
>> > +}
>>
>> Hmm.. Exit is normally called in two cases. Either cpufreq driver is
>> unregistered
>> or cpu is removed. In that case i am not sure if this routine makes sense? As
>> we have a uniprocessor SoC here.
>
> The current Kconfig entry does not allow it to be compiled as a
> module. But the code does allow for it. With the current trend of
> making the ARM kernel multiplatform, its likely that cpufreq drivers
> will become modular so that only the appropriate driver gets loaded in
> a multiplatform kernel. The question then becomes, is it O.K. not
> being able to unload the module?
You got me wrong. What i wanted to say is, even if you have it as module
and "rmmod" it, then also calling this routine wouldn't make any difference.
Then i went into the code to see the effect. So, this will set freq_table for
a cpu as NULL which will be returned by calling cpufreq_frequency_get_table().
And i now feel we must have this routine because once cpufreq driver isn't
there for a cpu, we must not return any table for it. :)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/3] Kirkwoode cpufreq driver
2013-01-26 23:04 ` Jason Cooper
@ 2013-01-27 22:39 ` Rafael J. Wysocki
0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2013-01-27 22:39 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday, January 26, 2013 06:04:59 PM Jason Cooper wrote:
> On Sat, Jan 26, 2013 at 10:52:42PM +0100, Rafael J. Wysocki wrote:
> > On Saturday, January 26, 2013 04:43:56 PM Andrew Lunn wrote:
> > > This patchset adds a cpufreq driver for Marvell Kirkwood SoCs.
> > >
> > > The changes to kirkwood_defconfig enable it and set the default
> > > governor to ondemand.
> > >
> > > Changes since v1:
> > >
> > > tabify Kconfig.arm entry
> > > Sort order of include files
> > > Remove some unnecassary include files
> > > Reformat multiline comment to be coding style conform.
> > >
> > > Andrew Lunn (3):
> > > cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs
> > > arm: kirkwood: Instantiate cpufreq driver
> > > arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig
> >
> > If you want me to take this, it'll need some ACKs from the clk people
> > and arm-soc maintainers at least.
>
> In the past, we've sent the driver changes (patch #1) through the
> appropriate driver maintainer's tree, and the subsequent patches (2 & 3)
> through arm-soc with a listed dependency on the driver maintainer's
> tree/branch. We've found that causes the fewest conflicts/headaches
> that way.
>
> If you're Ok with that, just let me know which tree and branch you pull
> it in to.
Yes, that would be fine by me. In the future, though, please say in the
comment part of the changelog (i.e. between the sign-off tag and the actual
patch) who's the target maintainer of the patch.
When patch [1/3] is ready, it will be applied to the pm-cpufreq branch of the
linux-pm.git tree (that branch doesn't contain any new material at the
moment, because I'm having issues with one of the candidate commits and
I'm debugging them).
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-01-27 22:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-26 15:43 [PATCH v2 0/3] Kirkwoode cpufreq driver Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 1/3] cpufreq: kirkwood: Add a cpufreq driver for Marvell Kirkwood SoCs Andrew Lunn
2013-01-27 3:51 ` Viresh Kumar
2013-01-27 8:59 ` Andrew Lunn
2013-01-27 14:43 ` Viresh Kumar
2013-01-26 15:43 ` [PATCH v2 2/3] arm: kirkwood: Instantiate cpufreq driver Andrew Lunn
2013-01-26 15:43 ` [PATCH v2 3/3] arm: kirkwood: Enable cpufreq and ondemand on kirkwood_defconfig Andrew Lunn
2013-01-26 21:52 ` [PATCH v2 0/3] Kirkwoode cpufreq driver Rafael J. Wysocki
2013-01-26 23:04 ` Jason Cooper
2013-01-27 22:39 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).