Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
On 16 April 2012 07:37, Zhang Rui rui.zh...@intel.com wrote:
On 三, 2012-04-11 at 18:17 +0530, Amit Kachhap wrote:
Hi Rui,
Thanks for looking into the patches.
On 10 April 2012 06:28, Zhang Rui rui.zh...@intel.com wrote:
Hi, Amit,
On 三, 2012-04-04 at 10:02 +0530, Amit Kachhap wrote:
Hi Len/Rui,
Any comment or feedback from your side about the status of this patch?
Is it merge-able or major re-work is needed? I have fixed most of the
comments in this patchset and currently working on some of the minor
comments received and will submit them shortly.
Sorry for the late response.
First of all, it makes sense to me to introduce the generic cpufrq
cooling implementation.
ok thanks
But I still have some questions.
I think the key reason why THERMAL_TRIP_STATE_INSTANCE is introduced is
that the MONIROR_ZONE and WARN_ZONE on exynos4 can not fit into the
current passive handling in the generic thermal layer well, right?
e.g. there is no tc1/tc2 on exynos4.
If yes, is it possible that we can enhance the passive cooling to
support the generic processor cooling?
say, introduce another way to throttle the processor in
thermal_zone_device_passive when tc1 and tc2 are not available?
I agree that this new trip type code can be moved into passive trip
type when tc1 and tc2 are 0. but this is special type of cooling
devices behaviour where only instances of the same cooling device is
binded to a trip point. The order of mapping is the only
differentiating criteria and there are some checks used to implement
this like
1) The trip points should be in ascending order.(This is missing in my
original patch, so I added below)
2) The set_cur_state has to be called for the exact temp range so
get_cur_state(state) and set_cur_state(state ++/state--) logic is not
possible.
3) set_cur_state is called as set_cur_state(cdev_instance).
Do you really need two THERMAL_TRIP_STATE_INSTANCE trip points?
Sorry for late reply as I was off for vacation.
Yes we need 2 trip points of type THERMAL_TRIP_STATE_INSTANCE as we
need different cooling for these 2 zones. Anyways Do you feel that
these whole/partial patch series(cpufreq cooling api's, new trip type
etc) is ack-able or some modification is needed?
I'm not sure if my understanding is right, but IMO, we can have one
THERMAL_TRIP_STATE_INSTANCE only, for both MONIROR_ZONE and WARN_ZONE,
and the trip temperature equals MONIROR_ZONE.
The cpufreq cooling device will work in the same way, no?
thanks,
rui
There is a chance that people might confuse that these checks are
applicable for passive trip types also which is not the case here.
@@ -1187,6 +1228,21 @@ struct thermal_zone_device
*thermal_zone_device_register(char *type,
tz-ops-get_trip_type(tz, count, trip_type);
if (trip_type == THERMAL_TRIP_PASSIVE)
passive = 1;
+ /*
+ * For THERMAL_TRIP_STATE_INSTANCE trips, thermal zone should
+ * be in ascending order.
+ */
+ if (trip_type == THERMAL_TRIP_STATE_INSTANCE) {
+ tz-ops-get_trip_temp(tz, count, trip_temp);
+ if (first_trip_temp == 0)
+ first_trip_temp = trip_temp;
+ else if (first_trip_temp trip_temp)
+ first_trip_temp = trip_temp;
+ else if (first_trip_temp trip_temp) {
+ pr_warn(Zone trip points should be in
ascending order\n);
+ goto unregister;
+ }
+ }
}
if (!passive)
Anyway there is other alternative where this new trip type is not
needed and I can just use the existing trip type THERMAL_TRIP_ACTIVE
and create 2 separate cooling devices for MONITOR_ZONE and WARN_ZONE.
I had thought to make this generic and just to manage with 1 cooling
device.
What is your view?
Thanks,
Amit Daniel
thanks,
rui
Regards,
Amit Daniel
On 19 March 2012 11:47, Amit Daniel Kachhap amit.kach...@linaro.org
wrote:
Changes since V1:
*Moved the sensor driver to driver/thermal folder from driver/hwmon
folder
as suggested by Mark Brown and Guenter Roeck
*Added notifier support to notify the registered drivers of any cpu
cooling
action. The driver can modify the default cooling behaviour(eg set
different
max clip frequency).
*The percentage based frequency replaced with absolute clipped
frequency.
*Some more conditional checks when setting max frequency.
*Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to
THERMAL_TRIP_STATE_INSTANCE
*Many review comments from R, Durgadoss durgados...@intel.com and
eduardo.valen...@ti.com implemented.
*Removed cooling stats through debugfs patch
*The V1 based can be found here,
Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
On 三, 2012-04-11 at 18:17 +0530, Amit Kachhap wrote:
Hi Rui,
Thanks for looking into the patches.
On 10 April 2012 06:28, Zhang Rui rui.zh...@intel.com wrote:
Hi, Amit,
On 三, 2012-04-04 at 10:02 +0530, Amit Kachhap wrote:
Hi Len/Rui,
Any comment or feedback from your side about the status of this patch?
Is it merge-able or major re-work is needed? I have fixed most of the
comments in this patchset and currently working on some of the minor
comments received and will submit them shortly.
Sorry for the late response.
First of all, it makes sense to me to introduce the generic cpufrq
cooling implementation.
ok thanks
But I still have some questions.
I think the key reason why THERMAL_TRIP_STATE_INSTANCE is introduced is
that the MONIROR_ZONE and WARN_ZONE on exynos4 can not fit into the
current passive handling in the generic thermal layer well, right?
e.g. there is no tc1/tc2 on exynos4.
If yes, is it possible that we can enhance the passive cooling to
support the generic processor cooling?
say, introduce another way to throttle the processor in
thermal_zone_device_passive when tc1 and tc2 are not available?
I agree that this new trip type code can be moved into passive trip
type when tc1 and tc2 are 0. but this is special type of cooling
devices behaviour where only instances of the same cooling device is
binded to a trip point. The order of mapping is the only
differentiating criteria and there are some checks used to implement
this like
1) The trip points should be in ascending order.(This is missing in my
original patch, so I added below)
2) The set_cur_state has to be called for the exact temp range so
get_cur_state(state) and set_cur_state(state ++/state--) logic is not
possible.
3) set_cur_state is called as set_cur_state(cdev_instance).
Do you really need two THERMAL_TRIP_STATE_INSTANCE trip points?
I'm not sure if my understanding is right, but IMO, we can have one
THERMAL_TRIP_STATE_INSTANCE only, for both MONIROR_ZONE and WARN_ZONE,
and the trip temperature equals MONIROR_ZONE.
The cpufreq cooling device will work in the same way, no?
thanks,
rui
There is a chance that people might confuse that these checks are
applicable for passive trip types also which is not the case here.
@@ -1187,6 +1228,21 @@ struct thermal_zone_device
*thermal_zone_device_register(char *type,
tz-ops-get_trip_type(tz, count, trip_type);
if (trip_type == THERMAL_TRIP_PASSIVE)
passive = 1;
+ /*
+* For THERMAL_TRIP_STATE_INSTANCE trips, thermal zone should
+* be in ascending order.
+ */
+ if (trip_type == THERMAL_TRIP_STATE_INSTANCE) {
+ tz-ops-get_trip_temp(tz, count, trip_temp);
+ if (first_trip_temp == 0)
+ first_trip_temp = trip_temp;
+ else if (first_trip_temp trip_temp)
+ first_trip_temp = trip_temp;
+ else if (first_trip_temp trip_temp) {
+ pr_warn(Zone trip points should be in
ascending order\n);
+ goto unregister;
+ }
+ }
}
if (!passive)
Anyway there is other alternative where this new trip type is not
needed and I can just use the existing trip type THERMAL_TRIP_ACTIVE
and create 2 separate cooling devices for MONITOR_ZONE and WARN_ZONE.
I had thought to make this generic and just to manage with 1 cooling
device.
What is your view?
Thanks,
Amit Daniel
thanks,
rui
Regards,
Amit Daniel
On 19 March 2012 11:47, Amit Daniel Kachhap amit.kach...@linaro.org
wrote:
Changes since V1:
*Moved the sensor driver to driver/thermal folder from driver/hwmon
folder
as suggested by Mark Brown and Guenter Roeck
*Added notifier support to notify the registered drivers of any cpu
cooling
action. The driver can modify the default cooling behaviour(eg set
different
max clip frequency).
*The percentage based frequency replaced with absolute clipped frequency.
*Some more conditional checks when setting max frequency.
*Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to
THERMAL_TRIP_STATE_INSTANCE
*Many review comments from R, Durgadoss durgados...@intel.com and
eduardo.valen...@ti.com implemented.
*Removed cooling stats through debugfs patch
*The V1 based can be found here,
https://lkml.org/lkml/2012/2/22/123
http://lkml.org/lkml/2012/3/3/32
Changes since RFC:
*Changed the cpu cooling registration/unregistration API's to instance
based
*Changed the STATE_ACTIVE trip type to pass correct instance id
*Adding support to restore back the policy-max_freq after doing
frequency
clipping.
*Moved the trip cooling stats from sysfs
Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
Hi, Amit, On 三, 2012-04-04 at 10:02 +0530, Amit Kachhap wrote: Hi Len/Rui, Any comment or feedback from your side about the status of this patch? Is it merge-able or major re-work is needed? I have fixed most of the comments in this patchset and currently working on some of the minor comments received and will submit them shortly. Sorry for the late response. First of all, it makes sense to me to introduce the generic cpufrq cooling implementation. But I still have some questions. I think the key reason why THERMAL_TRIP_STATE_INSTANCE is introduced is that the MONIROR_ZONE and WARN_ZONE on exynos4 can not fit into the current passive handling in the generic thermal layer well, right? e.g. there is no tc1/tc2 on exynos4. If yes, is it possible that we can enhance the passive cooling to support the generic processor cooling? say, introduce another way to throttle the processor in thermal_zone_device_passive when tc1 and tc2 are not available? thanks, rui Regards, Amit Daniel On 19 March 2012 11:47, Amit Daniel Kachhap amit.kach...@linaro.org wrote: Changes since V1: *Moved the sensor driver to driver/thermal folder from driver/hwmon folder as suggested by Mark Brown and Guenter Roeck *Added notifier support to notify the registered drivers of any cpu cooling action. The driver can modify the default cooling behaviour(eg set different max clip frequency). *The percentage based frequency replaced with absolute clipped frequency. *Some more conditional checks when setting max frequency. *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to THERMAL_TRIP_STATE_INSTANCE *Many review comments from R, Durgadoss durgados...@intel.com and eduardo.valen...@ti.com implemented. *Removed cooling stats through debugfs patch *The V1 based can be found here, https://lkml.org/lkml/2012/2/22/123 http://lkml.org/lkml/2012/3/3/32 Changes since RFC: *Changed the cpu cooling registration/unregistration API's to instance based *Changed the STATE_ACTIVE trip type to pass correct instance id *Adding support to restore back the policy-max_freq after doing frequency clipping. *Moved the trip cooling stats from sysfs node to debugfs node as suggested by Greg KH g...@kroah.com *Incorporated several review comments from eduardo.valen...@ti.com *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd as discussed with Guenter Roeck guenter.ro...@ericsson.com and Donggeun Kim dg77@samsung.com (https://lkml.org/lkml/2012/1/5/7) *Some changes according to the changes in common cpu cooling APIs *The RFC based patches can be found here, https://lkml.org/lkml/2011/12/13/186 https://lkml.org/lkml/2011/12/21/169 Brief Description: 1) The generic cooling devices code is placed inside driver/thermal/* as placing inside acpi folder will need un-necessary enabling of acpi code. This codes is architecture independent. 2) This patchset adds a new trip type THERMAL_TRIP_STATE_INSTANCE which passes cooling device instance number and may be helpful for cpufreq cooling devices to take the correct cooling action. This trip type avoids the temperature comparision check again inside the cooling handler. 3) This patchset adds generic cpu cooling low level implementation through frequency clipping and cpu hotplug. In future, other cpu related cooling devices may be added here. An ACPI version of this already exists (drivers/acpi/processor_thermal.c). But this will be useful for platforms like ARM using the generic thermal interface along with the generic cpu cooling devices. The cooling device registration API's return cooling device pointers which can be easily binded with the thermal zone trip points. The important APIs exposed are, a)struct thermal_cooling_device *cpufreq_cooling_register( struct freq_clip_table *tab_ptr, unsigned int tab_size, const struct cpumask *mask_val) b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) 4) Samsung exynos platform thermal implementation is done using the generic cpu cooling APIs and the new trip type. The temperature sensor driver present in the hwmon folder(registered as hwmon driver) is moved to thermal folder and registered as a thermal driver. All this patchset is based on Kernel version 3.3-rc7 A simple data/control flow diagrams is shown below, Core Linux thermal - Exynos thermal interface - Temperature Sensor | | \|/| Cpufreq cooling device --- Amit Daniel Kachhap (6): thermal: Add a new trip type to use cooling device instance number thermal: Add generic cpufreq cooling implementation thermal: Add generic cpuhotplug cooling implementation hwmon: exynos4: Move thermal sensor driver to driver/thermal directory thermal: exynos4: Register the tmu sensor
Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
Hi Len/Rui, Any comment or feedback from your side about the status of this patch? Is it merge-able or major re-work is needed? I have fixed most of the comments in this patchset and currently working on some of the minor comments received and will submit them shortly. Regards, Amit Daniel On 19 March 2012 11:47, Amit Daniel Kachhap amit.kach...@linaro.org wrote: Changes since V1: *Moved the sensor driver to driver/thermal folder from driver/hwmon folder as suggested by Mark Brown and Guenter Roeck *Added notifier support to notify the registered drivers of any cpu cooling action. The driver can modify the default cooling behaviour(eg set different max clip frequency). *The percentage based frequency replaced with absolute clipped frequency. *Some more conditional checks when setting max frequency. *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to THERMAL_TRIP_STATE_INSTANCE *Many review comments from R, Durgadoss durgados...@intel.com and eduardo.valen...@ti.com implemented. *Removed cooling stats through debugfs patch *The V1 based can be found here, https://lkml.org/lkml/2012/2/22/123 http://lkml.org/lkml/2012/3/3/32 Changes since RFC: *Changed the cpu cooling registration/unregistration API's to instance based *Changed the STATE_ACTIVE trip type to pass correct instance id *Adding support to restore back the policy-max_freq after doing frequency clipping. *Moved the trip cooling stats from sysfs node to debugfs node as suggested by Greg KH g...@kroah.com *Incorporated several review comments from eduardo.valen...@ti.com *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd as discussed with Guenter Roeck guenter.ro...@ericsson.com and Donggeun Kim dg77@samsung.com (https://lkml.org/lkml/2012/1/5/7) *Some changes according to the changes in common cpu cooling APIs *The RFC based patches can be found here, https://lkml.org/lkml/2011/12/13/186 https://lkml.org/lkml/2011/12/21/169 Brief Description: 1) The generic cooling devices code is placed inside driver/thermal/* as placing inside acpi folder will need un-necessary enabling of acpi code. This codes is architecture independent. 2) This patchset adds a new trip type THERMAL_TRIP_STATE_INSTANCE which passes cooling device instance number and may be helpful for cpufreq cooling devices to take the correct cooling action. This trip type avoids the temperature comparision check again inside the cooling handler. 3) This patchset adds generic cpu cooling low level implementation through frequency clipping and cpu hotplug. In future, other cpu related cooling devices may be added here. An ACPI version of this already exists (drivers/acpi/processor_thermal.c). But this will be useful for platforms like ARM using the generic thermal interface along with the generic cpu cooling devices. The cooling device registration API's return cooling device pointers which can be easily binded with the thermal zone trip points. The important APIs exposed are, a)struct thermal_cooling_device *cpufreq_cooling_register( struct freq_clip_table *tab_ptr, unsigned int tab_size, const struct cpumask *mask_val) b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) 4) Samsung exynos platform thermal implementation is done using the generic cpu cooling APIs and the new trip type. The temperature sensor driver present in the hwmon folder(registered as hwmon driver) is moved to thermal folder and registered as a thermal driver. All this patchset is based on Kernel version 3.3-rc7 A simple data/control flow diagrams is shown below, Core Linux thermal - Exynos thermal interface - Temperature Sensor | | \|/ | Cpufreq cooling device --- Amit Daniel Kachhap (6): thermal: Add a new trip type to use cooling device instance number thermal: Add generic cpufreq cooling implementation thermal: Add generic cpuhotplug cooling implementation hwmon: exynos4: Move thermal sensor driver to driver/thermal directory thermal: exynos4: Register the tmu sensor with the kernel thermal layer ARM: exynos4: Add thermal sensor driver platform device support Documentation/hwmon/exynos4_tmu | 81 --- Documentation/thermal/cpu-cooling-api.txt | 76 +++ Documentation/thermal/exynos4_tmu | 52 ++ Documentation/thermal/sysfs-api.txt | 4 +- arch/arm/mach-exynos/Kconfig | 11 + arch/arm/mach-exynos/Makefile | 1 + arch/arm/mach-exynos/clock.c | 4 + arch/arm/mach-exynos/dev-tmu.c | 39 ++ arch/arm/mach-exynos/include/mach/irqs.h | 2 + arch/arm/mach-exynos/include/mach/map.h | 1 + arch/arm/mach-exynos/mach-origen.c | 1 + arch/arm/plat-samsung/include/plat/devs.h | 1 + drivers/hwmon/Kconfig | 10 -
[PATCH V2 0/6] thermal: exynos: Add kernel thermal support for exynos platform
Changes since V1: *Moved the sensor driver to driver/thermal folder from driver/hwmon folder as suggested by Mark Brown and Guenter Roeck *Added notifier support to notify the registered drivers of any cpu cooling action. The driver can modify the default cooling behaviour(eg set different max clip frequency). *The percentage based frequency replaced with absolute clipped frequency. *Some more conditional checks when setting max frequency. *Renamed the new trip type THERMAL_TRIP_STATE_ACTIVE to THERMAL_TRIP_STATE_INSTANCE *Many review comments from R, Durgadoss durgados...@intel.com and eduardo.valen...@ti.com implemented. *Removed cooling stats through debugfs patch *The V1 based can be found here, https://lkml.org/lkml/2012/2/22/123 http://lkml.org/lkml/2012/3/3/32 Changes since RFC: *Changed the cpu cooling registration/unregistration API's to instance based *Changed the STATE_ACTIVE trip type to pass correct instance id *Adding support to restore back the policy-max_freq after doing frequency clipping. *Moved the trip cooling stats from sysfs node to debugfs node as suggested by Greg KH g...@kroah.com *Incorporated several review comments from eduardo.valen...@ti.com *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd as discussed with Guenter Roeck guenter.ro...@ericsson.com and Donggeun Kim dg77@samsung.com (https://lkml.org/lkml/2012/1/5/7) *Some changes according to the changes in common cpu cooling APIs *The RFC based patches can be found here, https://lkml.org/lkml/2011/12/13/186 https://lkml.org/lkml/2011/12/21/169 Brief Description: 1) The generic cooling devices code is placed inside driver/thermal/* as placing inside acpi folder will need un-necessary enabling of acpi code. This codes is architecture independent. 2) This patchset adds a new trip type THERMAL_TRIP_STATE_INSTANCE which passes cooling device instance number and may be helpful for cpufreq cooling devices to take the correct cooling action. This trip type avoids the temperature comparision check again inside the cooling handler. 3) This patchset adds generic cpu cooling low level implementation through frequency clipping and cpu hotplug. In future, other cpu related cooling devices may be added here. An ACPI version of this already exists (drivers/acpi/processor_thermal.c). But this will be useful for platforms like ARM using the generic thermal interface along with the generic cpu cooling devices. The cooling device registration API's return cooling device pointers which can be easily binded with the thermal zone trip points. The important APIs exposed are, a)struct thermal_cooling_device *cpufreq_cooling_register( struct freq_clip_table *tab_ptr, unsigned int tab_size, const struct cpumask *mask_val) b)void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev) 4) Samsung exynos platform thermal implementation is done using the generic cpu cooling APIs and the new trip type. The temperature sensor driver present in the hwmon folder(registered as hwmon driver) is moved to thermal folder and registered as a thermal driver. All this patchset is based on Kernel version 3.3-rc7 A simple data/control flow diagrams is shown below, Core Linux thermal - Exynos thermal interface - Temperature Sensor | | \|/| Cpufreq cooling device --- Amit Daniel Kachhap (6): thermal: Add a new trip type to use cooling device instance number thermal: Add generic cpufreq cooling implementation thermal: Add generic cpuhotplug cooling implementation hwmon: exynos4: Move thermal sensor driver to driver/thermal directory thermal: exynos4: Register the tmu sensor with the kernel thermal layer ARM: exynos4: Add thermal sensor driver platform device support Documentation/hwmon/exynos4_tmu | 81 --- Documentation/thermal/cpu-cooling-api.txt | 76 +++ Documentation/thermal/exynos4_tmu | 52 ++ Documentation/thermal/sysfs-api.txt |4 +- arch/arm/mach-exynos/Kconfig | 11 + arch/arm/mach-exynos/Makefile |1 + arch/arm/mach-exynos/clock.c |4 + arch/arm/mach-exynos/dev-tmu.c| 39 ++ arch/arm/mach-exynos/include/mach/irqs.h |2 + arch/arm/mach-exynos/include/mach/map.h |1 + arch/arm/mach-exynos/mach-origen.c|1 + arch/arm/plat-samsung/include/plat/devs.h |1 + drivers/hwmon/Kconfig | 10 - drivers/hwmon/Makefile|1 - drivers/hwmon/exynos4_tmu.c | 514 --- drivers/thermal/Kconfig | 21 + drivers/thermal/Makefile |2 + drivers/thermal/cpu_cooling.c | 529 +++ drivers/thermal/exynos4_thermal.c | 790 + drivers/thermal/thermal_sys.c | 45 ++-
