On 二, 2012-08-21 at 00:41 -0600, R, Durgadoss wrote:

Hi Rui,

quoted

quoted

quoted

-----Original Message-----
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
owner@vger.kernel.org] On Behalf Of Eduardo Valentin
Sent: Tuesday, August 21, 2012 11:10 AM
To: R, Durgadoss
Cc: lenb@kernel.org; Zhang, Rui; rjw@sisk.pl; linux-acpi@vger.kernel.org;
linux-pm@vger.kernel.org; eduardo.valentin@ti.com;
amit.kachhap@linaro.org; wni@nvidia.com
Subject: Re: [PATCH 13/13] Thermal: Platform layer changes to provide
thermal data

Hello,

On Thu, Aug 09, 2012 at 06:16:05PM +0530, Durgadoss R wrote:

quoted

This patch shows how can we add platform specific thermal data
required by the thermal framework. This is just an example
patch, and _not_ for merge.

Signed-off-by: Durgadoss R <redacted>
---
 arch/x86/platform/mrst/mrst.c |   42

+++++++++++++++++++++++++++++++++++++++++

quoted

 1 file changed, 42 insertions(+)

diff --git a/arch/x86/platform/mrst/mrst.c

b/arch/x86/platform/mrst/mrst.c

quoted

index fd41a92..0440db5 100644

--- a/arch/x86/platform/mrst/mrst.c
+++ b/arch/x86/platform/mrst/mrst.c

@@ -30,6 +30,7 @@
 #include <linux/mfd/intel_msic.h>
 #include <linux/gpio.h>
 #include <linux/i2c/tc35876x.h>
+#include <linux/thermal.h>

 #include <asm/setup.h>
 #include <asm/mpspec_def.h>

@@ -78,6 +79,30 @@ struct sfi_rtc_table_entry

sfi_mrtc_array[SFI_MRTC_MAX];

quoted

 EXPORT_SYMBOL_GPL(sfi_mrtc_array);
 int sfi_mrtc_num;

+#define MRST_THERMAL_ZONES	3
+struct thermal_zone_params tzp[MRST_THERMAL_ZONES] = {
+	{ .thermal_zone_name = "CPU",
+	.throttle_policy = THERMAL_FAIR_SHARE,
+	.num_cdevs = 2,
+	.cdevs_name = {"CPU", "Battery"},
+	.trip_mask = {0x0F, 0x08},
+	.weights = {80, 20}, },
+
+	{ .thermal_zone_name = "Battery",
+	.throttle_policy = THERMAL_FAIR_SHARE,
+	.num_cdevs = 1,
+	.cdevs_name = {"Battery"},
+	.trip_mask = {0x0F},
+	.weights = {100}, },
+
+	{ .thermal_zone_name = "Skin",
+	.throttle_policy = THERMAL_FAIR_SHARE,
+	.num_cdevs = 2,
+	.cdevs_name = {"Display", "Battery"},
+	.trip_mask = {0x0F, 0x0F},
+	.weights = {50, 50}, }

Please consider the comment I sent on your data definition and also the
comment I made on this patch on your RFC series.

Yes.. I don't know why/how I missed it.
Also, saw the same comment on one of the other patches also.

Will surely fix this thing in v2.

BTW, any suggestion for the 'name' of that structure ? :-)

hmmm,
do we still have thermal_zone_platforms in patch v2?
I do not think we need this if we only bind devices via .bind()
callback.

We can bind devices via .bind call back, and that will take some load
off the framework code. But even then, we would need this structure
right ?

why?
I'd prefer introduce something like this,
struct thermal_bind_params {
	int trip;
	unsigned long upper;
	unsinged long lower;
	int weight;
	int sample_period;
}

and use thermal_zone_bind_cooling_device(tz, cdev, thermal_bind_params),
throttle_policy should be set when invoking
thermal_zone_device_register.

is there any information in thermal_zone_params can not be convert to
thermal_bind_params?

thanks,
rui

 Say, when we obtain platform data from a thermal driver, it
should know 'what format the platform data is' ..correct ?

I theoretically agree with you that individual platform drivers can
have data in their own format, but that will be a heavy loss on
standardization.

So,
I will remove the extra bind code I added to framework, and
(keep it the old way it was) but still prefer to have the structure
put in thermal.h.

Thanks,
Durga

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html