[PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
This patch adds dvfs support for exynos5440 SOC. This soc has 4 cores and
they scale at same frequency. The nature of exynos5440 clock controller is
different from previous exynos controllers so not using the common exynos
cpufreq framework. The major difference being interrupt notfication for
frequency change. Also, OPP library is used for device tree parsing to get
different parameters like frequency, voltage etc. Since the opp library sorts
the frequency table in ascending order so they are again re-arranged in
descending order. This will have one-to-one mapping with the clock controller
state management logic.
Signed-off-by: Amit Daniel Kachhap
---
Changes in V3:
* Converted the driver to probe based as suggested by Viresh. This is also
beneficial for multiplatform kernel.
* Other coding guidelines related changes.
Changes in V2:
* Added OPP library support to parse DT parameters.
* Removed a hack to handle interrupts in bootup.
* Implemented other review comments from Viresh and Inder.
All these patches are dependent on Thomas Abraham common clock patches.
(http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg15860.html)
This whole patch series is based on 3.9-rc1.
.../bindings/cpufreq/cpufreq-exynos5440.txt| 29 ++
drivers/cpufreq/Kconfig.arm|9 +
drivers/cpufreq/Makefile |1 +
drivers/cpufreq/exynos5440-cpufreq.c | 467
4 files changed, 506 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
create mode 100644 drivers/cpufreq/exynos5440-cpufreq.c
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
new file mode 100644
index 000..a0dbe0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
@@ -0,0 +1,29 @@
+
+Exynos5440 cpufreq driver
+---
+
+Exynos5440 SoC cpufreq driver for CPU frequency scaling.
+
+Required properties:
+- interrupts: Interrupt to know the completion of cpu frequency change.
+- operating-points: Table of frequencies and voltage CPU could be transitioned
into,
+ in the decreasing order. Frequency should be in KHZ units and voltage
+ should be in microvolts.
+
+Optional properties:
+- clock-latency: Clock monitor latency in microsecond.
+
+All the required listed above must be defined under node cpufreq.
+
+Example:
+
+ cpufreq@16 {
+ compatible = "samsung,exynos5440-cpufreq";
+ reg = <0x16 0x1000>;
+ interrupts = <0 57 0>;
+ operating-points = <
+ 100 975000
+ 80 925000>;
+ clock-latency = <10>;
+ };
+
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 030ddf6..7ed9c4a 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -77,6 +77,15 @@ config ARM_EXYNOS5250_CPUFREQ
This adds the CPUFreq driver for Samsung EXYNOS5250
SoC.
+config ARM_EXYNOS5440_CPUFREQ
+ def_bool SOC_EXYNOS5440
+ depends on HAVE_CLK && PM_OPP && OF
+ help
+ This adds the CPUFreq driver for Samsung EXYNOS5440
+ SoC. The nature of exynos5440 clock controller is
+ different than previous exynos controllers so not using
+ the common exynos framework.
+
config ARM_KIRKWOOD_CPUFREQ
def_bool ARCH_KIRKWOOD && OF
help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 863fd18..c841438 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -52,6 +52,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_EXYNOS5440_CPUFREQ) += exynos5440-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/exynos5440-cpufreq.c
b/drivers/cpufreq/exynos5440-cpufreq.c
new file mode 100644
index 000..ea3d46d
--- /dev/null
+++ b/drivers/cpufreq/exynos5440-cpufreq.c
@@ -0,0 +1,467 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Amit Daniel Kachhap
+ *
+ * EXYNOS5440 - CPU frequency scaling support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
On 03/07/2013 05:13 AM, Amit Daniel Kachhap wrote:
> +/* Register definations */
s/definations/definitions
> +#define XMU_DVFS_CTRL0x0060
> +#define XMU_PMU_P0_7 0x0064
> +#define XMU_C0_3_PSTATE 0x0090
> +#define XMU_P_LIMIT 0x00A0
> +#define XMU_P_STATUS 0x00A4
> +#define XMU_PMUEVTEN 0x00D0
> +#define XMU_PMUIRQEN 0x00D4
> +#define XMU_PMUIRQ 0x00D8
Keeping all hex numbers lower case might be a good idea.
> +
> +/* PMU mask and shift definations */
> +#define P_VALUE_MASK 0x7
> +
> +#define XMU_DVFS_CTRL_EN_SHIFT 0
> +
> +#define P0_7_CPUCLKDEV_SHIFT 21
> +#define P0_7_CPUCLKDEV_MASK 0x7
> +#define P0_7_ATBCLKDEV_SHIFT 18
> +#define P0_7_ATBCLKDEV_MASK 0x7
> +#define P0_7_CSCLKDEV_SHIFT 15
> +#define P0_7_CSCLKDEV_MASK 0x7
> +#define P0_7_CPUEMA_SHIFT28
> +#define P0_7_CPUEMA_MASK 0xf
> +#define P0_7_L2EMA_SHIFT 24
> +#define P0_7_L2EMA_MASK 0xf
...
> +static int exynos_cpufreq_probe(struct platform_device *pdev)
> +{
> + int ret = -EINVAL;
> + struct device_node *np;
> + struct resource res;
> +
> + np = of_find_compatible_node(NULL, NULL, "samsung,exynos5440-cpufreq");
> + if (!np)
> + return -ENODEV;
> +
> + dvfs_info = devm_kzalloc(&pdev->dev, sizeof(*dvfs_info), GFP_KERNEL);
> + if (!dvfs_info) {
> + ret = -ENOMEM;
> + goto err_put_node;
> + }
> +
> + dvfs_info->dev = &pdev->dev;
> + dvfs_info->dev->of_node = np;
> +
> + ret = of_address_to_resource(np, 0, &res);
> + if (ret)
> + goto err_put_node;
> +
> + dvfs_info->base = devm_ioremap(dvfs_info->dev, res.start,
There is a devm_ioremap_resource() function that has been introduced
recently. It could simplify this code a bit and is preferred over
devm_ioremap().
> + resource_size(&res));
> + if (!dvfs_info->base) {
> + pr_err("No cpufreq memory map found\n");
> + ret = -ENODEV;
> + goto err_put_node;
> + }
> +
> + dvfs_info->irq = irq_of_parse_and_map(np, 0);
> + if (dvfs_info->irq == 0) {
> + pr_err("No cpufreq irq found\n");
Wouldn't dev_err() be more appropriate here ?
> + ret = -ENODEV;
> + goto err_put_node;
> + }
> +
> + ret = of_init_opp_table(dvfs_info->dev);
> + if (ret) {
> + pr_err("failed to init OPP table: %d\n", ret);
> + goto err_put_node;
> + }
> +
> + ret = opp_init_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table);
> + if (ret) {
> + pr_err("failed to init cpufreq table: %d\n", ret);
> + goto err_put_node;
> + }
> + dvfs_info->freq_count = opp_get_opp_count(dvfs_info->dev);
> + exynos_sort_descend_freq_table();
> +
> + if (of_property_read_u32(np, "clock-latency", &dvfs_info->latency))
> + dvfs_info->latency = DEF_TRANS_LATENCY;
> +
> + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
> + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
devm_clk_get() return value needs to be checked with IS_ERR(),
not IS_ERR_OR_NULL().
> + pr_err("Failed to get cpu clock\n");
> + ret = PTR_ERR(dvfs_info->cpu_clk);
> + goto err_free_table;
> + }
> +
> + dvfs_info->cur_frequency = clk_get_rate(dvfs_info->cpu_clk);
> + if (!dvfs_info->cur_frequency) {
> + pr_err("Failed to get clock rate\n");
> + ret = -EINVAL;
> + goto err_free_table;
> + }
> + dvfs_info->cur_frequency /= 1000;
> +
> + INIT_WORK(&dvfs_info->irq_work, exynos_cpufreq_work);
> + if (devm_request_irq(dvfs_info->dev, dvfs_info->irq, exynos_cpufreq_irq,
> + IRQF_TRIGGER_NONE, CPUFREQ_NAME, dvfs_info)) {
> + pr_err("Failed to register IRQ\n");
> + ret = -ENODEV;
> + goto err_free_table;
> + }
> +
> + ret = init_div_table();
> + if (ret) {
> + pr_err("Failed to initialise div table\n");
> + goto err_free_table;
> + }
> +
> + exynos_enable_dvfs();
> + ret = cpufreq_register_driver(&exynos_driver);
> + if (ret) {
> + pr_err("%s: failed to register cpufreq driver\n", __func__);
> + goto err_free_table;
> + }
> +
> + of_node_put(np);
> + dvfs_info->dvfs_enable = true;
> + pr_info("exynos5440 DVFS initialized.\n");
dev_info() ?
> + return 0;
> +
> +err_free_table:
> + opp_free_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table);
> +err_put_node:
> + of_node_put(np);
> + pr_err("%s: failed initialization\n", __func__);
Is this really needed ? This failure will be logged by the driver core
anyway.
> + return ret;
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordom
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
On Thu, Mar 07, 2013 at 04:28:00PM +0100, Sylwester Nawrocki wrote:
> On 03/07/2013 05:13 AM, Amit Daniel Kachhap wrote:
> > + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
> > + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
>
> devm_clk_get() return value needs to be checked with IS_ERR(),
> not IS_ERR_OR_NULL().
>
> > + pr_err("Failed to get cpu clock\n");
> > + ret = PTR_ERR(dvfs_info->cpu_clk);
> > + goto err_free_table;
Amit, to illustrate why this is wrong, consider this:
1. Set cpu_clk to NULL.
2. Realise IS_ERR_OR_NULL(NULL) is true.
3. What is the value of PTR_ERR(NULL) ?
4. What effect does that have when you jump to err_free_table ?
5. What value is returned from this function in that case ?
6. What does that return value mean to the driver core ?
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
On 7 March 2013 12:13, Amit Daniel Kachhap wrote:
> diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
> b/drivers/cpufreq/exynos5440-cpufreq.c
> +struct exynos_dvfs_data {
> + void __iomem *base;
> + struct resource *mem;
> + int irq;
> + struct clk *cpu_clk;
> + unsigned int cur_frequency;
> + unsigned int latency;
> + struct cpufreq_frequency_table *freq_table;
> + unsigned int freq_count;
> + struct device *dev;
> + bool dvfs_enable;
s/enable/enabled?
> +static int exynos_cpufreq_probe(struct platform_device *pdev)
> + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
> + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
s/IS_ERR_OR_NULL/IS_ERR
> + if (devm_request_irq(dvfs_info->dev, dvfs_info->irq,
> exynos_cpufreq_irq,
> + IRQF_TRIGGER_NONE, CPUFREQ_NAME, dvfs_info)) {
> + pr_err("Failed to register IRQ\n");
> + ret = -ENODEV;
use the value returned from this function rather than creating your
own.. true for all
other calls too, unless they return NULL on error.
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
Hi Sylwester,
Thanks for the review. Will update with your suggestion in the next version,
Thanks,
Amit D
On Thu, Mar 7, 2013 at 8:58 PM, Sylwester Nawrocki
wrote:
> On 03/07/2013 05:13 AM, Amit Daniel Kachhap wrote:
>> +/* Register definations */
>
> s/definations/definitions
ok
>
>> +#define XMU_DVFS_CTRL0x0060
>> +#define XMU_PMU_P0_7 0x0064
>> +#define XMU_C0_3_PSTATE 0x0090
>> +#define XMU_P_LIMIT 0x00A0
>> +#define XMU_P_STATUS 0x00A4
>> +#define XMU_PMUEVTEN 0x00D0
>> +#define XMU_PMUIRQEN 0x00D4
>> +#define XMU_PMUIRQ 0x00D8
>
> Keeping all hex numbers lower case might be a good idea.
Ok
>
>> +
>> +/* PMU mask and shift definations */
>> +#define P_VALUE_MASK 0x7
>> +
>> +#define XMU_DVFS_CTRL_EN_SHIFT 0
>> +
>> +#define P0_7_CPUCLKDEV_SHIFT 21
>> +#define P0_7_CPUCLKDEV_MASK 0x7
>> +#define P0_7_ATBCLKDEV_SHIFT 18
>> +#define P0_7_ATBCLKDEV_MASK 0x7
>> +#define P0_7_CSCLKDEV_SHIFT 15
>> +#define P0_7_CSCLKDEV_MASK 0x7
>> +#define P0_7_CPUEMA_SHIFT28
>> +#define P0_7_CPUEMA_MASK 0xf
>> +#define P0_7_L2EMA_SHIFT 24
>> +#define P0_7_L2EMA_MASK 0xf
> ...
>> +static int exynos_cpufreq_probe(struct platform_device *pdev)
>> +{
>> + int ret = -EINVAL;
>> + struct device_node *np;
>> + struct resource res;
>> +
>> + np = of_find_compatible_node(NULL, NULL,
>> "samsung,exynos5440-cpufreq");
>> + if (!np)
>> + return -ENODEV;
>> +
>> + dvfs_info = devm_kzalloc(&pdev->dev, sizeof(*dvfs_info), GFP_KERNEL);
>> + if (!dvfs_info) {
>> + ret = -ENOMEM;
>> + goto err_put_node;
>> + }
>> +
>> + dvfs_info->dev = &pdev->dev;
>> + dvfs_info->dev->of_node = np;
>> +
>> + ret = of_address_to_resource(np, 0, &res);
>> + if (ret)
>> + goto err_put_node;
>> +
>> + dvfs_info->base = devm_ioremap(dvfs_info->dev, res.start,
>
> There is a devm_ioremap_resource() function that has been introduced
> recently. It could simplify this code a bit and is preferred over
> devm_ioremap().
yes right. I somehow missed this api.
>
>> + resource_size(&res));
>> + if (!dvfs_info->base) {
>> + pr_err("No cpufreq memory map found\n");
>> + ret = -ENODEV;
>> + goto err_put_node;
>> + }
>> +
>> + dvfs_info->irq = irq_of_parse_and_map(np, 0);
>> + if (dvfs_info->irq == 0) {
>> + pr_err("No cpufreq irq found\n");
>
> Wouldn't dev_err() be more appropriate here ?
Ok
>
>> + ret = -ENODEV;
>> + goto err_put_node;
>> + }
>> +
>> + ret = of_init_opp_table(dvfs_info->dev);
>> + if (ret) {
>> + pr_err("failed to init OPP table: %d\n", ret);
>> + goto err_put_node;
>> + }
>> +
>> + ret = opp_init_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table);
>> + if (ret) {
>> + pr_err("failed to init cpufreq table: %d\n", ret);
>> + goto err_put_node;
>> + }
>> + dvfs_info->freq_count = opp_get_opp_count(dvfs_info->dev);
>> + exynos_sort_descend_freq_table();
>> +
>> + if (of_property_read_u32(np, "clock-latency", &dvfs_info->latency))
>> + dvfs_info->latency = DEF_TRANS_LATENCY;
>> +
>> + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
>> + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
>
> devm_clk_get() return value needs to be checked with IS_ERR(),
> not IS_ERR_OR_NULL().
ok
>
>> + pr_err("Failed to get cpu clock\n");
>> + ret = PTR_ERR(dvfs_info->cpu_clk);
>> + goto err_free_table;
>> + }
>> +
>> + dvfs_info->cur_frequency = clk_get_rate(dvfs_info->cpu_clk);
>> + if (!dvfs_info->cur_frequency) {
>> + pr_err("Failed to get clock rate\n");
>> + ret = -EINVAL;
>> + goto err_free_table;
>> + }
>> + dvfs_info->cur_frequency /= 1000;
>> +
>> + INIT_WORK(&dvfs_info->irq_work, exynos_cpufreq_work);
>> + if (devm_request_irq(dvfs_info->dev, dvfs_info->irq,
>> exynos_cpufreq_irq,
>> + IRQF_TRIGGER_NONE, CPUFREQ_NAME, dvfs_info)) {
>> + pr_err("Failed to register IRQ\n");
>> + ret = -ENODEV;
>> + goto err_free_table;
>> + }
>> +
>> + ret = init_div_table();
>> + if (ret) {
>> + pr_err("Failed to initialise div table\n");
>> + goto err_free_table;
>> + }
>> +
>> + exynos_enable_dvfs();
>> + ret = cpufreq_register_driver(&exynos_driver);
>> + if (ret) {
>> + pr_err("%s: failed to register cpufreq driver\n", __func__);
>> + goto err_free_table;
>> + }
>> +
>> + of_node_put(np);
>> + dvfs_info->dvfs_enable = true;
>> + pr_info("exynos5440 DVFS initialized.\n");
>
> dev_info() ?
>
>> + return 0;
>> +
>> +err_free_table:
>> + opp_free_cpufreq_table(dvfs_info
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
Hi Russell,
On Thu, Mar 7, 2013 at 9:07 PM, Russell King - ARM Linux
wrote:
> On Thu, Mar 07, 2013 at 04:28:00PM +0100, Sylwester Nawrocki wrote:
>> On 03/07/2013 05:13 AM, Amit Daniel Kachhap wrote:
>> > + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
>> > + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
>>
>> devm_clk_get() return value needs to be checked with IS_ERR(),
>> not IS_ERR_OR_NULL().
>>
>> > + pr_err("Failed to get cpu clock\n");
>> > + ret = PTR_ERR(dvfs_info->cpu_clk);
>> > + goto err_free_table;
>
> Amit, to illustrate why this is wrong, consider this:
> 1. Set cpu_clk to NULL.
> 2. Realise IS_ERR_OR_NULL(NULL) is true.
> 3. What is the value of PTR_ERR(NULL) ?
> 4. What effect does that have when you jump to err_free_table ?
> 5. What value is returned from this function in that case ?
> 6. What does that return value mean to the driver core ?
Thanks for the detailed explaination. Usage of IS_ERR_OR_NULL is not
suitable here.
Thanks,
Amit D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440
Hi Viresh,
On Sat, Mar 9, 2013 at 4:17 PM, Viresh Kumar wrote:
> On 7 March 2013 12:13, Amit Daniel Kachhap wrote:
>> diff --git a/drivers/cpufreq/exynos5440-cpufreq.c
>> b/drivers/cpufreq/exynos5440-cpufreq.c
>> +struct exynos_dvfs_data {
>> + void __iomem *base;
>> + struct resource *mem;
>> + int irq;
>> + struct clk *cpu_clk;
>> + unsigned int cur_frequency;
>> + unsigned int latency;
>> + struct cpufreq_frequency_table *freq_table;
>> + unsigned int freq_count;
>> + struct device *dev;
>> + bool dvfs_enable;
>
> s/enable/enabled?
Ok
>
>> +static int exynos_cpufreq_probe(struct platform_device *pdev)
>
>> + dvfs_info->cpu_clk = devm_clk_get(dvfs_info->dev, "armclk");
>> + if (IS_ERR_OR_NULL(dvfs_info->cpu_clk)) {
>
> s/IS_ERR_OR_NULL/IS_ERR
Ok.
>
>> + if (devm_request_irq(dvfs_info->dev, dvfs_info->irq,
>> exynos_cpufreq_irq,
>> + IRQF_TRIGGER_NONE, CPUFREQ_NAME, dvfs_info))
>> {
>> + pr_err("Failed to register IRQ\n");
>> + ret = -ENODEV;
>
> use the value returned from this function rather than creating your
> own.. true for all
> other calls too, unless they return NULL on error.
>
Will integrate your suggestions in the next version.
Thanks,
Amit D
>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
