On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan <a.kesa...@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.kesa...@samsung.com>
> Cc: Jonghwan Choi <jhbird.c...@samsung.com>
> Cc: Kukjin Kim <kgene....@samsung.com>
I've got a few general comments and questions on your patch.
> ---
> This code is based on Jonghwan Choi's <jhbird.c...@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
--
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/
