Re: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for exynos platform

2012-05-09 Thread Amit Kachhap
On 9 May 2012 01:36, Zhang, Rui rui.zh...@intel.com wrote:
 Hi, Amit,

 Sorry for the late response as I'm in a travel recently.

 I think the generic cpufreq cooling patches are good.

 But about the THERMAL_TRIP_STATE_INSTANCE patch, what I'd like to see is that
 1. from thermal zone point of view, when the temperature is higher than a 
 trip point, either ACTIVE or PASSIVE, what we should do is to set device 
 cooling state to cur_state+1, right?
  The only difference is that if we should take passive cooling actions or 
 active cooling actions based on the policy.
  So my question would be if it is possible to combine these two kind of trip 
 points together. Maybe something like this:

  In thermal_zone_device_update(),

  ...
  case THERMAL_TRIP_PASSIVE:
      if (passive cooling not allowed)
           continue;
      if (tc1)
           thermal_zone_device_passive();
      else
           thermal_set_cooling_state();
      break;
  case THERMAL_TRIP_ACTIVE:
     if (active cooling not allowed)
          continue;
     thermal_set_cooling_state();
     break;
  ...

 and thermal_set_cooling_state() {
   list_for_each_entry(instance, tz-cooling_devices, node) {
      if (instance-trip != count)
         continue;

      cdev = instance-cdev;

      if (temp = trip_temp)
          cdev-ops-set_cur_state(cdev, 1);
      else
          cdev-ops-set_cur_state(cdev, 0);
   }
 }

 2. use only one cooling_device instance for a thermal_zone, and introduce 
 cdev-trips which shows the trip points this cooling device is bind to.
  And we may use multiple cooling devices states for one active trip point.

 Then, thermal_set_cooling_state() would be look like

 list_for_each_entry(instance, tz-cooling_devices, node) {
   cdev = instance-cdev;
   /* check whether this cooling device is bind to the trip point */
   if (!(cdev-trips  1count))
      continue;
   cdev-ops-get_max_state(cdev, max_state);
   cdev-ops-get_cur_state(cdev, cur_state);

   if (temp = trip_temp) {
      if (cur_state++ = max_state))
        cdev-ops-set_cur_state(cdev, cur_state);
   } else if ((temp  trip_temp)  (cur_state-- = 0))
      cdev-ops-set_cur_state(cdev, cur_state);
                        }
 }

Hi Rui,

The above implementation  also cools instance based cooling devices
like passive trip type. I need some changes on top of your
implementation such as,
thermal_set_cooling_state() {
list_for_each_entry(instance, tz-cooling_devices,
 node) {
cdev = instance-cdev;
if (!cdev-trips  1count)
  continue;

inst_id = 0;
for_each_bit_set(i, cdev-trips, count)
   inst_id++;
 cdev-ops-get_max_state(cdev, max_state);
 if ((temp = trip_temp)  (inst_id = max_state))
  cdev-ops-set_cur_state(cdev, inst_id);
 else if ((temp  trip_temp)  (--inst_id = 0))
   cdev-ops-set_cur_state(cdev, inst_id);
 }
}

I agree with you that the instance based trip types violates the
concept like reading the cur_state and do cur_state++/cur_state--
depending upon threshold increase or decrease because it needs the
state_id/inst_id.
I am actually thinking of dropping this new trip type and use the
existing THERMAL_TRIP_ACTIVE because there is so much logic in
calculating the state_id. The only flip side of using TRIP_ACTIVE is
that I need to create so many cooling devices.

Thanks,
Amit D

 With these two things, I think the WARN_ZONE AND MONITOR_ZONE can be 
 registered as two PASSIVE trip points in the generic thermal layer, right?
 Actually, this is one thing in my TODO list. And I'd glad to make it high 
 priority if this solves the problem you have.

 Thanks,
 rui

 -Original Message-
 From: linux-acpi-ow...@vger.kernel.org 
 [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Amit Daniel Kachhap
 Sent: Tuesday, May 08, 2012 9:18 AM
 To: a...@linux-foundation.org; linux...@lists.linux-foundation.org
 Cc: R, Durgadoss; linux-a...@vger.kernel.org; l...@kernel.org; Zhang, Rui; 
 amit.kach...@linaro.org; linaro-...@lists.linaro.org; 
 linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
 linux-samsung-soc@vger.kernel.org; patc...@linaro.org
 Subject: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for 
 exynos platform
 Importance: High

 Hi Andrew,

 This patchset introduces a new generic cooling device based on cpufreq that 
 can be used on non-ACPI platforms. As a proof of concept, we have drivers for 
 the following platforms using this mechanism now:

  * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git 
 omap4460_thermal)
  * Samsung Exynos (Exynos4 and Exynos5) in the current patchset.
  * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git 
 imx6q_thermal)

 These patches have been reviewed by Rui Zhang 
 (https://lkml.org/lkml/2012/4/9/448)
 who seems to agree with them in principle, but I haven't had any luck getting 
 them merged, perhaps 

RE: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for exynos platform

2012-05-08 Thread Zhang, Rui
Hi, Amit,

Sorry for the late response as I'm in a travel recently.

I think the generic cpufreq cooling patches are good.

But about the THERMAL_TRIP_STATE_INSTANCE patch, what I'd like to see is that 
1. from thermal zone point of view, when the temperature is higher than a trip 
point, either ACTIVE or PASSIVE, what we should do is to set device cooling 
state to cur_state+1, right?
  The only difference is that if we should take passive cooling actions or 
active cooling actions based on the policy.
  So my question would be if it is possible to combine these two kind of trip 
points together. Maybe something like this:

  In thermal_zone_device_update(),

  ...
  case THERMAL_TRIP_PASSIVE:
  if (passive cooling not allowed)
   continue;
  if (tc1)
   thermal_zone_device_passive();
  else
   thermal_set_cooling_state();
  break;
  case THERMAL_TRIP_ACTIVE:
 if (active cooling not allowed)
  continue;
 thermal_set_cooling_state();
 break;
  ...

and thermal_set_cooling_state() {
   list_for_each_entry(instance, tz-cooling_devices, node) {
  if (instance-trip != count)
 continue;

  cdev = instance-cdev;

  if (temp = trip_temp)
  cdev-ops-set_cur_state(cdev, 1);
  else
  cdev-ops-set_cur_state(cdev, 0);
   }
}

2. use only one cooling_device instance for a thermal_zone, and introduce 
cdev-trips which shows the trip points this cooling device is bind to.
  And we may use multiple cooling devices states for one active trip point.

Then, thermal_set_cooling_state() would be look like

list_for_each_entry(instance, tz-cooling_devices, node) {
   cdev = instance-cdev;
   /* check whether this cooling device is bind to the trip point */
   if (!(cdev-trips  1count))
  continue;
   cdev-ops-get_max_state(cdev, max_state);
   cdev-ops-get_cur_state(cdev, cur_state);

   if (temp = trip_temp) {
  if (cur_state++ = max_state))
cdev-ops-set_cur_state(cdev, cur_state);
   } else if ((temp  trip_temp)  (cur_state-- = 0))
  cdev-ops-set_cur_state(cdev, cur_state);
}
}

With these two things, I think the WARN_ZONE AND MONITOR_ZONE can be registered 
as two PASSIVE trip points in the generic thermal layer, right?
Actually, this is one thing in my TODO list. And I'd glad to make it high 
priority if this solves the problem you have.

Thanks,
rui

-Original Message-
From: linux-acpi-ow...@vger.kernel.org 
[mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Amit Daniel Kachhap
Sent: Tuesday, May 08, 2012 9:18 AM
To: a...@linux-foundation.org; linux...@lists.linux-foundation.org
Cc: R, Durgadoss; linux-a...@vger.kernel.org; l...@kernel.org; Zhang, Rui; 
amit.kach...@linaro.org; linaro-...@lists.linaro.org; 
linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
linux-samsung-soc@vger.kernel.org; patc...@linaro.org
Subject: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for exynos 
platform
Importance: High

Hi Andrew,

This patchset introduces a new generic cooling device based on cpufreq that can 
be used on non-ACPI platforms. As a proof of concept, we have drivers for the 
following platforms using this mechanism now:

 * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git omap4460_thermal)
 * Samsung Exynos (Exynos4 and Exynos5) in the current patchset.
 * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git 
imx6q_thermal)

These patches have been reviewed by Rui Zhang 
(https://lkml.org/lkml/2012/4/9/448)
who seems to agree with them in principle, but I haven't had any luck getting 
them merged, perhaps a lack of maintainer bandwidth.

ACPI platforms currently have such a mechanism but it is wrapped in ACPI'isms 
that we don't have on ARM platforms. If this is accepted, I'm proposing to 
convert over the ACPI thermal driver to use this common code too.

Can you please merge these patches for 3.5?

Thanks,
Amit Daniel


Changes since V2:
*Added Exynos5 TMU sensor support by enhancing the exynos4 tmu driver. Exynos5 
TMU  driver was internally developed by SangWook Ju sw...@samsung.com.
*Removed cpuhotplug cooling code in this patchset.
*Rebased the patches against 3.4-rc6 kernel.

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 v3 0/6] thermal: exynos: Add kernel thermal support for exynos platform

2012-05-08 Thread Andrew Morton
On Tue,  8 May 2012 21:48:12 +0530
Amit Daniel Kachhap amit.kach...@linaro.org wrote:

 This patchset introduces a new generic cooling device based on cpufreq that
 can be used on non-ACPI platforms. As a proof of concept, we have drivers for
 the following platforms using this mechanism now:
 
  * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git 
 omap4460_thermal)
  * Samsung Exynos (Exynos4 and Exynos5) in the current patchset.
  * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git 
 imx6q_thermal)
 
 These patches have been reviewed by Rui Zhang 
 (https://lkml.org/lkml/2012/4/9/448)

But we don't have explicit Reviewed-by:s for the changelogs?

 who seems to agree with them in principle, but I haven't had any luck getting 
 them
 merged, perhaps a lack of maintainer bandwidth.
 
 ACPI platforms currently have such a mechanism but it is wrapped in ACPI'isms
 that we don't have on ARM platforms. If this is accepted, I'm proposing to
 convert over the ACPI thermal driver to use this common code too.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html