This series pacthes to support the next soc for this thermal driver.
I don't add the dts thermal data since these SoCs have *_not_* land
in this mainline. I believe these SoCs dts will land in this mainline
lately,
then I will add the thermal data for Heiko.
This series patches can apply into Eduardo branch.
https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux-soc-thermal.git
Also, this series pacthes have built on github branch.
https://github.com/rockchip-linux/kernel/commits/develop4.4
PATCH[1/5]:
That's bit ugly typo, sorry for sending again :(.
PATCH[2/5]:
To fix a build warning came from Dan Carpenter report smatch check,
Thanks. :)
PATCH[3/5]:
Add the rk3228/rk3399 SoCs compatible for dt-bindings.
PATCH[4/5]:
Add the rk3228 SoCs for thermal driver.
PATCH[5/5]:
Add the rk3399 SoCs for thermal driver based on PATCH[4/5].
I'd appreciate if someone have free time to review that. :)
Changes in v3:
- As Brian comments on https://patchwork.kernel.org/patch/7580661/,
let's remove the impossible condition.
Changes in v2:
- As Heiko comments, move to documenting the fields in the header
instead of inside the table.
Changes in v1:
- Search more trivial typo for me.
- As Heiko comments, fix a copy incorrect name.
- Add a Acked from Rob.
- fix a irq ack is similar with RK3228 SoCs.
Caesar Wang (5):
thermal: rockchip: fix a trivial typo
thermal: rockchip: fix a impossible condition caused by the warning
dt-bindings: rockchip-thermal: Support the RK3228/RK3399 SoCs
compatible
thermal: rockchip: Support the RK3228 SoCs in thermal driver
thermal: rockchip: Support the RK3399 SoCs in thermal driver
.../bindings/thermal/rockchip-thermal.txt | 2 +
drivers/thermal/rockchip_thermal.c | 178 ++++++++++++++++++---
2 files changed, 162 insertions(+), 18 deletions(-)
--
1.9.1
This patchset trys to dictate unified format for driver.
Signed-off-by: Caesar Wang <[email protected]>
---
Changes in v3: None
Changes in v2:
- As Heiko comments, move to documenting the fields in the header
instead of inside the table.
Changes in v1:
- Search more trivial typo for me.
drivers/thermal/rockchip_thermal.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index e845841..ae796ec 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -38,7 +38,7 @@ enum tshut_mode {
};
/**
- * the system Temperature Sensors tshut(tshut) polarity
+ * The system Temperature Sensors tshut(tshut) polarity
* the bit 8 is tshut polarity.
* 0: low active, 1: high active
*/
@@ -57,10 +57,10 @@ enum sensor_id {
};
/**
-* The conversion table has the adc value and temperature.
-* ADC_DECREMENT is the adc value decremnet.(e.g. v2_code_table)
-* ADC_INCREMNET is the adc value incremnet.(e.g. v3_code_table)
-*/
+ * The conversion table has the adc value and temperature.
+ * ADC_DECREMENT: the adc value is of diminishing.(e.g. v2_code_table)
+ * ADC_INCREMENT: the adc value is incremental.(e.g. v3_code_table)
+ */
enum adc_sort_mode {
ADC_DECREMENT = 0,
ADC_INCREMENT,
@@ -72,16 +72,17 @@ enum adc_sort_mode {
*/
#define SOC_MAX_SENSORS 2
+/**
+ * struct chip_tsadc_table: hold information about chip-specific differences
+ * @id: conversion table
+ * @length: size of conversion table
+ * @data_mask: mask to apply on data inputs
+ * @mode: sort mode of this adc variant (incrementing or decrementing)
+ */
struct chip_tsadc_table {
const struct tsadc_table *id;
-
- /* the array table size*/
unsigned int length;
-
- /* that analogic mask data */
u32 data_mask;
-
- /* the sort mode is adc value that increment or decrement in table */
enum adc_sort_mode mode;
};
@@ -617,7 +618,7 @@ rockchip_thermal_register_sensor(struct platform_device *pdev,
return 0;
}
-/*
+/**
* Reset TSADC Controller, reset all tsadc registers.
*/
static void rockchip_thermal_reset_controller(struct reset_control *reset)
--
1.9.1
As the Dan report the smatch check the thermal driver warning:
drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
(s32min-s32max > s32max)'
Let's we remove the imposssible condition Since the Temperature is
currently represented as int not long in the thermal driver.
Fixes: commit 437df2172e8d
("thermal: rockchip: consistently use int for temperatures")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Caesar Wang <[email protected]>
---
Changes in v3:
- As Brian comments on https://patchwork.kernel.org/patch/7580661/,
let's remove the impossible condition.
Changes in v2: None
Changes in v1: None
drivers/thermal/rockchip_thermal.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index ae796ec..611de00 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -549,12 +549,6 @@ static int rockchip_configure_from_dt(struct device *dev,
thermal->tshut_temp = shut_temp;
}
- if (thermal->tshut_temp > INT_MAX) {
- dev_err(dev, "Invalid tshut temperature specified: %d\n",
- thermal->tshut_temp);
- return -ERANGE;
- }
-
if (of_property_read_u32(np, "rockchip,hw-tshut-mode", &tshut_mode)) {
dev_warn(dev,
"Missing tshut mode property, using default (%s)\n",
--
1.9.1
This patchset attempts to new compatible for thermal founding
on RK3228/RK3399 SoCs.
Signed-off-by: Caesar Wang <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes in v3: None
Changes in v2: None
Changes in v1:
- As Heiko comments, fix a copy incorrect name.
- Add a Acked from Rob.
Documentation/devicetree/bindings/thermal/rockchip-thermal.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
index 0dfa60d..08efe6b 100644
--- a/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/rockchip-thermal.txt
@@ -2,8 +2,10 @@
Required properties:
- compatible : should be "rockchip,<name>-tsadc"
+ "rockchip,rk3228-tsadc": found on RK3228 SoCs
"rockchip,rk3288-tsadc": found on RK3288 SoCs
"rockchip,rk3368-tsadc": found on RK3368 SoCs
+ "rockchip,rk3399-tsadc": found on RK3399 SoCs
- reg : physical base address of the controller and length of memory mapped
region.
- interrupts : The interrupt number to the cpu. The interrupt specifier format
--
1.9.1
The RK3228 SoCs has one Temperature Sensor, channel 0 is for CPU.
Signed-off-by: Caesar Wang <[email protected]>
---
Changes in v3: None
Changes in v2: None
Changes in v1: None
drivers/thermal/rockchip_thermal.c | 81 ++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 611de00..95415ac 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -154,6 +154,7 @@ struct rockchip_thermal_data {
#define TSADCV2_SHUT_2GPIO_SRC_EN(chn) BIT(4 + (chn))
#define TSADCV2_SHUT_2CRU_SRC_EN(chn) BIT(8 + (chn))
+#define TSADCV1_INT_PD_CLEAR_MASK ~BIT(16)
#define TSADCV2_INT_PD_CLEAR_MASK ~BIT(8)
#define TSADCV2_DATA_MASK 0xfff
@@ -169,6 +170,51 @@ struct tsadc_table {
int temp;
};
+/**
+ * Note:
+ * Code to Temperature mapping of the Temperature sensor is a piece wise linear
+ * curve.Any temperature, code faling between to 2 give temperatures can be
+ * linearly interpolated.
+ * Code to Temperature mapping should be updated based on sillcon results.
+ */
+static const struct tsadc_table v1_code_table[] = {
+ {TSADCV3_DATA_MASK, -40000},
+ {436, -40000},
+ {431, -35000},
+ {426, -30000},
+ {421, -25000},
+ {416, -20000},
+ {411, -15000},
+ {406, -10000},
+ {401, -5000},
+ {395, 0},
+ {390, 5000},
+ {385, 10000},
+ {380, 15000},
+ {375, 20000},
+ {370, 25000},
+ {364, 30000},
+ {359, 35000},
+ {354, 40000},
+ {349, 45000},
+ {343, 50000},
+ {338, 55000},
+ {333, 60000},
+ {328, 65000},
+ {322, 70000},
+ {317, 75000},
+ {312, 80000},
+ {307, 85000},
+ {301, 90000},
+ {296, 95000},
+ {291, 100000},
+ {286, 105000},
+ {280, 110000},
+ {275, 115000},
+ {270, 120000},
+ {264, 125000},
+};
+
static const struct tsadc_table v2_code_table[] = {
{TSADCV2_DATA_MASK, -40000},
{3800, -40000},
@@ -369,6 +415,14 @@ static void rk_tsadcv2_initialize(void __iomem *regs,
regs + TSADCV2_HIGHT_TSHUT_DEBOUNCE);
}
+static void rk_tsadcv1_irq_ack(void __iomem *regs)
+{
+ u32 val;
+
+ val = readl_relaxed(regs + TSADCV2_INT_PD);
+ writel_relaxed(val & TSADCV1_INT_PD_CLEAR_MASK, regs + TSADCV2_INT_PD);
+}
+
static void rk_tsadcv2_irq_ack(void __iomem *regs)
{
u32 val;
@@ -430,6 +484,29 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem *regs,
writel_relaxed(val, regs + TSADCV2_INT_EN);
}
+static const struct rockchip_tsadc_chip rk3228_tsadc_data = {
+ .chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */
+ .chn_num = 1, /* one channel 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_tsadcv1_irq_ack,
+ .control = rk_tsadcv2_control,
+ .get_temp = rk_tsadcv2_get_temp,
+ .set_tshut_temp = rk_tsadcv2_tshut_temp,
+ .set_tshut_mode = rk_tsadcv2_tshut_mode,
+
+ .table = {
+ .id = v1_code_table,
+ .length = ARRAY_SIZE(v1_code_table),
+ .data_mask = TSADCV3_DATA_MASK,
+ .mode = ADC_DECREMENT,
+ },
+};
+
static const struct rockchip_tsadc_chip rk3288_tsadc_data = {
.chn_id[SENSOR_CPU] = 1, /* cpu sensor is channel 1 */
.chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */
@@ -480,6 +557,10 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
static const struct of_device_id of_rockchip_thermal_match[] = {
{
+ .compatible = "rockchip,rk3228-tsadc",
+ .data = (void *)&rk3228_tsadc_data,
+ },
+ {
.compatible = "rockchip,rk3288-tsadc",
.data = (void *)&rk3288_tsadc_data,
},
--
1.9.1
The RK3399 SoCs have two Temperature Sensors, channel 0 is for CPU.
channel 1 is for GPU.
Signed-off-by: Caesar Wang <[email protected]>
---
Changes in v3: None
Changes in v2: None
Changes in v1:
- fix a irq ack is similar with RK3228 SoCs.
drivers/thermal/rockchip_thermal.c | 66 ++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index 95415ac..c597784 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -292,6 +292,44 @@ static const struct tsadc_table v3_code_table[] = {
{TSADCV3_DATA_MASK, 125000},
};
+static const struct tsadc_table v4_code_table[] = {
+ {TSADCV3_DATA_MASK, -40000},
+ {431, -40000},
+ {426, -35000},
+ {421, -30000},
+ {415, -25000},
+ {410, -20000},
+ {405, -15000},
+ {399, -10000},
+ {394, -5000},
+ {389, 0},
+ {383, 5000},
+ {378, 10000},
+ {373, 15000},
+ {367, 20000},
+ {362, 25000},
+ {357, 30000},
+ {351, 35000},
+ {346, 40000},
+ {340, 45000},
+ {335, 50000},
+ {330, 55000},
+ {324, 60000},
+ {319, 65000},
+ {313, 70000},
+ {308, 75000},
+ {302, 80000},
+ {297, 85000},
+ {291, 90000},
+ {286, 95000},
+ {281, 100000},
+ {275, 105000},
+ {270, 110000},
+ {264, 115000},
+ {259, 120000},
+ {253, 125000},
+};
+
static u32 rk_tsadcv2_temp_to_code(struct chip_tsadc_table table,
int temp)
{
@@ -555,6 +593,30 @@ static const struct rockchip_tsadc_chip rk3368_tsadc_data = {
},
};
+static const struct rockchip_tsadc_chip rk3399_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_tsadcv1_irq_ack,
+ .control = rk_tsadcv2_control,
+ .get_temp = rk_tsadcv2_get_temp,
+ .set_tshut_temp = rk_tsadcv2_tshut_temp,
+ .set_tshut_mode = rk_tsadcv2_tshut_mode,
+
+ .table = {
+ .id = v4_code_table,
+ .length = ARRAY_SIZE(v4_code_table),
+ .data_mask = TSADCV3_DATA_MASK,
+ .mode = ADC_DECREMENT,
+ },
+};
+
static const struct of_device_id of_rockchip_thermal_match[] = {
{
.compatible = "rockchip,rk3228-tsadc",
@@ -568,6 +630,10 @@ static const struct of_device_id of_rockchip_thermal_match[] = {
.compatible = "rockchip,rk3368-tsadc",
.data = (void *)&rk3368_tsadc_data,
},
+ {
+ .compatible = "rockchip,rk3399-tsadc",
+ .data = (void *)&rk3399_tsadc_data,
+ },
{ /* end */ },
};
MODULE_DEVICE_TABLE(of, of_rockchip_thermal_match);
--
1.9.1
On Thu, Dec 03, 2015 at 04:48:40PM +0800, Caesar Wang wrote:
> As the Dan report the smatch check the thermal driver warning:
> drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
> warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
> (s32min-s32max > s32max)'
>
> Let's we remove the imposssible condition Since the Temperature is
> currently represented as int not long in the thermal driver.
>
> Fixes: commit 437df2172e8d
> ("thermal: rockchip: consistently use int for temperatures")
>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Caesar Wang <[email protected]>
>
> ---
>
> Changes in v3:
> - As Brian comments on https://patchwork.kernel.org/patch/7580661/,
> let's remove the impossible condition.
>
> Changes in v2: None
> Changes in v1: None
>
> drivers/thermal/rockchip_thermal.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> index ae796ec..611de00 100644
> --- a/drivers/thermal/rockchip_thermal.c
> +++ b/drivers/thermal/rockchip_thermal.c
> @@ -549,12 +549,6 @@ static int rockchip_configure_from_dt(struct device *dev,
> thermal->tshut_temp = shut_temp;
> }
>
> - if (thermal->tshut_temp > INT_MAX) {
> - dev_err(dev, "Invalid tshut temperature specified: %d\n",
> - thermal->tshut_temp);
> - return -ERANGE;
> - }
Well, that is not entirely correct. The value that we read from DT is
u32, but we convert it down to int. I believe you want to move the check
up so that you do:
} else if (tshut_temp > INT_MAX) {
dev_err(dev, "Invalid tshut temperature specified: %d\n",
thermal->tshut_temp);
return -ERANGE;
} else {
thermal->tshut_temp = shut_temp;
}
Thanks.
--
Dmitry
On Thu, Dec 03, 2015 at 12:19:08PM -0800, Dmitry Torokhov wrote:
> On Thu, Dec 03, 2015 at 04:48:40PM +0800, Caesar Wang wrote:
> > As the Dan report the smatch check the thermal driver warning:
> > drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
> > warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
> > (s32min-s32max > s32max)'
> >
> > Let's we remove the imposssible condition Since the Temperature is
> > currently represented as int not long in the thermal driver.
> >
> > Fixes: commit 437df2172e8d
> > ("thermal: rockchip: consistently use int for temperatures")
> >
> > Reported-by: Dan Carpenter <[email protected]>
> > Signed-off-by: Caesar Wang <[email protected]>
> >
> > ---
> >
> > Changes in v3:
> > - As Brian comments on https://patchwork.kernel.org/patch/7580661/,
> > let's remove the impossible condition.
> >
> > Changes in v2: None
> > Changes in v1: None
> >
> > drivers/thermal/rockchip_thermal.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> > index ae796ec..611de00 100644
> > --- a/drivers/thermal/rockchip_thermal.c
> > +++ b/drivers/thermal/rockchip_thermal.c
> > @@ -549,12 +549,6 @@ static int rockchip_configure_from_dt(struct device *dev,
> > thermal->tshut_temp = shut_temp;
> > }
> >
> > - if (thermal->tshut_temp > INT_MAX) {
> > - dev_err(dev, "Invalid tshut temperature specified: %d\n",
> > - thermal->tshut_temp);
> > - return -ERANGE;
> > - }
>
> Well, that is not entirely correct. The value that we read from DT is
> u32, but we convert it down to int. I believe you want to move the check
Do we really account for the possibility of sizeof(int) < sizeof(u32)?
EDIT: A bit after writing the above line, I notice my error, but in case
anyone else is thinking the same thing... I guess you're referring to
the sign bit, since we're casting unsigned to signed.
Brian
> up so that you do:
>
> } else if (tshut_temp > INT_MAX) {
> dev_err(dev, "Invalid tshut temperature specified: %d\n",
> thermal->tshut_temp);
> return -ERANGE;
> } else {
> thermal->tshut_temp = shut_temp;
> }
>
> Thanks.
On Thu, Dec 03, 2015 at 12:33:57PM -0800, Brian Norris wrote:
> On Thu, Dec 03, 2015 at 12:19:08PM -0800, Dmitry Torokhov wrote:
> > On Thu, Dec 03, 2015 at 04:48:40PM +0800, Caesar Wang wrote:
> > > As the Dan report the smatch check the thermal driver warning:
> > > drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
> > > warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
> > > (s32min-s32max > s32max)'
> > >
> > > Let's we remove the imposssible condition Since the Temperature is
> > > currently represented as int not long in the thermal driver.
> > >
> > > Fixes: commit 437df2172e8d
> > > ("thermal: rockchip: consistently use int for temperatures")
> > >
> > > Reported-by: Dan Carpenter <[email protected]>
> > > Signed-off-by: Caesar Wang <[email protected]>
> > >
> > > ---
> > >
> > > Changes in v3:
> > > - As Brian comments on https://patchwork.kernel.org/patch/7580661/,
> > > let's remove the impossible condition.
> > >
> > > Changes in v2: None
> > > Changes in v1: None
> > >
> > > drivers/thermal/rockchip_thermal.c | 6 ------
> > > 1 file changed, 6 deletions(-)
> > >
> > > diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
> > > index ae796ec..611de00 100644
> > > --- a/drivers/thermal/rockchip_thermal.c
> > > +++ b/drivers/thermal/rockchip_thermal.c
> > > @@ -549,12 +549,6 @@ static int rockchip_configure_from_dt(struct device *dev,
> > > thermal->tshut_temp = shut_temp;
> > > }
> > >
> > > - if (thermal->tshut_temp > INT_MAX) {
> > > - dev_err(dev, "Invalid tshut temperature specified: %d\n",
> > > - thermal->tshut_temp);
> > > - return -ERANGE;
> > > - }
> >
> > Well, that is not entirely correct. The value that we read from DT is
> > u32, but we convert it down to int. I believe you want to move the check
>
> Do we really account for the possibility of sizeof(int) < sizeof(u32)?
>
> EDIT: A bit after writing the above line, I notice my error, but in case
> anyone else is thinking the same thing... I guess you're referring to
> the sign bit, since we're casting unsigned to signed.
Yes, exactly. Sorry I was not clear.
--
Dmitry
Hello,
On Thu, Dec 03, 2015 at 04:48:38PM +0800, Caesar Wang wrote:
> This series pacthes to support the next soc for this thermal driver.
> I don't add the dts thermal data since these SoCs have *_not_* land
> in this mainline. I believe these SoCs dts will land in this mainline
> lately,
> then I will add the thermal data for Heiko.
>
> This series patches can apply into Eduardo branch.
> https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux-soc-thermal.git
>
> Also, this series pacthes have built on github branch.
> https://github.com/rockchip-linux/kernel/commits/develop4.4
>
> PATCH[1/5]:
> That's bit ugly typo, sorry for sending again :(.
>
> PATCH[2/5]:
> To fix a build warning came from Dan Carpenter report smatch check,
> Thanks. :)
>
> PATCH[3/5]:
> Add the rk3228/rk3399 SoCs compatible for dt-bindings.
>
> PATCH[4/5]:
> Add the rk3228 SoCs for thermal driver.
>
> PATCH[5/5]:
> Add the rk3399 SoCs for thermal driver based on PATCH[4/5].
I applied all but 2/5.
在 2015年12月04日 04:19, Dmitry Torokhov 写道:
> On Thu, Dec 03, 2015 at 04:48:40PM +0800, Caesar Wang wrote:
>> As the Dan report the smatch check the thermal driver warning:
>> drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
>> warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
>> (s32min-s32max > s32max)'
>>
>> Let's we remove the imposssible condition Since the Temperature is
>> currently represented as int not long in the thermal driver.
>>
>> Fixes: commit 437df2172e8d
>> ("thermal: rockchip: consistently use int for temperatures")
>>
>> Reported-by: Dan Carpenter <[email protected]>
>> Signed-off-by: Caesar Wang <[email protected]>
>>
>> ---
>>
>> Changes in v3:
>> - As Brian comments on https://patchwork.kernel.org/patch/7580661/,
>> let's remove the impossible condition.
>>
>> Changes in v2: None
>> Changes in v1: None
>>
>> drivers/thermal/rockchip_thermal.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
>> index ae796ec..611de00 100644
>> --- a/drivers/thermal/rockchip_thermal.c
>> +++ b/drivers/thermal/rockchip_thermal.c
>> @@ -549,12 +549,6 @@ static int rockchip_configure_from_dt(struct device *dev,
>> thermal->tshut_temp = shut_temp;
>> }
>>
>> - if (thermal->tshut_temp > INT_MAX) {
>> - dev_err(dev, "Invalid tshut temperature specified: %d\n",
>> - thermal->tshut_temp);
>> - return -ERANGE;
>> - }
> Well, that is not entirely correct. The value that we read from DT is
> u32, but we convert it down to int. I believe you want to move the check
> up so that you do:
>
> } else if (tshut_temp > INT_MAX) {
> dev_err(dev, "Invalid tshut temperature specified: %d\n",
> thermal->tshut_temp);
> return -ERANGE;
> } else {
> thermal->tshut_temp = shut_temp;
> }
Okay,
that's seem the following wll be resonable since the checkcode is a warning.
if (of_property_read_u32(np, "rockchip,hw-tshut-temp", &shut_temp)) {
dev_warn(dev,
"Missing tshut temp property, using default %d\n",
thermal->chip->tshut_temp);
thermal->tshut_temp = thermal->chip->tshut_temp;
} else {
if (tshut_temp > INT_MAX) {
dev_err(dev, "Invalid tshut temperature specified: %d\n",
tshut_temp);
return -ERANGE;
}
thermal->tshut_temp = shut_temp;
}
Thanks,
> Thanks.
>
As the Dan report the smatch check the thermal driver warning:
drivers/thermal/rockchip_thermal.c:551 rockchip_configure_from_dt()
warn: impossible condition '(thermal->tshut_temp > ((~0 >> 1))) =>
(s32min-s32max > s32max)'
Although The shut_temp read from DT is u32,the temperature is currently
represented as int not long in the thermal driver.
Let's change to make shut_temp instead of the thermal->tshut_temp for
the condition.
Fixes: commit 437df2172e8d
("thermal: rockchip: consistently use int for temperatures")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Caesar Wang <[email protected]>
---
Changes in v4:
- As the Dmitry and Brain comments, let's change to make.sh
tshut_temp instead of thermal->tshut_temp.
Changes in v3:
- As Brian comments on https://patchwork.kernel.org/patch/7580661/,
let's remove the impossible condition.
Changes in v2:
- None
Changes in v1:
- None
drivers/thermal/rockchip_thermal.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/rockchip_thermal.c b/drivers/thermal/rockchip_thermal.c
index e845841..7106288 100644
--- a/drivers/thermal/rockchip_thermal.c
+++ b/drivers/thermal/rockchip_thermal.c
@@ -545,15 +545,14 @@ static int rockchip_configure_from_dt(struct device *dev,
thermal->chip->tshut_temp);
thermal->tshut_temp = thermal->chip->tshut_temp;
} else {
+ if (shut_temp > INT_MAX) {
+ dev_err(dev, "Invalid tshut temperature specified: %d\n",
+ shut_temp);
+ return -ERANGE;
+ }
thermal->tshut_temp = shut_temp;
}
- if (thermal->tshut_temp > INT_MAX) {
- dev_err(dev, "Invalid tshut temperature specified: %d\n",
- thermal->tshut_temp);
- return -ERANGE;
- }
-
if (of_property_read_u32(np, "rockchip,hw-tshut-mode", &tshut_mode)) {
dev_warn(dev,
"Missing tshut mode property, using default (%s)\n",
--
1.9.1
在 2015年12月18日 04:09, Eduardo Valentin 写道:
> Hello,
>
> On Thu, Dec 03, 2015 at 04:48:38PM +0800, Caesar Wang wrote:
>> This series pacthes to support the next soc for this thermal driver.
>> I don't add the dts thermal data since these SoCs have *_not_* land
>> in this mainline. I believe these SoCs dts will land in this mainline
>> lately,
>> then I will add the thermal data for Heiko.
>>
>> This series patches can apply into Eduardo branch.
>> https://git.kernel.org/cgit/linux/kernel/git/evalenti/linux-soc-thermal.git
>>
>> Also, this series pacthes have built on github branch.
>> https://github.com/rockchip-linux/kernel/commits/develop4.4
>>
>> PATCH[1/5]:
>> That's bit ugly typo, sorry for sending again :(.
>>
>> PATCH[2/5]:
>> To fix a build warning came from Dan Carpenter report smatch check,
>> Thanks. :)
>>
>> PATCH[3/5]:
>> Add the rk3228/rk3399 SoCs compatible for dt-bindings.
>>
>> PATCH[4/5]:
>> Add the rk3228 SoCs for thermal driver.
>>
>> PATCH[5/5]:
>> Add the rk3399 SoCs for thermal driver based on PATCH[4/5].
> I applied all but 2/5.
Thanks Eduardo,
I send the new patch[2/5].
https://patchwork.kernel.org/patch/7891381/
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
caesar wang | software engineer | [email protected]