Re: [1/9] thermal: exynos: Adapt to temperature emulation core thermal framework
Hi Eduardo,
Thanks for your review comments.
On Fri, Apr 12, 2013 at 1:03 AM, Eduardo Valentin
eduardo.valen...@ti.com wrote:
Hello Amit,
Couple of comments inline.
On 26-03-2013 07:33, Amit Daniel Kachhap wrote:
This removes the driver specific sysfs support of the temperature
emulation and uses the newly added core thermal framework for thermal
emulation. An exynos platform specific handler is added to support this.
In this patch, the exynos senor(tmu) related code and exynos framework
related (thermal zone, cooling devices) code are intentionally kept
separate.
So an emulated function pointer is passed from sensor to framework. This
is
beneficial in adding more sensor support using the same framework code
which is an ongoing work. The goal is to finally split them totally. Even
the existing read_temperature also follows the same execution method.
Acked-by: Kukjin Kim kgene@samsung.com
Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
---
Documentation/thermal/exynos_thermal_emulation |8 +-
drivers/thermal/Kconfig|9 --
drivers/thermal/exynos_thermal.c | 158
++--
3 files changed, 67 insertions(+), 108 deletions(-)
diff --git a/Documentation/thermal/exynos_thermal_emulation
b/Documentation/thermal/exynos_thermal_emulation
index b73bbfb..36a3e79 100644
--- a/Documentation/thermal/exynos_thermal_emulation
+++ b/Documentation/thermal/exynos_thermal_emulation
@@ -13,11 +13,11 @@ Thermal emulation mode supports software debug for
TMU's operation. User can set
manually with software code and TMU will read current temperature from
user value not from
sensor's value.
-Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support in
available.
-When it's enabled, sysfs node will be created under
-/sys/bus/platform/devices/'exynos device name'/ with name of 'emulation'.
+Enabling CONFIG_THERMAL_EMULATION option will make this support
available.
+When it's enabled, sysfs node will be created as
+/sys/devices/virtual/thermal/thermal_zone'zone id'/emul_temp.
-The sysfs node, 'emulation', will contain value 0 for the initial state.
When you input any
+The sysfs node, 'emul_node', will contain value 0 for the initial state.
When you input any
temperature you want to update to sysfs node, it automatically enable
emulation mode and
current temperature will be changed into it.
(Exynos also supports user changable delay time which would be used to
delay of
not part of this patch but:
s/changable/changeable
Ok
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index a764f16..da4c19e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -117,15 +117,6 @@ config EXYNOS_THERMAL
If you say yes here you get support for TMU (Thermal Management
Unit) on SAMSUNG EXYNOS series of SoC.
-config EXYNOS_THERMAL_EMUL
- bool EXYNOS TMU emulation mode support
- depends on EXYNOS_THERMAL
- help
- Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
- Enable this option will be make sysfs node in exynos thermal
platform
- device directory to support emulation mode. With emulation mode
sysfs
- node, you can manually input temperature to TMU for simulation
purpose.
-
config DOVE_THERMAL
tristate Temperature sensor on Marvell Dove SoCs
depends on ARCH_DOVE
diff --git a/drivers/thermal/exynos_thermal.c
b/drivers/thermal/exynos_thermal.c
index 46568c0..1cd7837 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -100,13 +100,13 @@
#define IDLE_INTERVAL 1
#define MCELSIUS 1000
-#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+#ifdef CONFIG_THERMAL_EMULATION
#define EXYNOS_EMUL_TIME 0x57F0
#define EXYNOS_EMUL_TIME_SHIFT16
#define EXYNOS_EMUL_DATA_SHIFT8
#define EXYNOS_EMUL_DATA_MASK 0xFF
#define EXYNOS_EMUL_ENABLE0x1
-#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
+#endif /* CONFIG_THERMAL_EMULATION */
As the above is only used in one single function, I suggest moving it to
same ifdef where the function belongs below. It reduces your ifdefery and
also makes your code cleaner.
Right makes sense.
/* CPU Zone information */
#define PANIC_ZONE 4
@@ -145,6 +145,7 @@ struct thermal_cooling_conf {
struct thermal_sensor_conf {
char name[SENSOR_NAME_LEN];
int (*read_temperature)(void *data);
+ int (*write_emul_temp)(void *drv_data, unsigned long temp);
I dont get why you need a private callback to do this, assuming you have
only one sensor_conf for exynos.
Actually It is done in this way to keep sensor related stuffs and
common thermal zone related parts. With this it is easy to split this
file in later patches.
struct thermal_trip_point_conf trip_data;
struct thermal_cooling_conf cooling_data;
Re: [1/9] thermal: exynos: Adapt to temperature emulation core thermal framework
Hello Amit,
Couple of comments inline.
On 26-03-2013 07:33, Amit Daniel Kachhap wrote:
This removes the driver specific sysfs support of the temperature
emulation and uses the newly added core thermal framework for thermal
emulation. An exynos platform specific handler is added to support this.
In this patch, the exynos senor(tmu) related code and exynos framework
related (thermal zone, cooling devices) code are intentionally kept separate.
So an emulated function pointer is passed from sensor to framework. This is
beneficial in adding more sensor support using the same framework code
which is an ongoing work. The goal is to finally split them totally. Even
the existing read_temperature also follows the same execution method.
Acked-by: Kukjin Kim kgene@samsung.com
Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
---
Documentation/thermal/exynos_thermal_emulation |8 +-
drivers/thermal/Kconfig|9 --
drivers/thermal/exynos_thermal.c | 158 ++--
3 files changed, 67 insertions(+), 108 deletions(-)
diff --git a/Documentation/thermal/exynos_thermal_emulation
b/Documentation/thermal/exynos_thermal_emulation
index b73bbfb..36a3e79 100644
--- a/Documentation/thermal/exynos_thermal_emulation
+++ b/Documentation/thermal/exynos_thermal_emulation
@@ -13,11 +13,11 @@ Thermal emulation mode supports software debug for TMU's
operation. User can set
manually with software code and TMU will read current temperature from user
value not from
sensor's value.
-Enabling CONFIG_EXYNOS_THERMAL_EMUL option will make this support in available.
-When it's enabled, sysfs node will be created under
-/sys/bus/platform/devices/'exynos device name'/ with name of 'emulation'.
+Enabling CONFIG_THERMAL_EMULATION option will make this support available.
+When it's enabled, sysfs node will be created as
+/sys/devices/virtual/thermal/thermal_zone'zone id'/emul_temp.
-The sysfs node, 'emulation', will contain value 0 for the initial state. When
you input any
+The sysfs node, 'emul_node', will contain value 0 for the initial state. When
you input any
temperature you want to update to sysfs node, it automatically enable
emulation mode and
current temperature will be changed into it.
(Exynos also supports user changable delay time which would be used to delay
of
not part of this patch but:
s/changable/changeable
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index a764f16..da4c19e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -117,15 +117,6 @@ config EXYNOS_THERMAL
If you say yes here you get support for TMU (Thermal Management
Unit) on SAMSUNG EXYNOS series of SoC.
-config EXYNOS_THERMAL_EMUL
- bool EXYNOS TMU emulation mode support
- depends on EXYNOS_THERMAL
- help
- Exynos 4412 and 4414 and 5 series has emulation mode on TMU.
- Enable this option will be make sysfs node in exynos thermal platform
- device directory to support emulation mode. With emulation mode sysfs
- node, you can manually input temperature to TMU for simulation
purpose.
-
config DOVE_THERMAL
tristate Temperature sensor on Marvell Dove SoCs
depends on ARCH_DOVE
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 46568c0..1cd7837 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -100,13 +100,13 @@
#define IDLE_INTERVAL 1
#define MCELSIUS 1000
-#ifdef CONFIG_EXYNOS_THERMAL_EMUL
+#ifdef CONFIG_THERMAL_EMULATION
#define EXYNOS_EMUL_TIME 0x57F0
#define EXYNOS_EMUL_TIME_SHIFT16
#define EXYNOS_EMUL_DATA_SHIFT8
#define EXYNOS_EMUL_DATA_MASK 0xFF
#define EXYNOS_EMUL_ENABLE0x1
-#endif /* CONFIG_EXYNOS_THERMAL_EMUL */
+#endif /* CONFIG_THERMAL_EMULATION */
As the above is only used in one single function, I suggest moving it to
same ifdef where the function belongs below. It reduces your ifdefery
and also makes your code cleaner.
/* CPU Zone information */
#define PANIC_ZONE 4
@@ -145,6 +145,7 @@ struct thermal_cooling_conf {
struct thermal_sensor_conf {
char name[SENSOR_NAME_LEN];
int (*read_temperature)(void *data);
+ int (*write_emul_temp)(void *drv_data, unsigned long temp);
I dont get why you need a private callback to do this, assuming you
have only one sensor_conf for exynos.
struct thermal_trip_point_conf trip_data;
struct thermal_cooling_conf cooling_data;
void *private_data;
@@ -369,6 +370,23 @@ static int exynos_get_temp(struct thermal_zone_device
*thermal,
return 0;
}
+/* Get temperature callback functions for thermal zone */
+static int exynos_set_emul_temp(struct thermal_zone_device *thermal,
+ unsigned long temp)
+{
+ void *data;
+ int ret = -EINVAL;
In case you
