Re: [PATCH v1 3/6] thermal: rockchip: Support the RK3368 SoCs in thermal driver
From: Caesar Wang
Date: Wed Nov 04 2015 - 23:54:41 EST
Hello Eduardo,
Thanks your comments.
å 2015å11æ04æ 01:48, Eduardo Valentin åé:
Hello Caesar,
On Thu, Oct 29, 2015 at 03:14:15PM +0800, Caesar Wang wrote:
The RK3368 SoCs support to 2 channel TS-ADC, the temperature criteria
of each channel can be configurable.
The system has two Temperature Sensors, channel 0 is for CPU,
and channel 1 is for GPU.
Please improve your patch description. I dont think this patch only
adds the support for RK3368.
I see, for example, at least two other (maybe dependencies of the
new chip support) changes:
- conversion function improvement
- tshut value/temperature configuration
Could you please split this patch in smaller changes?
Okay, Done.
That's on my local oneline.
92ffb82 thermal: rockchip: Support the RK3368 SoCs in thermal drivers
e4f5e61 thermal: rockchip: Add the flag for adc value increment or decrement
b599a6b thermal: rockchip: improve the conversion function
d629c52 thermal: rockchip: trivial: fix typo in commit
I will send these in next version.
Signed-off-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>
---
Changes in v1:
- As Dmitry comment, make the conversion table in as a parameter.
Are you sure that this version implements this suggestion?
drivers/thermal/rockchip_thermal.c | 183 ++++++++++++++++++++++++++++++++-----
1 file changed, 158 insertions(+), 25 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index f96c151..4748a8e 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -1,6 +1,9 @@
/*
* Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
*
+ * Copyright (c) 2015, Fuzhou Rockchip Electronics Co., Ltd
+ * Caesar Wang <wxt@xxxxxxxxxxxxxx>
+ *
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
* version 2, as published by the Free Software Foundation.
@@ -75,8 +78,12 @@ struct rockchip_tsadc_chip {
/* Per-sensor methods */
int (*get_temp)(int chn, void __iomem *reg, int *temp);
- void (*set_tshut_temp)(int chn, void __iomem *reg, long temp);
+ void (*set_tshut_value)(int chn, void __iomem *reg, u32 value);
Do you have any explanation why do you need to change from temp to
value? Can you include this in your patch description?
Okay, I think that's *not* really needed.
I have removed this change.
void (*set_tshut_mode)(int chn, void __iomem *reg, enum tshut_mode m);
+
+ /* Per-table methods */
+ const struct tsadc_table *table;
+ int table_num;
};
struct rockchip_thermal_sensor {
@@ -102,7 +109,7 @@ struct rockchip_thermal_data {
enum tshut_polarity tshut_polarity;
};
-/* TSADC V2 Sensor info define: */
+/* TSADC Sensor info define: */
#define TSADCV2_AUTO_CON 0x04
#define TSADCV2_INT_EN 0x08
#define TSADCV2_INT_PD 0x0c
@@ -124,6 +131,8 @@ struct rockchip_thermal_data {
#define TSADCV2_INT_PD_CLEAR_MASK ~BIT(8)
#define TSADCV2_DATA_MASK 0xfff
+#define TSADCV3_DATA_MASK 0x3ff
+
#define TSADCV2_HIGHT_INT_DEBOUNCE_COUNT 4
#define TSADCV2_HIGHT_TSHUT_DEBOUNCE_COUNT 4
#define TSADCV2_AUTO_PERIOD_TIME 250 /* msec */
@@ -172,21 +181,62 @@ static const struct tsadc_table v2_code_table[] = {
{3421, 125000},
};
-static u32 rk_tsadcv2_temp_to_code(long temp)
+static const struct tsadc_table v3_code_table[] = {
+ {0, -40000},
+ {106, -40000},
+ {108, -35000},
+ {110, -30000},
+ {112, -25000},
+ {114, -20000},
+ {116, -15000},
+ {118, -10000},
+ {120, -5000},
+ {122, 0},
+ {124, 5000},
+ {126, 10000},
+ {128, 15000},
+ {130, 20000},
+ {132, 25000},
+ {134, 30000},
+ {136, 35000},
+ {138, 40000},
+ {140, 45000},
+ {142, 50000},
+ {144, 55000},
+ {146, 60000},
+ {148, 65000},
+ {150, 70000},
+ {152, 75000},
+ {154, 80000},
+ {156, 85000},
+ {158, 90000},
+ {160, 95000},
+ {162, 100000},
+ {163, 105000},
+ {165, 110000},
+ {167, 115000},
+ {169, 120000},
+ {171, 125000},
+ {TSADCV3_DATA_MASK, 125000},
+};
+
+static u32 rk_tsadcv2_temp_to_code(const struct rockchip_tsadc_chip *chip,
+ long temp)
{
This function receives the chip structure as parameter...
Okay, we should do the table as a parameter.
+ const struct tsadc_table *table = chip->table;
int high, low, mid;
low = 0;
- high = ARRAY_SIZE(v2_code_table) - 1;
+ high = chip->table_num - 1;
mid = (high + low) / 2;
- if (temp < v2_code_table[low].temp || temp > v2_code_table[high].temp)
+ if (temp < table[low].temp || temp > table[high].temp)
return 0;
while (low <= high) {
- if (temp == v2_code_table[mid].temp)
- return v2_code_table[mid].code;
- else if (temp < v2_code_table[mid].temp)
+ if (temp == table[mid].temp)
+ return table[mid].code;
+ else if (temp < table[mid].temp)
high = mid - 1;
else
low = mid + 1;
And seams to do the work.. but..
I am assuming you have forgotten to continue the change in the remaining
functions: rk_tsadcv2_code_to_temp:
215
216 /*
217 * The 5C granularity provided by the table is too much. Let's
218 * assume that the relationship between sensor readings and
219 * temperature between 2 table entries is linear and interpolate
220 * to produce less granular result.
221 */
222 num = v2_code_table[mid].temp - v2_code_table[mid - 1].temp;
223 num *= v2_code_table[mid - 1].code - code;
224 denom = v2_code_table[mid - 1].code - v2_code_table[mid].code;
225 *temp = v2_code_table[mid - 1].temp + (num / denom);
226
Please finish the change in rk_tsadcv2_code_to_temp, to use chip->table, and
@@ -235,16 +285,59 @@ static int rk_tsadcv2_code_to_temp(u32 code, int *temp)
return 0;
}
+static int rk_tsadcv3_code_to_temp(u32 code, int *temp)
+{
+ unsigned int low = 1;
+ unsigned int high = ARRAY_SIZE(v3_code_table) - 1;
+ unsigned int mid = (low + high) / 2;
+ unsigned int num;
+ unsigned long denom;
+
+ BUILD_BUG_ON(ARRAY_SIZE(v3_code_table) < 2);
+
+ code &= TSADCV3_DATA_MASK;
+ if (code < v3_code_table[low].code)
+ return -EAGAIN; /* Incorrect reading */
+
+ while (low <= high) {
+ if (code >= v3_code_table[mid - 1].code &&
+ code < v3_code_table[mid].code)
+ break;
+ else if (code > v3_code_table[mid].code)
+ low = mid + 1;
+ else
+ high = mid - 1;
+ mid = (low + high) / 2;
+ }
+
+ /*
+ * The 5C granularity provided by the table is too much. Let's
+ * assume that the relationship between sensor readings and
+ * temperature between 2 table entries is linear and interpolate
+ * to produce less granular result.
+ */
+ num = v3_code_table[mid].temp - v3_code_table[mid - 1].temp;
+ num *= code - v3_code_table[mid - 1].code;
+ denom = v3_code_table[mid].code - v3_code_table[mid - 1].code;
+ *temp = v3_code_table[mid - 1].temp + (num / denom);
+
+ return 0;
+}
Do not add the above function, as it is a code duplication of
rk_tsadcv2_code_to_temp. The above function, functionality wise, the same
as rk_tsadcv2_code_to_temp, except that it uses a different table.
The suggestion is to pass the table as parameter to rk_tsadcv2_code_to_temp,
then just reuse rk_tsadcv2_code_to_temp in both cases.
Would that work for you?
Thanks, I think that's ok on next version.
+
/**
- * rk_tsadcv2_initialize - initialize TASDC Controller
- * (1) Set TSADCV2_AUTO_PERIOD, configure the interleave between
- * every two accessing of TSADC in normal operation.
- * (2) Set TSADCV2_AUTO_PERIOD_HT, configure the interleave between
- * every two accessing of TSADC after the temperature is higher
- * than COM_SHUT or COM_INT.
- * (3) Set TSADCV2_HIGH_INT_DEBOUNCE and TSADC_HIGHT_TSHUT_DEBOUNCE,
- * if the temperature is higher than COMP_INT or COMP_SHUT for
- * "debounce" times, TSADC controller will generate interrupt or TSHUT.
+ * rk_tsadcv2_initialize - initialize TASDC Controller.
+ *
+ * (1) Set TSADC_V2_AUTO_PERIOD:
+ * Configure the interleave between every two accessing of
+ * TSADC in normal operation.
+ *
+ * (2) Set TSADCV2_AUTO_PERIOD_HT:
+ * Configure the interleave between every two accessing of
+ * TSADC after the temperature is higher than COM_SHUT or COM_INT.
+ *
+ * (3) Set TSADCV2_HIGH_INT_DEBOUNCE and TSADC_HIGHT_TSHUT_DEBOUNCE:
+ * If the temperature is higher than COMP_INT or COMP_SHUT for
+ * "debounce" times, TSADC controller will generate interrupt or TSHUT.
*/
static void rk_tsadcv2_initialize(void __iomem *regs,
enum tshut_polarity tshut_polarity)
@@ -286,6 +379,15 @@ static void rk_tsadcv2_control(void __iomem *regs, bool enable)
writel_relaxed(val, regs + TSADCV2_AUTO_CON);
}
+static int rk_tsadcv3_get_temp(int chn, void __iomem *regs, int *temp)
+{
+ u32 val;
+
+ val = readl_relaxed(regs + TSADCV2_DATA(chn));
+
+ return rk_tsadcv3_code_to_temp(val, temp);
+}
Same suggestion here. This function looks exactly the same as rk_tsadcv2_get_temp. can you just pass the chip as parameter, then?
Done, make a table as a parameter.
+
static int rk_tsadcv2_get_temp(int chn, void __iomem *regs, int *temp)
{
u32 val;
@@ -295,12 +397,11 @@ static int rk_tsadcv2_get_temp(int chn, void __iomem *regs, int *temp)
return rk_tsadcv2_code_to_temp(val, temp);
}
-static void rk_tsadcv2_tshut_temp(int chn, void __iomem *regs, long temp)
+static void rk_tsadcv2_tshut_value(int chn, void __iomem *regs, u32 value)
{
- u32 tshut_value, val;
+ u32 val;
- tshut_value = rk_tsadcv2_temp_to_code(temp);
- writel_relaxed(tshut_value, regs + TSADCV2_COMP_SHUT(chn));
+ writel_relaxed(value, regs + TSADCV2_COMP_SHUT(chn));
/* TSHUT will be valid */
val = readl_relaxed(regs + TSADCV2_AUTO_CON);
@@ -337,8 +438,31 @@ static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
.irq_ack = rk_tsadcv2_irq_ack,
.control = rk_tsadcv2_control,
.get_temp = rk_tsadcv2_get_temp,
- .set_tshut_temp = rk_tsadcv2_tshut_temp,
+ .set_tshut_value = rk_tsadcv2_tshut_value,
+ .set_tshut_mode = rk_tsadcv2_tshut_mode,
+
+ .table = v2_code_table,
+ .table_num = ARRAY_SIZE(v2_code_table),
+};
+
+static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
+ .chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+ .chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */
+ .chn_num = 2, /* two channels for tsadc */
+
+ .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */
+ .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
+ .tshut_temp = 95000,
+
+ .initialize = rk_tsadcv2_initialize,
+ .irq_ack = rk_tsadcv2_irq_ack,
+ .control = rk_tsadcv2_control,
+ .get_temp = rk_tsadcv3_get_temp,
+ .set_tshut_value = rk_tsadcv2_tshut_value,
.set_tshut_mode = rk_tsadcv2_tshut_mode,
+
+ .table = v3_code_table,
+ .table_num = ARRAY_SIZE(v3_code_table),
};
static const struct of_device_id of_rockchip_thermal_match[] = {
@@ -346,6 +470,10 @@ static const struct of_device_id of_rockchip_thermal_match[] = {
.compatible = "rockchip,rk3288-tsadc",
.data = (void *)&rk3288_tsadc_data,
},
+ {
+ .compatible = "rockchip,rk3368-tsadc",
+ .data = (void *)&rk3368_tsadc_data,
+ },
{ /* end */ },
};
MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
@@ -458,8 +586,10 @@ rockchip_thermal_register_sensor(struct platform_device *pdev,
const struct rockchip_tsadc_chip *tsadc = thermal->chip;
int error;
+ u32 tshut_value = rk_tsadcv2_temp_to_code(tsadc, thermal->tshut_temp);
+
tsadc->set_tshut_mode(id, thermal->regs, thermal->tshut_mode);
- tsadc->set_tshut_temp(id, thermal->regs, thermal->tshut_temp);
+ tsadc->set_tshut_value(id, thermal->regs, tshut_value);
sensor->thermal = thermal;
sensor->id = id;
@@ -660,6 +790,9 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
int i;
int error;
+ u32 tshut_value = rk_tsadcv2_temp_to_code(thermal->chip,
+ thermal->tshut_temp);
+
error = clk_enable(thermal->clk);
if (error)
return error;
@@ -677,8 +810,8 @@ static int __maybe_unused rockchip_thermal_resume(struct device *dev)
thermal->chip->set_tshut_mode(id, thermal->regs,
thermal->tshut_mode);
- thermal->chip->set_tshut_temp(id, thermal->regs,
- thermal->tshut_temp);
+ thermal->chip->set_tshut_value(id, thermal->regs,
+ tshut_value);
}
thermal->chip->control(thermal->regs, true);
--
1.9.1
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
Thanks,
Caesar
--
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/