Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
Hi, On Tue, Dec 11, 2012 at 10:20:51AM +0900, MyungJoo Ham wrote: > On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan > wrote: > > drivers/devfreq/Kconfig| 10 + > > drivers/devfreq/Makefile |1 + > > drivers/devfreq/exynos5_bus.c | 595 > > > > drivers/devfreq/exynos5_ppmu.c | 395 ++ > > drivers/devfreq/exynos5_ppmu.h | 26 ++ > > drivers/devfreq/exynos_ppmu.c | 56 > > drivers/devfreq/exynos_ppmu.h | 79 ++ > > 7 files changed, 1162 insertions(+), 0 deletions(-) > > create mode 100644 drivers/devfreq/exynos5_bus.c > > create mode 100644 drivers/devfreq/exynos5_ppmu.c > > create mode 100644 drivers/devfreq/exynos5_ppmu.h > > create mode 100644 drivers/devfreq/exynos_ppmu.c > > create mode 100644 drivers/devfreq/exynos_ppmu.h > > I understand that Exynos PPMU drivers seem not to be used (at least in > mainline Linux) widely and it'd be convinent for a bus driver to have > ppmu driver located in the same source directory. > > However, I don't feel very comfortable to have ppmu drivers explicitly > landing in devfreq directory. Would it be possible to place them > somewhere else? (in drivers/misc, arch/arm/mach-exynos, or somewhere > appropriate?) If PPMU drivers really have nowhere to relocate, they > may be located along with its sole user (exynos5_bus.c) anyway. Why can't they be in drivers/busfreq? Create a subdirectory for platform-specific subdrivers if needed, but they definitiely do NOT belong in drivers/misc, and there seems to be little reason to have them in arch/arm. -Olof -- 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/
Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan wrote:
> Exynos5-bus device devfreq driver monitors PPMU counters and
> adjusts operating frequencies and voltages with OPP. ASV should
> be used to provide appropriate voltages as per the speed group
> of the SoC rather than using a constant 1.025V.
>
> Signed-off-by: Abhilash Kesavan
> Cc: Jonghwan Choi
> Cc: Kukjin Kim
I've got a few general comments and questions on your patch.
> ---
> This code is based on Jonghwan Choi's devfreq work
> for Exynos5250. This requires corresponding machine specific changes which
> will be posted once the driver is reviewed.
>
> drivers/devfreq/Kconfig| 10 +
> drivers/devfreq/Makefile |1 +
> drivers/devfreq/exynos5_bus.c | 595
>
> drivers/devfreq/exynos5_ppmu.c | 395 ++
> drivers/devfreq/exynos5_ppmu.h | 26 ++
> drivers/devfreq/exynos_ppmu.c | 56
> drivers/devfreq/exynos_ppmu.h | 79 ++
> 7 files changed, 1162 insertions(+), 0 deletions(-)
> create mode 100644 drivers/devfreq/exynos5_bus.c
> create mode 100644 drivers/devfreq/exynos5_ppmu.c
> create mode 100644 drivers/devfreq/exynos5_ppmu.h
> create mode 100644 drivers/devfreq/exynos_ppmu.c
> create mode 100644 drivers/devfreq/exynos_ppmu.h
I understand that Exynos PPMU drivers seem not to be used (at least in
mainline Linux) widely and it'd be convinent for a bus driver to have
ppmu driver located in the same source directory.
However, I don't feel very comfortable to have ppmu drivers explicitly
landing in devfreq directory. Would it be possible to place them
somewhere else? (in drivers/misc, arch/arm/mach-exynos, or somewhere
appropriate?) If PPMU drivers really have nowhere to relocate, they
may be located along with its sole user (exynos5_bus.c) anyway.
> +
> +struct exynos5_bus_int_handle {
> + struct list_head node;
> + struct delayed_work work;
> + bool boost;
> + bool poll;
> + unsigned long min;
> +};
It appears that "boost" is something may be handled by per-dev QoS.
It looks like that you are reimplementing the pm-qos infrastructure in
the driver.
Could you please implement this w/ per-dev QoS?
Or explain why this is required instead of per-dev PM-QoS?
Cheers,
MyungJoo
--
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
--
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/
Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
Cc'ing missed out recipients -- Forwarded message -- From: Abhilash Kesavan Date: Tue, Dec 4, 2012 at 9:48 PM Subject: Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250. To: linux...@vger.kernel.org Jonghwan Choi samsung.com> writes: > > > Hi Abhilash Kesavan. Hi, > > I compiled in 3.7-rc8 > I got a compile error & warning. > > Compile error. > > CC drivers/devfreq/exynos5_ppmu.o > drivers/devfreq/exynos5_ppmu.c:56:14: error: 'S5P_VA_PPMU_DDR_C' undeclared > here (not in a function) > drivers/devfreq/exynos5_ppmu.c:59:14: error: 'S5P_VA_PPMU_DDR_R1' undeclared > here (not in a function) > drivers/devfreq/exynos5_ppmu.c:62:13: error: 'S5P_VA_PPMU_DDR_L' undeclared > here (not in a function) > drivers/devfreq/exynos5_ppmu.c:65:13: error: 'S5P_VA_PPMU_RIGHT' undeclared > here (not in a function) > drivers/devfreq/exynos5_ppmu.c:68:14: error: 'S5P_VA_PPMU_CPU' undeclared > here (not in a function) > make[2]: *** [drivers/devfreq/exynos5_ppmu.o] Error 1 I have submitted only the driver changes. As mentioned in the commit message I will post the arch side changes after the drivers gets a complete review. However, if you want to test the driver I will post it tommorrow. [...] > drivers/devfreq/exynos5_bus.c: In function 'exynos5_busfreq_int_probe': > drivers/devfreq/exynos5_bus.c:388:9: warning: passing argument 3 of > 'devfreq_add_device' from incompatible pointer type [enabled by default] > include/linux/devfreq.h:170:24: note: expected 'struct devfreq_governor > const *' but argument is of type 'char *' [...] > Need change? These changes are based on git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git : for-rafael devfreq_add_device prototype has recently changed and that might be the cause of the compilation error. Can you please try on this tree. > How to change the INT clock? I think you have to change > arch/arm/mach-exynos/clock-exynos5250.c > (If you want to control via clock framework.) [...] > exynos5_int_clk.ops = &exynos5_clk_int_ops; This is part of the arch side patch that has not been posted. Regards, Abhilash -- To unsubscribe from this list: send the line "unsubscribe linux-pm" 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-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/
RE: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
Hi Abhilash Kesavan.
I compiled in 3.7-rc8
I got a compile error & warning.
Compile error.
CC drivers/devfreq/exynos5_ppmu.o
drivers/devfreq/exynos5_ppmu.c:56:14: error: 'S5P_VA_PPMU_DDR_C' undeclared
here (not in a function)
drivers/devfreq/exynos5_ppmu.c:59:14: error: 'S5P_VA_PPMU_DDR_R1' undeclared
here (not in a function)
drivers/devfreq/exynos5_ppmu.c:62:13: error: 'S5P_VA_PPMU_DDR_L' undeclared
here (not in a function)
drivers/devfreq/exynos5_ppmu.c:65:13: error: 'S5P_VA_PPMU_RIGHT' undeclared
here (not in a function)
drivers/devfreq/exynos5_ppmu.c:68:14: error: 'S5P_VA_PPMU_CPU' undeclared
here (not in a function)
make[2]: *** [drivers/devfreq/exynos5_ppmu.o] Error 1
> + data->devfreq = devfreq_add_device(dev,
> &exynos5_devfreq_int_profile,
> +"simple_ondemand",
> +&exynos5_int_ondemand_data);
drivers/devfreq/exynos5_bus.c: In function 'exynos5_busfreq_int_probe':
drivers/devfreq/exynos5_bus.c:388:9: warning: passing argument 3 of
'devfreq_add_device' from incompatible pointer type [enabled by default]
include/linux/devfreq.h:170:24: note: expected 'struct devfreq_governor
const *' but argument is of type 'char *'
include/linux/devfreq.h
extern struct devfreq *devfreq_add_device(struct device *dev,
struct devfreq_dev_profile *profile,
const struct devfreq_governor *governor,
void *data);
Need change?
> +
> + if (IS_ERR(data->devfreq)) {
> + err = PTR_ERR(data->devfreq);
> + goto err_devfreq_add;
> + }
> +static int exynos5_busfreq_int_target(struct device *dev, unsigned long
> *_freq,
> + u32 flags)
> +{
> + int err = 0;
> + struct platform_device *pdev = container_of(dev, struct
> platform_device,
...
> +
> + err = clk_set_rate(data->int_clk, freq * 1000);
How to change the INT clock? I think you have to change
arch/arm/mach-exynos/clock-exynos5250.c
(If you want to control via clock framework.)
Like this.
static struct clk exynos5_int_clk = {
.name = "int_clk",
.id = -1,
};
static struct clk_ops exynos5_clk_int_ops = {
.get_rate = exynos5_clk_int_get_rate,
.set_rate = exynos5_clk_int_set_rate
};
exynos5_int_clk.ops = &exynos5_clk_int_ops;
> +
> + if (err)
> + goto out;
> +
Thanks~
Best Regards.
> -Original Message-
> From: Abhilash Kesavan [mailto:a.kesa...@samsung.com]
> Sent: Friday, November 30, 2012 5:43 PM
> To: myungjoo....@samsung.com; kyungmin.p...@samsung.com; r...@sisk.pl;
> linux-kernel@vger.kernel.org; linux...@vger.kernel.org
> Cc: kgene@samsung.com; jhbird.c...@samsung.com
> Subject: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for
> Exynos5250.
>
> Exynos5-bus device devfreq driver monitors PPMU counters and
> adjusts operating frequencies and voltages with OPP. ASV should
> be used to provide appropriate voltages as per the speed group
> of the SoC rather than using a constant 1.025V.
>
> Signed-off-by: Abhilash Kesavan
> Cc: Jonghwan Choi
> Cc: Kukjin Kim
> ---
> This code is based on Jonghwan Choi's devfreq
> work
> for Exynos5250. This requires corresponding machine specific changes which
> will be posted once the driver is reviewed.
>
> drivers/devfreq/Kconfig| 10 +
> drivers/devfreq/Makefile |1 +
> drivers/devfreq/exynos5_bus.c | 595
>
> drivers/devfreq/exynos5_ppmu.c | 395 ++
> drivers/devfreq/exynos5_ppmu.h | 26 ++
> drivers/devfreq/exynos_ppmu.c | 56
> drivers/devfreq/exynos_ppmu.h | 79 ++
> 7 files changed, 1162 insertions(+), 0 deletions(-)
> create mode 100644 drivers/devfreq/exynos5_bus.c
> create mode 100644 drivers/devfreq/exynos5_ppmu.c
> create mode 100644 drivers/devfreq/exynos5_ppmu.h
> create mode 100644 drivers/devfreq/exynos_ppmu.c
> create mode 100644 drivers/devfreq/exynos_ppmu.h
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 0f079be..1560d0d 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -78,4 +78,14 @@ config ARM_EXYNOS4_BUS_DEVFREQ
> To operate with optimal voltages, ASV support is required
> (CONFIG_EXYNOS_ASV).
>
> +config ARM_EXYNOS5_BUS_DEVFREQ
> + bool "ARM Exynos5250 Bus DEVFREQ Driver"
> + depends on SOC_EXYNOS5250
> + select ARCH_HAS_OPP
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + help
>
[PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
Exynos5-bus device devfreq driver monitors PPMU counters and
adjusts operating frequencies and voltages with OPP. ASV should
be used to provide appropriate voltages as per the speed group
of the SoC rather than using a constant 1.025V.
Signed-off-by: Abhilash Kesavan
Cc: Jonghwan Choi
Cc: Kukjin Kim
---
This code is based on Jonghwan Choi's devfreq work
for Exynos5250. This requires corresponding machine specific changes which
will be posted once the driver is reviewed.
drivers/devfreq/Kconfig| 10 +
drivers/devfreq/Makefile |1 +
drivers/devfreq/exynos5_bus.c | 595
drivers/devfreq/exynos5_ppmu.c | 395 ++
drivers/devfreq/exynos5_ppmu.h | 26 ++
drivers/devfreq/exynos_ppmu.c | 56
drivers/devfreq/exynos_ppmu.h | 79 ++
7 files changed, 1162 insertions(+), 0 deletions(-)
create mode 100644 drivers/devfreq/exynos5_bus.c
create mode 100644 drivers/devfreq/exynos5_ppmu.c
create mode 100644 drivers/devfreq/exynos5_ppmu.h
create mode 100644 drivers/devfreq/exynos_ppmu.c
create mode 100644 drivers/devfreq/exynos_ppmu.h
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 0f079be..1560d0d 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -78,4 +78,14 @@ config ARM_EXYNOS4_BUS_DEVFREQ
To operate with optimal voltages, ASV support is required
(CONFIG_EXYNOS_ASV).
+config ARM_EXYNOS5_BUS_DEVFREQ
+ bool "ARM Exynos5250 Bus DEVFREQ Driver"
+ depends on SOC_EXYNOS5250
+ select ARCH_HAS_OPP
+ select DEVFREQ_GOV_SIMPLE_ONDEMAND
+ help
+ This adds the DEVFREQ driver for Exynos5250 bus interface (vdd_int).
+ It reads PPMU counters of memory controllers and adjusts the
+ operating frequencies and voltages with OPP support.
+
endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 8c46423..6259aa4 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
# DEVFREQ Drivers
obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ) += exynos4_bus.o
+obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ) += exynos_ppmu.o exynos5_ppmu.o
exynos5_bus.o
diff --git a/drivers/devfreq/exynos5_bus.c b/drivers/devfreq/exynos5_bus.c
new file mode 100644
index 000..8ced376
--- /dev/null
+++ b/drivers/devfreq/exynos5_bus.c
@@ -0,0 +1,595 @@
+/*
+ * Copyright (c) 2012 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com/
+ *
+ * EXYNOS5 INT clock frequency scaling support using DEVFREQ framework
+ * Based on work done by Jonghwan Choi
+ * Support for only EXYNOS5250 is present.
+ *
+ * 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.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "exynos_ppmu.h"
+#include "exynos5_ppmu.h"
+#include "governor.h"
+
+#define MAX_SAFEVOLT 110 /* 1.10V */
+/* Assume that the bus is saturated if the utilization is 25% */
+#define INT_BUS_SATURATION_RATIO 25
+#define EXYNOS5_BUS_INT_POLL_TIME msecs_to_jiffies(100)
+
+enum int_level_idx {
+ LV_0,
+ LV_1,
+ LV_2,
+ LV_3,
+ LV_4,
+ _LV_END
+};
+
+struct busfreq_data_int {
+ struct device *dev;
+ struct devfreq *devfreq;
+ bool disabled;
+ struct regulator *vdd_int;
+ unsigned long curr_freq;
+ unsigned long curr_volt;
+ unsigned long suspend_freq;
+ struct mutex lock;
+ struct clk *int_clk;
+ struct exynos5_ppmu_handle *ppmu;
+ struct delayed_work work;
+ int busy;
+};
+
+struct exynos5_bus_int_handle {
+ struct list_head node;
+ struct delayed_work work;
+ bool boost;
+ bool poll;
+ unsigned long min;
+};
+
+struct int_bus_opp_table {
+ unsigned int idx;
+ unsigned long clk;
+ unsigned long volt;
+};
+
+static struct int_bus_opp_table exynos5_int_opp_table[] = {
+ {LV_0, 266000, 1025000},
+ {LV_1, 20, 1025000},
+ {LV_2, 16, 1025000},
+ {LV_3, 133000, 1025000},
+ {LV_4, 10, 1025000},
+ {0, 0, 0},
+};
+
+static struct busfreq_data_int *exynos5_bus_int_data;
+static DEFINE_MUTEX(exynos5_bus_int_data_lock);
+static LIST_HEAD(exynos5_bus_int_requests);
+static DEFINE_MUTEX(exynos5_bus_int_requests_lock);
+
+static int exynos5_int_setvolt(struct busfreq_data_int *data,
+ unsigned long volt)
+{
+ return regulator_set_voltage(data->vdd_int, volt, MAX_SAFEVOLT);
+}
+
+static int exynos5_busfreq_int_target(struct device *dev, unsigned long *_freq,
+ u32 flags)
+{
+ int err = 0;
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+
