Hi Rui,

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

quoted

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;

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

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;
}

Yes, this can work with little bit change like below:

struct thermal_zone_params {
	const char *zone_name;
	int num_cdevs;
	struct thermal_bind_params[num_cdevs];
}; 

Where struct thermal_bind_params will be like this:
	{
	.cdev_name = "CPU"
	.trip_mask = 0x0F
	.weight = 70
	.lower = 2
	.upper = 4
	.sample_period = 1000 (1 ms)
	};

Let me know what you think.

Thanks,
Durga