lkml.org 
[lkml]   [2012]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
From
On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan <a.kesavan@samsung.com> 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 <a.kesavan@samsung.com>
> Cc: Jonghwan Choi <jhbird.choi@samsung.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>

I've got a few general comments and questions on your patch.

> ---
> This code is based on Jonghwan Choi's <jhbird.choi@samsung.com> 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


\
 
 \ /
  Last update: 2012-12-11 02:41    [W:0.070 / U:1.732 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site