Here is another series of tsens-related bugfixes and cleanups to prepare
for IRQ support, among other things. It applies on top of another series[1]
that did some code consolidation to add support for sdm845.
[1] https://lore.kernel.org/lkml/[email protected]/T/#t
Amit Kucheria (3):
thermal: tsens: Rename variable
thermal: tsens: switch from of_iomap() to devm_ioremap_resource()
thermal: tsens: Fix negative temperature reporting
drivers/thermal/qcom/tsens-common.c | 19 ++++++++++---------
drivers/thermal/qcom/tsens-v2.c | 23 ++++++++++-------------
2 files changed, 20 insertions(+), 22 deletions(-)
--
2.7.4
We're actually reading the temperature from the status register. Fix the
variable name to reflect that.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 6 +++---
drivers/thermal/qcom/tsens-v2.c | 10 +++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index c22dc18..25e7f24 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -104,11 +104,11 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
{
struct tsens_sensor *s = &tmdev->sensor[id];
u32 code;
- unsigned int sensor_addr;
+ unsigned int status_reg;
int last_temp = 0, ret;
- sensor_addr = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
- ret = regmap_read(tmdev->map, sensor_addr, &code);
+ status_reg = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
+ ret = regmap_read(tmdev->map, status_reg, &code);
if (ret)
return ret;
last_temp = code & SN_ST_TEMP_MASK;
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index f40150f..908e3dc 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -16,11 +16,11 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
{
struct tsens_sensor *s = &tmdev->sensor[id];
u32 code;
- unsigned int sensor_addr;
+ unsigned int status_reg;
int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
- sensor_addr = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
- ret = regmap_read(tmdev->map, sensor_addr, &code);
+ status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
+ ret = regmap_read(tmdev->map, status_reg, &code);
if (ret)
return ret;
last_temp = code & LAST_TEMP_MASK;
@@ -28,7 +28,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
goto done;
/* Try a second time */
- ret = regmap_read(tmdev->map, sensor_addr, &code);
+ ret = regmap_read(tmdev->map, status_reg, &code);
if (ret)
return ret;
if (code & STATUS_VALID_BIT) {
@@ -39,7 +39,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
}
/* Try a third/last time */
- ret = regmap_read(tmdev->map, sensor_addr, &code);
+ ret = regmap_read(tmdev->map, status_reg, &code);
if (ret)
return ret;
if (code & STATUS_VALID_BIT) {
--
2.7.4
devm_ioremap_resources() automatically requests resources (so that the I/O
region shows up in /proc/iomem) and devm_ wrappers do better error handling
and unmapping of the I/O region when needed.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 25e7f24..6207d8d 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -127,13 +127,11 @@ static const struct regmap_config tsens_config = {
int __init init_common(struct tsens_device *tmdev)
{
void __iomem *base;
+ struct resource *res;
struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
if (!op)
return -EINVAL;
- base = of_iomap(tmdev->dev->of_node, 0);
- if (!base)
- return -EINVAL;
/* The driver only uses the TM register address space for now */
if (op->num_resources > 1) {
@@ -143,11 +141,14 @@ int __init init_common(struct tsens_device *tmdev)
tmdev->tm_offset = 0x1000;
}
+ res = platform_get_resource(op, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&op->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
tmdev->map = devm_regmap_init_mmio(tmdev->dev, base, &tsens_config);
- if (IS_ERR(tmdev->map)) {
- iounmap(base);
+ if (IS_ERR(tmdev->map))
return PTR_ERR(tmdev->map);
- }
return 0;
}
--
2.7.4
The current code will always return 0xffffffff in case of negative
temperatures due to a bug in how the binary sign extension is being done.
Use sign_extend32() instead.
Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-v2.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 908e3dc..46abc21 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -5,19 +5,20 @@
*/
#include <linux/regmap.h>
+#include <linux/bitops.h>
#include "tsens.h"
#define STATUS_OFFSET 0xa0
#define LAST_TEMP_MASK 0xfff
#define STATUS_VALID_BIT BIT(21)
-#define CODE_SIGN_BIT BIT(11)
static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
{
struct tsens_sensor *s = &tmdev->sensor[id];
u32 code;
unsigned int status_reg;
- int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
+ u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
+ int ret;
status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
ret = regmap_read(tmdev->map, status_reg, &code);
@@ -54,12 +55,8 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
else if (last_temp2 == last_temp3)
last_temp = last_temp3;
done:
- /* Code sign bit is the sign extension for a negative value */
- if (last_temp & CODE_SIGN_BIT)
- last_temp |= ~CODE_SIGN_BIT;
-
- /* Temperatures are in deciCelicius */
- *temp = last_temp * 100;
+ /* Convert temperatures to milliCelicius */
+ *temp = sign_extend32(last_temp, fls(LAST_TEMP_MASK) - 1) * 100;
return 0;
}
--
2.7.4
On Wed, Jul 18, 2018 at 12:55:02PM +0530, Amit Kucheria wrote:
> devm_ioremap_resources() automatically requests resources (so that the I/O
> region shows up in /proc/iomem) and devm_ wrappers do better error handling
> and unmapping of the I/O region when needed.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/thermal/qcom/tsens-common.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 25e7f24..6207d8d 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -127,13 +127,11 @@ static const struct regmap_config tsens_config = {
> int __init init_common(struct tsens_device *tmdev)
> {
> void __iomem *base;
> + struct resource *res;
> struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
>
> if (!op)
> return -EINVAL;
> - base = of_iomap(tmdev->dev->of_node, 0);
> - if (!base)
> - return -EINVAL;
>
> /* The driver only uses the TM register address space for now */
> if (op->num_resources > 1) {
> @@ -143,11 +141,14 @@ int __init init_common(struct tsens_device *tmdev)
> tmdev->tm_offset = 0x1000;
> }
>
> + res = platform_get_resource(op, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&op->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> tmdev->map = devm_regmap_init_mmio(tmdev->dev, base, &tsens_config);
> - if (IS_ERR(tmdev->map)) {
> - iounmap(base);
> + if (IS_ERR(tmdev->map))
> return PTR_ERR(tmdev->map);
> - }
>
> return 0;
> }
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Wed, Jul 18, 2018 at 12:55:03PM +0530, Amit Kucheria wrote:
> The current code will always return 0xffffffff in case of negative
> temperatures due to a bug in how the binary sign extension is being done.
>
> Use sign_extend32() instead.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/thermal/qcom/tsens-v2.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 908e3dc..46abc21 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -5,19 +5,20 @@
> */
>
> #include <linux/regmap.h>
> +#include <linux/bitops.h>
> #include "tsens.h"
>
> #define STATUS_OFFSET 0xa0
> #define LAST_TEMP_MASK 0xfff
> #define STATUS_VALID_BIT BIT(21)
> -#define CODE_SIGN_BIT BIT(11)
>
> static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> {
> struct tsens_sensor *s = &tmdev->sensor[id];
> u32 code;
> unsigned int status_reg;
> - int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
> + u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
> + int ret;
>
> status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> ret = regmap_read(tmdev->map, status_reg, &code);
> @@ -54,12 +55,8 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> else if (last_temp2 == last_temp3)
> last_temp = last_temp3;
> done:
> - /* Code sign bit is the sign extension for a negative value */
> - if (last_temp & CODE_SIGN_BIT)
> - last_temp |= ~CODE_SIGN_BIT;
> -
> - /* Temperatures are in deciCelicius */
> - *temp = last_temp * 100;
> + /* Convert temperatures to milliCelicius */
nits:
s/temperatures/temperature/
s/milliCelicius/milliCelsius/
> + *temp = sign_extend32(last_temp, fls(LAST_TEMP_MASK) - 1) * 100;
>
> return 0;
> }
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Wed, Jul 18, 2018 at 12:55:01PM +0530, Amit Kucheria wrote:
> We're actually reading the temperature from the status register. Fix the
> variable name to reflect that.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/thermal/qcom/tsens-common.c | 6 +++---
> drivers/thermal/qcom/tsens-v2.c | 10 +++++-----
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index c22dc18..25e7f24 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -104,11 +104,11 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
> {
> struct tsens_sensor *s = &tmdev->sensor[id];
> u32 code;
> - unsigned int sensor_addr;
> + unsigned int status_reg;
> int last_temp = 0, ret;
>
> - sensor_addr = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
> - ret = regmap_read(tmdev->map, sensor_addr, &code);
> + status_reg = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
> + ret = regmap_read(tmdev->map, status_reg, &code);
> if (ret)
> return ret;
> last_temp = code & SN_ST_TEMP_MASK;
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index f40150f..908e3dc 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -16,11 +16,11 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> {
> struct tsens_sensor *s = &tmdev->sensor[id];
> u32 code;
> - unsigned int sensor_addr;
> + unsigned int status_reg;
> int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
>
> - sensor_addr = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> - ret = regmap_read(tmdev->map, sensor_addr, &code);
> + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> + ret = regmap_read(tmdev->map, status_reg, &code);
> if (ret)
> return ret;
> last_temp = code & LAST_TEMP_MASK;
> @@ -28,7 +28,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> goto done;
>
> /* Try a second time */
> - ret = regmap_read(tmdev->map, sensor_addr, &code);
> + ret = regmap_read(tmdev->map, status_reg, &code);
> if (ret)
> return ret;
> if (code & STATUS_VALID_BIT) {
> @@ -39,7 +39,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> }
>
> /* Try a third/last time */
> - ret = regmap_read(tmdev->map, sensor_addr, &code);
> + ret = regmap_read(tmdev->map, status_reg, &code);
> if (ret)
> return ret;
> if (code & STATUS_VALID_BIT) {
Reviewed-by: Matthias Kaehlcke <[email protected]>
On Thu, Jul 26, 2018 at 5:19 AM, Matthias Kaehlcke <[email protected]> wrote:
> On Wed, Jul 18, 2018 at 12:55:03PM +0530, Amit Kucheria wrote:
>> The current code will always return 0xffffffff in case of negative
>> temperatures due to a bug in how the binary sign extension is being done.
>>
>> Use sign_extend32() instead.
>>
>> Signed-off-by: Amit Kucheria <[email protected]>
>> ---
>> drivers/thermal/qcom/tsens-v2.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
>> index 908e3dc..46abc21 100644
>> --- a/drivers/thermal/qcom/tsens-v2.c
>> +++ b/drivers/thermal/qcom/tsens-v2.c
>> @@ -5,19 +5,20 @@
>> */
>>
>> #include <linux/regmap.h>
>> +#include <linux/bitops.h>
>> #include "tsens.h"
>>
>> #define STATUS_OFFSET 0xa0
>> #define LAST_TEMP_MASK 0xfff
>> #define STATUS_VALID_BIT BIT(21)
>> -#define CODE_SIGN_BIT BIT(11)
>>
>> static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
>> {
>> struct tsens_sensor *s = &tmdev->sensor[id];
>> u32 code;
>> unsigned int status_reg;
>> - int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
>> + u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
>> + int ret;
>>
>> status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
>> ret = regmap_read(tmdev->map, status_reg, &code);
>> @@ -54,12 +55,8 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
>> else if (last_temp2 == last_temp3)
>> last_temp = last_temp3;
>> done:
>> - /* Code sign bit is the sign extension for a negative value */
>> - if (last_temp & CODE_SIGN_BIT)
>> - last_temp |= ~CODE_SIGN_BIT;
>> -
>> - /* Temperatures are in deciCelicius */
>> - *temp = last_temp * 100;
>> + /* Convert temperatures to milliCelicius */
>
> nits:
>
> s/temperatures/temperature/
> s/milliCelicius/milliCelsius/
Embarrassing to have two typos in a single sentence. :-)
>> + *temp = sign_extend32(last_temp, fls(LAST_TEMP_MASK) - 1) * 100;
>>
>> return 0;
>> }
>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
Thanks for the review.