On 10 April 2012 06:28, Zhang Rui <rui.zhang@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 triptype when tc1 and tc2 are 0. but this is special type of coolingdevices behaviour where only instances of the same cooling device isbinded to a trip point. The order of mapping is the onlydifferentiating criteria and there are some checks used to implementthis like1) The trip points should be in ascending order.(This is missing in myoriginal patch, so I added below)2) The set_cur_state has to be called for the exact temp range soget_cur_state(&state) and set_cur_state(state ++/state--) logic is notpossible.3) set_cur_state is called as set_cur_state(cdev_instance).There is a chance that people might confuse that these checks areapplicable for passive trip types also which is not the case here.

if (!passive)Anyway there is other alternative where this new trip type is notneeded and I can just use the existing trip type THERMAL_TRIP_ACTIVEand create 2 separate cooling devices for MONITOR_ZONE and WARN_ZONE.I had thought to make this generic and just to manage with 1 coolingdevice.What is your view?