linux-kernel - Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ0PZbTTDdAXJ378R7CDO2=spx7WF_RpztG5ch7iFZWx-v3N2Q@mail.gmail.com>
Date:	Tue, 11 Dec 2012 10:20:51 +0900
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Abhilash Kesavan <a.kesavan@...sung.com>
Cc:	kyungmin.park@...sung.com, rjw@...k.pl,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
	kgene.kim@...sung.com, jhbird.choi@...sung.com
Subject: Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.

On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan <a.kesavan@...sung.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@...sung.com>
> Cc: Jonghwan Choi <jhbird.choi@...sung.com>
> Cc: Kukjin Kim <kgene.kim@...sung.com>

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

> ---
> This code is based on Jonghwan Choi's <jhbird.choi@...sung.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/