Re: [PATCH V2 0/6] thermal: exynos: Add kernel thermal support forexynos platform

From: Amit Kachhap
Date: Wed Apr 11 2012 - 08:47:46 EST


Hi Rui,

Thanks for looking into the patches.

On 10 April 2012 06:28, Zhang Rui <rui.zhang@xxxxxxxxx> 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).
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.kachhap@xxxxxxxxxx> 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 <durgadoss.r@xxxxxxxxx> and
>> > Âeduardo.valentin@xxxxxx 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 greg@xxxxxxxxx
>> > *Incorporated several review comments from eduardo.valentin@xxxxxx
>> > *Moved the Temperature sensor driver from driver/hwmon/ to driver/mfd
>> > Âas discussed with Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> and
>> > ÂDonggeun Kim <dg77.kim@xxxxxxxxxxx> (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 ++-
>> > Âinclude/linux/cpu_cooling.h        |  78 +++
>> > Âinclude/linux/platform_data/exynos4_tmu.h | Â Â7 +
>> > Âinclude/linux/thermal.h          |  Â1 +
>> > Â23 files changed, 1660 insertions(+), 611 deletions(-)
>> > Âdelete mode 100644 Documentation/hwmon/exynos4_tmu
>> > Âcreate mode 100644 Documentation/thermal/cpu-cooling-api.txt
>> > Âcreate mode 100644 Documentation/thermal/exynos4_tmu
>> > Âcreate mode 100644 arch/arm/mach-exynos/dev-tmu.c
>> > Âdelete mode 100644 drivers/hwmon/exynos4_tmu.c
>> > Âcreate mode 100644 drivers/thermal/cpu_cooling.c
>> > Âcreate mode 100644 drivers/thermal/exynos4_thermal.c
>> > Âcreate mode 100644 include/linux/cpu_cooling.h
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/