Re: [PATCH RFC] PM/Devfreq: Add Exynos5-bus devfreq driver for Exynos5250.
From: MyungJoo Ham <myungjoo.ham@samsung.com>
Date: 2012-12-11 01:20:54
Also in:
lkml
On Fri, Nov 30, 2012 at 5:42 PM, Abhilash Kesavan [off-list ref] 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 <redacted> Cc: Jonghwan Choi <redacted> Cc: Kukjin Kim <redacted>
I've got a few general comments and questions on your patch.
--- This code is based on Jonghwan Choi's [off-list ref] 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.
quoted hunk ↗ jump to hunk
+ +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