2018-08-28 13:40:26

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 00/11] Another round of tsens cleanups

This is another series of tsens cleanups before we add interrupt support. This applies on top of 4.19-rc1.

In this series, we have the following:
- splitup 8916 and 8974 register address spaces for SROT and TM
- cleanups: move to spdx, dead code removal, removal of id field
- Add support to map the SROT address space for DTs that list it separately
- Check if TSENS IP is enabled in firmware by querying the SROT space
- Add reg-names property to get more readable outputs in /proc/iomem
- Add myself as maintainer of tsens

Changes since v1:
- Split up changes that split the address space and added qcom,sensors
property into two separate patches
- Remove brackets in typo correction patch

Amit Kucheria (11):
arm/arm64: dts: msm8974/msm8916: thermal: Split address space into two
arm/arm64: dts: msm8974/msm8916: thermal: Add "qcom,sensors" property
dt-bindings: thermal: Fix a typo in documentation
thermal: tsens: Add SPDX license identifiers
thermal: tsens: Get rid of dead code
thermal: tsens: Rename map field in order to add a second address map
thermal: tsens: Add the SROT address map
thermal: tsens: Check if the IP is correctly enabled by firmware
thermal: tsens: Get rid of 'id' field
arm64: dts: qcom: Add reg-names for all tsens nodes
MAINTAINERS: Add entry for Qualcomm TSENS thermal drivers

.../devicetree/bindings/thermal/thermal.txt | 2 +-
MAINTAINERS | 7 +++
arch/arm/boot/dts/qcom-msm8974.dtsi | 7 ++-
arch/arm64/boot/dts/qcom/msm8916.dtsi | 7 ++-
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +
drivers/thermal/qcom/tsens-8916.c | 11 +---
drivers/thermal/qcom/tsens-8960.c | 41 +++++-------
drivers/thermal/qcom/tsens-8974.c | 11 +---
drivers/thermal/qcom/tsens-common.c | 62 ++++++++++++-------
drivers/thermal/qcom/tsens-v2.c | 6 +-
drivers/thermal/qcom/tsens.c | 21 +------
drivers/thermal/qcom/tsens.h | 24 +++----
13 files changed, 99 insertions(+), 104 deletions(-)

--
2.17.1



2018-08-28 13:40:31

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 01/11] arm/arm64: dts: msm8974/msm8916: thermal: Split address space into two

We've earlier added support to split the register address space into TM
and SROT regions.

Split up the regmap address space into two for the remaining platforms
that have a similar register layout and make corresponding changes to
the get_temp_common() function used by these platforms.

Since tsens-common.c/init_common() currently only registers one address
space, the order is important (TM before SROT). This is OK since the
code doesn't really use the SROT functionality yet.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 5 +++--
arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++--
drivers/thermal/qcom/tsens-common.c | 5 +++--
3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d9019a49b292..56dbbf788d15 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -427,9 +427,10 @@
};
};

- tsens: thermal-sensor@fc4a8000 {
+ tsens: thermal-sensor@fc4a9000 {
compatible = "qcom,msm8974-tsens";
- reg = <0xfc4a8000 0x2000>;
+ reg = <0xfc4a9000 0x1000>, /* TM */
+ <0xfc4a8000 0x1000>; /* SROT */
nvmem-cells = <&tsens_calib>, <&tsens_backup>;
nvmem-cell-names = "calib", "calib_backup";
#thermal-sensor-cells = <1>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 7b32b8990d62..6a277fce3333 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -761,9 +761,10 @@
};
};

- tsens: thermal-sensor@4a8000 {
+ tsens: thermal-sensor@4a9000 {
compatible = "qcom,msm8916-tsens";
- reg = <0x4a8000 0x2000>;
+ reg = <0x4a9000 0x1000>, /* TM */
+ <0x4a8000 0x1000>; /* SROT */
nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
nvmem-cell-names = "calib", "calib_sel";
#thermal-sensor-cells = <1>;
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 6207d8d92351..478739543bbc 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -21,7 +21,7 @@
#include <linux/regmap.h>
#include "tsens.h"

-#define S0_ST_ADDR 0x1030
+#define STATUS_OFFSET 0x30
#define SN_ADDR_OFFSET 0x4
#define SN_ST_TEMP_MASK 0x3ff
#define CAL_DEGC_PT1 30
@@ -107,8 +107,9 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
unsigned int status_reg;
int last_temp = 0, ret;

- status_reg = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
+ status_reg = tmdev->tm_offset + STATUS_OFFSET + 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;
--
2.17.1


2018-08-28 13:40:44

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 02/11] arm/arm64: dts: msm8974/msm8916: thermal: Add "qcom,sensors" property

This new property allows the number of sensors to be configured from DT
instead of being hardcoded in platform data. Use it.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 56dbbf788d15..3c4b81c29798 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -433,6 +433,7 @@
<0xfc4a8000 0x1000>; /* SROT */
nvmem-cells = <&tsens_calib>, <&tsens_backup>;
nvmem-cell-names = "calib", "calib_backup";
+ #qcom,sensors = <11>;
#thermal-sensor-cells = <1>;
};

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 6a277fce3333..be27d8dc9e6b 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -767,6 +767,7 @@
<0x4a8000 0x1000>; /* SROT */
nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
nvmem-cell-names = "calib", "calib_sel";
+ #qcom,sensors = <5>;
#thermal-sensor-cells = <1>;
};

--
2.17.1


2018-08-28 13:40:51

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 03/11] dt-bindings: thermal: Fix a typo in documentation

c(1) + x(1) was actually meant to be c(1) * x(1).

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/thermal/thermal.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
index eb7ee91556a5..ca14ba959e0d 100644
--- a/Documentation/devicetree/bindings/thermal/thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/thermal.txt
@@ -152,7 +152,7 @@ Optional property:
Elem size: one cell the sensors listed in the thermal-sensors property.
Elem type: signed Coefficients defaults to 1, in case this property
is not specified. A simple linear polynomial is used:
- Z = c0 * x0 + c1 + x1 + ... + c(n-1) * x(n-1) + cn.
+ Z = c0 * x0 + c1 * x1 + ... + c(n-1) * x(n-1) + cn.

The coefficients are ordered and they match with sensors
by means of sensor ID. Additional coefficients are
--
2.17.1


2018-08-28 13:41:01

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 04/11] thermal: tsens: Add SPDX license identifiers

The TSENS drivers use a GPL-2.0 license. Replace with equivalent SPDX
tags and delete the full license text.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens-8916.c | 11 +----------
drivers/thermal/qcom/tsens-8960.c | 11 +----------
drivers/thermal/qcom/tsens-8974.c | 11 +----------
drivers/thermal/qcom/tsens-common.c | 11 +----------
drivers/thermal/qcom/tsens.c | 11 +----------
drivers/thermal/qcom/tsens.h | 11 ++---------
6 files changed, 7 insertions(+), 59 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8916.c b/drivers/thermal/qcom/tsens-8916.c
index fdf561b8b81d..c4955c85e922 100644
--- a/drivers/thermal/qcom/tsens-8916.c
+++ b/drivers/thermal/qcom/tsens-8916.c
@@ -1,15 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
*/

#include <linux/platform_device.h>
diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 0451277d3a8f..4af76de7dc2e 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -1,15 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
*/

#include <linux/platform_device.h>
diff --git a/drivers/thermal/qcom/tsens-8974.c b/drivers/thermal/qcom/tsens-8974.c
index 9baf77e8cbe3..7e149edbfeb6 100644
--- a/drivers/thermal/qcom/tsens-8974.c
+++ b/drivers/thermal/qcom/tsens-8974.c
@@ -1,15 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
*/

#include <linux/platform_device.h>
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 478739543bbc..303e3fdaca98 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -1,15 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
*/

#include <linux/err.h>
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index a2c9bfae3d86..90bb431cf740 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -1,15 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
*/

#include <linux/err.h>
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 14331eb45a86..8207610f326a 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -1,15 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
+
#ifndef __QCOM_TSENS_H__
#define __QCOM_TSENS_H__

--
2.17.1


2018-08-28 13:41:12

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 05/11] thermal: tsens: Get rid of dead code

hw_id is dynamically allocated but not used anywhere. Get rid of dead
code.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 90bb431cf740..9a8e8f7b4ae1 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -80,11 +80,6 @@ static int tsens_register(struct tsens_device *tmdev)
{
int i;
struct thermal_zone_device *tzd;
- u32 *hw_id, n = tmdev->num_sensors;
-
- hw_id = devm_kcalloc(tmdev->dev, n, sizeof(u32), GFP_KERNEL);
- if (!hw_id)
- return -ENOMEM;

for (i = 0; i < tmdev->num_sensors; i++) {
tmdev->sensor[i].tmdev = tmdev;
--
2.17.1


2018-08-28 13:41:19

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 06/11] thermal: tsens: Rename map field in order to add a second address map

The TSENS driver currently only uses a limited set of registers from the TM
address space. So it was ok to map just that set of registers and call it
"map".

We'd now like to map a second set: SROT registers to introduce new
functionality. Rename the "map" field to a more appropriate "tm_map".

The 8960 doesn't have a clear split between TM and SROT registers. To avoid
complicating the data structure, it will switchover to using tm_map for its
maps.

There is no functional change with this patch.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens-8960.c | 30 ++++++++++++++---------------
drivers/thermal/qcom/tsens-common.c | 17 ++++++++--------
drivers/thermal/qcom/tsens-v2.c | 6 +++---
drivers/thermal/qcom/tsens.h | 2 +-
4 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 4af76de7dc2e..0f0adb302a7b 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -60,7 +60,7 @@ static int suspend_8960(struct tsens_device *tmdev)
{
int ret;
unsigned int mask;
- struct regmap *map = tmdev->map;
+ struct regmap *map = tmdev->tm_map;

ret = regmap_read(map, THRESHOLD_ADDR, &tmdev->ctx.threshold);
if (ret)
@@ -85,7 +85,7 @@ static int suspend_8960(struct tsens_device *tmdev)
static int resume_8960(struct tsens_device *tmdev)
{
int ret;
- struct regmap *map = tmdev->map;
+ struct regmap *map = tmdev->tm_map;

ret = regmap_update_bits(map, CNTL_ADDR, SW_RST, SW_RST);
if (ret)
@@ -117,12 +117,12 @@ static int enable_8960(struct tsens_device *tmdev, int id)
int ret;
u32 reg, mask;

- ret = regmap_read(tmdev->map, CNTL_ADDR, &reg);
+ ret = regmap_read(tmdev->tm_map, CNTL_ADDR, &reg);
if (ret)
return ret;

mask = BIT(id + SENSOR0_SHIFT);
- ret = regmap_write(tmdev->map, CNTL_ADDR, reg | SW_RST);
+ ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg | SW_RST);
if (ret)
return ret;

@@ -131,7 +131,7 @@ static int enable_8960(struct tsens_device *tmdev, int id)
else
reg |= mask | SLP_CLK_ENA_8660 | EN;

- ret = regmap_write(tmdev->map, CNTL_ADDR, reg);
+ ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg);
if (ret)
return ret;

@@ -148,7 +148,7 @@ static void disable_8960(struct tsens_device *tmdev)
mask <<= SENSOR0_SHIFT;
mask |= EN;

- ret = regmap_read(tmdev->map, CNTL_ADDR, &reg_cntl);
+ ret = regmap_read(tmdev->tm_map, CNTL_ADDR, &reg_cntl);
if (ret)
return;

@@ -159,7 +159,7 @@ static void disable_8960(struct tsens_device *tmdev)
else
reg_cntl &= ~SLP_CLK_ENA_8660;

- regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
+ regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
}

static int init_8960(struct tsens_device *tmdev)
@@ -167,8 +167,8 @@ static int init_8960(struct tsens_device *tmdev)
int ret, i;
u32 reg_cntl;

- tmdev->map = dev_get_regmap(tmdev->dev, NULL);
- if (!tmdev->map)
+ tmdev->tm_map = dev_get_regmap(tmdev->dev, NULL);
+ if (!tmdev->tm_map)
return -ENODEV;

/*
@@ -184,14 +184,14 @@ static int init_8960(struct tsens_device *tmdev)
}

reg_cntl = SW_RST;
- ret = regmap_update_bits(tmdev->map, CNTL_ADDR, SW_RST, reg_cntl);
+ ret = regmap_update_bits(tmdev->tm_map, CNTL_ADDR, SW_RST, reg_cntl);
if (ret)
return ret;

if (tmdev->num_sensors > 1) {
reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18);
reg_cntl &= ~SW_RST;
- ret = regmap_update_bits(tmdev->map, CONFIG_ADDR,
+ ret = regmap_update_bits(tmdev->tm_map, CONFIG_ADDR,
CONFIG_MASK, CONFIG);
} else {
reg_cntl |= SLP_CLK_ENA_8660 | (MEASURE_PERIOD << 16);
@@ -200,12 +200,12 @@ static int init_8960(struct tsens_device *tmdev)
}

reg_cntl |= GENMASK(tmdev->num_sensors - 1, 0) << SENSOR0_SHIFT;
- ret = regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
+ ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
if (ret)
return ret;

reg_cntl |= EN;
- ret = regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
+ ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
if (ret)
return ret;

@@ -252,12 +252,12 @@ static int get_temp_8960(struct tsens_device *tmdev, int id, int *temp)

timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
do {
- ret = regmap_read(tmdev->map, INT_STATUS_ADDR, &trdy);
+ ret = regmap_read(tmdev->tm_map, INT_STATUS_ADDR, &trdy);
if (ret)
return ret;
if (!(trdy & TRDY_MASK))
continue;
- ret = regmap_read(tmdev->map, s->status, &code);
+ ret = regmap_read(tmdev->tm_map, s->status, &code);
if (ret)
return ret;
*temp = code_to_mdegC(code, s);
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 303e3fdaca98..0585084630b3 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -99,8 +99,7 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
int last_temp = 0, ret;

status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * SN_ADDR_OFFSET;
- ret = regmap_read(tmdev->map, status_reg, &code);
-
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
if (ret)
return ret;
last_temp = code & SN_ST_TEMP_MASK;
@@ -118,7 +117,7 @@ static const struct regmap_config tsens_config = {

int __init init_common(struct tsens_device *tmdev)
{
- void __iomem *base;
+ void __iomem *tm_base;
struct resource *res;
struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);

@@ -134,13 +133,13 @@ int __init init_common(struct tsens_device *tmdev)
}

res = platform_get_resource(op, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&op->dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ tm_base = devm_ioremap_resource(&op->dev, res);
+ if (IS_ERR(tm_base))
+ return PTR_ERR(tm_base);

- tmdev->map = devm_regmap_init_mmio(tmdev->dev, base, &tsens_config);
- if (IS_ERR(tmdev->map))
- return PTR_ERR(tmdev->map);
+ tmdev->tm_map = devm_regmap_init_mmio(tmdev->dev, tm_base, &tsens_config);
+ if (IS_ERR(tmdev->tm_map))
+ return PTR_ERR(tmdev->tm_map);

return 0;
}
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 44da02f594ac..1bdef92e4521 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -21,7 +21,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
int ret;

status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
- ret = regmap_read(tmdev->map, status_reg, &code);
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
if (ret)
return ret;
last_temp = code & LAST_TEMP_MASK;
@@ -29,7 +29,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, status_reg, &code);
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
if (ret)
return ret;
if (code & STATUS_VALID_BIT) {
@@ -40,7 +40,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, status_reg, &code);
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
if (ret)
return ret;
if (code & STATUS_VALID_BIT) {
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 8207610f326a..58e98c4d3a8b 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -69,7 +69,7 @@ struct tsens_context {
struct tsens_device {
struct device *dev;
u32 num_sensors;
- struct regmap *map;
+ struct regmap *tm_map;
u32 tm_offset;
struct tsens_context ctx;
const struct tsens_ops *ops;
--
2.17.1


2018-08-28 13:41:35

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 07/11] thermal: tsens: Add the SROT address map

On platforms whose device trees specify two address spaces for TSENS, the
second one points to the SROT registers. Initialise the SROT map on those
platforms.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 14 ++++++++++++--
drivers/thermal/qcom/tsens.h | 1 +
2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 0585084630b3..0b8a793f15f4 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -117,16 +117,26 @@ static const struct regmap_config tsens_config = {

int __init init_common(struct tsens_device *tmdev)
{
- void __iomem *tm_base;
+ void __iomem *tm_base, *srot_base;
struct resource *res;
struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);

if (!op)
return -EINVAL;

- /* The driver only uses the TM register address space for now */
if (op->num_resources > 1) {
+ /* DT with separate SROT and TM address space */
tmdev->tm_offset = 0;
+ res = platform_get_resource(op, IORESOURCE_MEM, 1);
+ srot_base = devm_ioremap_resource(&op->dev, res);
+ if (IS_ERR(srot_base))
+ return PTR_ERR(srot_base);
+
+ tmdev->srot_map = devm_regmap_init_mmio(tmdev->dev,
+ srot_base, &tsens_config);
+ if (IS_ERR(tmdev->srot_map))
+ return PTR_ERR(tmdev->srot_map);
+
} else {
/* old DTs where SROT and TM were in a contiguous 2K block */
tmdev->tm_offset = 0x1000;
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 58e98c4d3a8b..b9c4bcf255fa 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -70,6 +70,7 @@ struct tsens_device {
struct device *dev;
u32 num_sensors;
struct regmap *tm_map;
+ struct regmap *srot_map;
u32 tm_offset;
struct tsens_context ctx;
const struct tsens_ops *ops;
--
2.17.1


2018-08-28 13:41:45

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 09/11] thermal: tsens: Get rid of 'id' field

The hw_id field in 'struct tsens_sensor' can do the job of tracking
unique ids for each sensor connected to each tsens device instance. It
also allows hw_ids to be overridden (e.g. 8916) in cases where some
sensors in a sequence are disabled on a particular platform.

Use the hw_id field instead of the id field consistently across the
tsens code.

While we're at it, document the fields of struct tsens_sensor.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens.c | 5 ++---
drivers/thermal/qcom/tsens.h | 10 +++++++++-
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 9a8e8f7b4ae1..fb728ec5d77f 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -17,7 +17,7 @@ static int tsens_get_temp(void *data, int *temp)
const struct tsens_sensor *s = data;
struct tsens_device *tmdev = s->tmdev;

- return tmdev->ops->get_temp(tmdev, s->id, temp);
+ return tmdev->ops->get_temp(tmdev, s->hw_id, temp);
}

static int tsens_get_trend(void *p, int trip, enum thermal_trend *trend)
@@ -26,7 +26,7 @@ static int tsens_get_trend(void *p, int trip, enum thermal_trend *trend)
struct tsens_device *tmdev = s->tmdev;

if (tmdev->ops->get_trend)
- return tmdev->ops->get_trend(tmdev, s->id, trend);
+ return tmdev->ops->get_trend(tmdev, s->hw_id, trend);

return -ENOTSUPP;
}
@@ -83,7 +83,6 @@ static int tsens_register(struct tsens_device *tmdev)

for (i = 0; i < tmdev->num_sensors; i++) {
tmdev->sensor[i].tmdev = tmdev;
- tmdev->sensor[i].id = i;
tzd = devm_thermal_zone_of_sensor_register(tmdev->dev, i,
&tmdev->sensor[i],
&tsens_of_ops);
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index b9c4bcf255fa..2a3174dfc1a9 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -14,11 +14,19 @@

struct tsens_device;

+/**
+ * struct tsens_sensor - sensor-specific data
+ * @tmdev: tsens device instance this sensor is connected to
+ * @tzd: thermal zone corresponding to this sensor
+ * @offset: offset from calibration data to convert ADC data to degrees
+ * @hw_id: unique sensor ID for each sensor connected to tsens device instance
+ * @slope: slope from calibration data to convert ADC data to degrees
+ * @status: 8960-specific status register addresses
+ */
struct tsens_sensor {
struct tsens_device *tmdev;
struct thermal_zone_device *tzd;
int offset;
- int id;
int hw_id;
int slope;
u32 status;
--
2.17.1


2018-08-28 13:42:06

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 08/11] thermal: tsens: Check if the IP is correctly enabled by firmware

The SROT registers are initialised by the secure firmware at boot. We
don't have write access to the registers. Check if the block is enabled
before continuing.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 0b8a793f15f4..d250b757d1f0 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -12,6 +12,11 @@
#include <linux/regmap.h>
#include "tsens.h"

+/* SROT */
+#define CTRL_OFFSET 0x4
+#define TSENS_EN BIT(0)
+
+/* TM */
#define STATUS_OFFSET 0x30
#define SN_ADDR_OFFSET 0x4
#define SN_ST_TEMP_MASK 0x3ff
@@ -119,6 +124,8 @@ int __init init_common(struct tsens_device *tmdev)
{
void __iomem *tm_base, *srot_base;
struct resource *res;
+ u32 code;
+ int ret;
struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);

if (!op)
@@ -151,5 +158,15 @@ int __init init_common(struct tsens_device *tmdev)
if (IS_ERR(tmdev->tm_map))
return PTR_ERR(tmdev->tm_map);

+ if (tmdev->srot_map) {
+ ret = regmap_read(tmdev->srot_map, CTRL_OFFSET, &code);
+ if (ret)
+ return ret;
+ if (!(code & TSENS_EN)) {
+ dev_err(tmdev->dev, "tsens device is not enabled\n");
+ return -ENODEV;
+ }
+ }
+
return 0;
}
--
2.17.1


2018-08-28 13:42:24

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 10/11] arm64: dts: qcom: Add reg-names for all tsens nodes

Instead of showing up as thermal-sensor@<addr>, the nodes will show up as
tsens0_tm, tsen1_tm, tsens1_srot, etc. in /proc/iomem making it easier to
read.

IOW,

0c222000-0c2221fe : thermal-sensor@c263000
0c223000-0c2231fe : thermal-sensor@c265000
0c263000-0c2631fe : thermal-sensor@c263000
0c265000-0c2651fe : thermal-sensor@c265000

becomes

0c222000-0c2221fe : tsens0_srot
0c223000-0c2231fe : tsens1_srot
0c263000-0c2631fe : tsens0_tm
0c265000-0c2651fe : tsens1_tm

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++
4 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 3c4b81c29798..64c9f81ddd90 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -431,6 +431,7 @@
compatible = "qcom,msm8974-tsens";
reg = <0xfc4a9000 0x1000>, /* TM */
<0xfc4a8000 0x1000>; /* SROT */
+ reg-names = "tsens_tm", "tsens_srot";
nvmem-cells = <&tsens_calib>, <&tsens_backup>;
nvmem-cell-names = "calib", "calib_backup";
#qcom,sensors = <11>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index be27d8dc9e6b..c172731625f3 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -765,6 +765,7 @@
compatible = "qcom,msm8916-tsens";
reg = <0x4a9000 0x1000>, /* TM */
<0x4a8000 0x1000>; /* SROT */
+ reg-names = "tsens_tm", "tsens_srot";
nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
nvmem-cell-names = "calib", "calib_sel";
#qcom,sensors = <5>;
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index cd3865e7a270..ac1d30006e6a 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -381,6 +381,7 @@
compatible = "qcom,msm8996-tsens";
reg = <0x4a9000 0x1000>, /* TM */
<0x4a8000 0x1000>; /* SROT */
+ reg-names = "tsens0_tm", "tsens0_srot";
#qcom,sensors = <13>;
#thermal-sensor-cells = <1>;
};
@@ -389,6 +390,7 @@
compatible = "qcom,msm8996-tsens";
reg = <0x4ad000 0x1000>, /* TM */
<0x4ac000 0x1000>; /* SROT */
+ reg-names = "tsens1_tm", "tsens1_srot";
#qcom,sensors = <8>;
#thermal-sensor-cells = <1>;
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa6a1b5..f1a36d6829cf 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -966,6 +966,7 @@
compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
reg = <0xc263000 0x1ff>, /* TM */
<0xc222000 0x1ff>; /* SROT */
+ reg-names = "tsens0_tm", "tsens0_srot";
#qcom,sensors = <13>;
#thermal-sensor-cells = <1>;
};
@@ -974,6 +975,7 @@
compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
reg = <0xc265000 0x1ff>, /* TM */
<0xc223000 0x1ff>; /* SROT */
+ reg-names = "tsens1_tm", "tsens1_srot";
#qcom,sensors = <8>;
#thermal-sensor-cells = <1>;
};
--
2.17.1


2018-08-28 13:42:35

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 11/11] MAINTAINERS: Add entry for Qualcomm TSENS thermal drivers

Create an entry for the TSENS drivers and mark them as maintained

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Matthias Kaehlcke <[email protected]>
Acked-by: Rajendra Nayak <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a5b256b25905..b8c96e0699c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12076,6 +12076,13 @@ L: [email protected]
S: Maintained
F: drivers/iommu/qcom_iommu.c

+QUALCOMM TSENS THERMAL DRIVER
+M: Amit Kucheria <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Maintained
+F: drivers/thermal/qcom/
+
QUALCOMM VENUS VIDEO ACCELERATOR DRIVER
M: Stanimir Varbanov <[email protected]>
L: [email protected]
--
2.17.1


2018-08-30 19:25:40

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 08/11] thermal: tsens: Check if the IP is correctly enabled by firmware

On Tue, Aug 28, 2018 at 7:08 PM, Amit Kucheria <[email protected]> wrote:
> The SROT registers are initialised by the secure firmware at boot. We
> don't have write access to the registers. Check if the block is enabled
> before continuing.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
> drivers/thermal/qcom/tsens-common.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 0b8a793f15f4..d250b757d1f0 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -12,6 +12,11 @@
> #include <linux/regmap.h>
> #include "tsens.h"
>
> +/* SROT */
> +#define CTRL_OFFSET 0x4

This OFFSET is not constant across the TSENS family and breaks atleast
one platform in the test below. I have a patch to fix this for all
platforms, will post it after a bit more testing.


> +#define TSENS_EN BIT(0)
> +
> +/* TM */
> #define STATUS_OFFSET 0x30
> #define SN_ADDR_OFFSET 0x4
> #define SN_ST_TEMP_MASK 0x3ff
> @@ -119,6 +124,8 @@ int __init init_common(struct tsens_device *tmdev)
> {
> void __iomem *tm_base, *srot_base;
> struct resource *res;
> + u32 code;
> + int ret;
> struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
>
> if (!op)
> @@ -151,5 +158,15 @@ int __init init_common(struct tsens_device *tmdev)
> if (IS_ERR(tmdev->tm_map))
> return PTR_ERR(tmdev->tm_map);
>
> + if (tmdev->srot_map) {
> + ret = regmap_read(tmdev->srot_map, CTRL_OFFSET, &code);
> + if (ret)
> + return ret;
> + if (!(code & TSENS_EN)) {
> + dev_err(tmdev->dev, "tsens device is not enabled\n");
> + return -ENODEV;
> + }
> + }
> +
> return 0;
> }
> --
> 2.17.1
>

2018-09-03 20:01:02

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arm/arm64: dts: msm8974/msm8916: thermal: Split address space into two

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> We've earlier added support to split the register address space into TM
> and SROT regions.
>
> Split up the regmap address space into two for the remaining platforms
> that have a similar register layout and make corresponding changes to
> the get_temp_common() function used by these platforms.
>
> Since tsens-common.c/init_common() currently only registers one address
> space, the order is important (TM before SROT). This is OK since the
> code doesn't really use the SROT functionality yet.
>

Having a single patch touching both code and dts will cause merge issues
as this patch travel upstream. Even more arm-soc expects arm and arm64
dts changes to come in different pull requests.

Please split it so that the three pieces can be picked up by respective
maintainer.

> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 5 +++--
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++--
> drivers/thermal/qcom/tsens-common.c | 5 +++--
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index d9019a49b292..56dbbf788d15 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -427,9 +427,10 @@
> };
> };
>
> - tsens: thermal-sensor@fc4a8000 {
> + tsens: thermal-sensor@fc4a9000 {
> compatible = "qcom,msm8974-tsens";
> - reg = <0xfc4a8000 0x2000>;
> + reg = <0xfc4a9000 0x1000>, /* TM */
> + <0xfc4a8000 0x1000>; /* SROT */
> nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> nvmem-cell-names = "calib", "calib_backup";
> #thermal-sensor-cells = <1>;
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 7b32b8990d62..6a277fce3333 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -761,9 +761,10 @@
> };
> };
>
> - tsens: thermal-sensor@4a8000 {
> + tsens: thermal-sensor@4a9000 {
> compatible = "qcom,msm8916-tsens";
> - reg = <0x4a8000 0x2000>;
> + reg = <0x4a9000 0x1000>, /* TM */
> + <0x4a8000 0x1000>; /* SROT */
> nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
> nvmem-cell-names = "calib", "calib_sel";
> #thermal-sensor-cells = <1>;
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 6207d8d92351..478739543bbc 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -21,7 +21,7 @@
> #include <linux/regmap.h>
> #include "tsens.h"
>
> -#define S0_ST_ADDR 0x1030
> +#define STATUS_OFFSET 0x30
> #define SN_ADDR_OFFSET 0x4
> #define SN_ST_TEMP_MASK 0x3ff
> #define CAL_DEGC_PT1 30
> @@ -107,8 +107,9 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
> unsigned int status_reg;
> int last_temp = 0, ret;
>
> - status_reg = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
> + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * SN_ADDR_OFFSET;

Wasn't this change part of the previous set that introduced the
tm_offset? If not how did we handle the fact that tmdev->map is already
indented 0x1000 bytes?

Both changes looks good, but I'm worries about the order of things.

Regards,
Bjorn

> ret = regmap_read(tmdev->map, status_reg, &code);
> +
> if (ret)
> return ret;
> last_temp = code & SN_ST_TEMP_MASK;
> --
> 2.17.1
>

2018-09-03 20:01:24

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 02/11] arm/arm64: dts: msm8974/msm8916: thermal: Add "qcom,sensors" property

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> This new property allows the number of sensors to be configured from DT
> instead of being hardcoded in platform data. Use it.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Please split in arm and arm64.

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 56dbbf788d15..3c4b81c29798 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -433,6 +433,7 @@
> <0xfc4a8000 0x1000>; /* SROT */
> nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> nvmem-cell-names = "calib", "calib_backup";
> + #qcom,sensors = <11>;
> #thermal-sensor-cells = <1>;
> };
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 6a277fce3333..be27d8dc9e6b 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -767,6 +767,7 @@
> <0x4a8000 0x1000>; /* SROT */
> nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
> nvmem-cell-names = "calib", "calib_sel";
> + #qcom,sensors = <5>;
> #thermal-sensor-cells = <1>;
> };
>
> --
> 2.17.1
>

2018-09-03 20:01:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 03/11] dt-bindings: thermal: Fix a typo in documentation

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> c(1) + x(1) was actually meant to be c(1) * x(1).
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Acked-by: Rob Herring <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> Documentation/devicetree/bindings/thermal/thermal.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> index eb7ee91556a5..ca14ba959e0d 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> @@ -152,7 +152,7 @@ Optional property:
> Elem size: one cell the sensors listed in the thermal-sensors property.
> Elem type: signed Coefficients defaults to 1, in case this property
> is not specified. A simple linear polynomial is used:
> - Z = c0 * x0 + c1 + x1 + ... + c(n-1) * x(n-1) + cn.
> + Z = c0 * x0 + c1 * x1 + ... + c(n-1) * x(n-1) + cn.
>
> The coefficients are ordered and they match with sensors
> by means of sensor ID. Additional coefficients are
> --
> 2.17.1
>

2018-09-03 20:02:37

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 04/11] thermal: tsens: Add SPDX license identifiers

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> The TSENS drivers use a GPL-2.0 license. Replace with equivalent SPDX
> tags and delete the full license text.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/thermal/qcom/tsens-8916.c | 11 +----------
> drivers/thermal/qcom/tsens-8960.c | 11 +----------
> drivers/thermal/qcom/tsens-8974.c | 11 +----------
> drivers/thermal/qcom/tsens-common.c | 11 +----------
> drivers/thermal/qcom/tsens.c | 11 +----------
> drivers/thermal/qcom/tsens.h | 11 ++---------
> 6 files changed, 7 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-8916.c b/drivers/thermal/qcom/tsens-8916.c
> index fdf561b8b81d..c4955c85e922 100644
> --- a/drivers/thermal/qcom/tsens-8916.c
> +++ b/drivers/thermal/qcom/tsens-8916.c
> @@ -1,15 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> */
>
> #include <linux/platform_device.h>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 0451277d3a8f..4af76de7dc2e 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -1,15 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> */
>
> #include <linux/platform_device.h>
> diff --git a/drivers/thermal/qcom/tsens-8974.c b/drivers/thermal/qcom/tsens-8974.c
> index 9baf77e8cbe3..7e149edbfeb6 100644
> --- a/drivers/thermal/qcom/tsens-8974.c
> +++ b/drivers/thermal/qcom/tsens-8974.c
> @@ -1,15 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> */
>
> #include <linux/platform_device.h>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 478739543bbc..303e3fdaca98 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -1,15 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> */
>
> #include <linux/err.h>
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index a2c9bfae3d86..90bb431cf740 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -1,15 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 and
> - * only version 2 as published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> */
>
> #include <linux/err.h>
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 14331eb45a86..8207610f326a 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -1,15 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> /*
> * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> - *
> - * This software is licensed under the terms of the GNU General Public
> - * License version 2, as published by the Free Software Foundation, and
> - * may be copied, distributed, and modified under those terms.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> */
> +
> #ifndef __QCOM_TSENS_H__
> #define __QCOM_TSENS_H__
>
> --
> 2.17.1
>

2018-09-03 20:20:07

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 05/11] thermal: tsens: Get rid of dead code

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> hw_id is dynamically allocated but not used anywhere. Get rid of dead
> code.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Good catch.

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/thermal/qcom/tsens.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 90bb431cf740..9a8e8f7b4ae1 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -80,11 +80,6 @@ static int tsens_register(struct tsens_device *tmdev)
> {
> int i;
> struct thermal_zone_device *tzd;
> - u32 *hw_id, n = tmdev->num_sensors;
> -
> - hw_id = devm_kcalloc(tmdev->dev, n, sizeof(u32), GFP_KERNEL);
> - if (!hw_id)
> - return -ENOMEM;
>
> for (i = 0; i < tmdev->num_sensors; i++) {
> tmdev->sensor[i].tmdev = tmdev;
> --
> 2.17.1
>

2018-09-03 20:21:18

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 06/11] thermal: tsens: Rename map field in order to add a second address map

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> The TSENS driver currently only uses a limited set of registers from the TM
> address space. So it was ok to map just that set of registers and call it
> "map".
>
> We'd now like to map a second set: SROT registers to introduce new
> functionality. Rename the "map" field to a more appropriate "tm_map".
>
> The 8960 doesn't have a clear split between TM and SROT registers. To avoid
> complicating the data structure, it will switchover to using tm_map for its
> maps.
>
> There is no functional change with this patch.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/thermal/qcom/tsens-8960.c | 30 ++++++++++++++---------------
> drivers/thermal/qcom/tsens-common.c | 17 ++++++++--------
> drivers/thermal/qcom/tsens-v2.c | 6 +++---
> drivers/thermal/qcom/tsens.h | 2 +-
> 4 files changed, 27 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 4af76de7dc2e..0f0adb302a7b 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -60,7 +60,7 @@ static int suspend_8960(struct tsens_device *tmdev)
> {
> int ret;
> unsigned int mask;
> - struct regmap *map = tmdev->map;
> + struct regmap *map = tmdev->tm_map;
>
> ret = regmap_read(map, THRESHOLD_ADDR, &tmdev->ctx.threshold);
> if (ret)
> @@ -85,7 +85,7 @@ static int suspend_8960(struct tsens_device *tmdev)
> static int resume_8960(struct tsens_device *tmdev)
> {
> int ret;
> - struct regmap *map = tmdev->map;
> + struct regmap *map = tmdev->tm_map;
>
> ret = regmap_update_bits(map, CNTL_ADDR, SW_RST, SW_RST);
> if (ret)
> @@ -117,12 +117,12 @@ static int enable_8960(struct tsens_device *tmdev, int id)
> int ret;
> u32 reg, mask;
>
> - ret = regmap_read(tmdev->map, CNTL_ADDR, &reg);
> + ret = regmap_read(tmdev->tm_map, CNTL_ADDR, &reg);
> if (ret)
> return ret;
>
> mask = BIT(id + SENSOR0_SHIFT);
> - ret = regmap_write(tmdev->map, CNTL_ADDR, reg | SW_RST);
> + ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg | SW_RST);
> if (ret)
> return ret;
>
> @@ -131,7 +131,7 @@ static int enable_8960(struct tsens_device *tmdev, int id)
> else
> reg |= mask | SLP_CLK_ENA_8660 | EN;
>
> - ret = regmap_write(tmdev->map, CNTL_ADDR, reg);
> + ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg);
> if (ret)
> return ret;
>
> @@ -148,7 +148,7 @@ static void disable_8960(struct tsens_device *tmdev)
> mask <<= SENSOR0_SHIFT;
> mask |= EN;
>
> - ret = regmap_read(tmdev->map, CNTL_ADDR, &reg_cntl);
> + ret = regmap_read(tmdev->tm_map, CNTL_ADDR, &reg_cntl);
> if (ret)
> return;
>
> @@ -159,7 +159,7 @@ static void disable_8960(struct tsens_device *tmdev)
> else
> reg_cntl &= ~SLP_CLK_ENA_8660;
>
> - regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
> + regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
> }
>
> static int init_8960(struct tsens_device *tmdev)
> @@ -167,8 +167,8 @@ static int init_8960(struct tsens_device *tmdev)
> int ret, i;
> u32 reg_cntl;
>
> - tmdev->map = dev_get_regmap(tmdev->dev, NULL);
> - if (!tmdev->map)
> + tmdev->tm_map = dev_get_regmap(tmdev->dev, NULL);
> + if (!tmdev->tm_map)
> return -ENODEV;
>
> /*
> @@ -184,14 +184,14 @@ static int init_8960(struct tsens_device *tmdev)
> }
>
> reg_cntl = SW_RST;
> - ret = regmap_update_bits(tmdev->map, CNTL_ADDR, SW_RST, reg_cntl);
> + ret = regmap_update_bits(tmdev->tm_map, CNTL_ADDR, SW_RST, reg_cntl);
> if (ret)
> return ret;
>
> if (tmdev->num_sensors > 1) {
> reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18);
> reg_cntl &= ~SW_RST;
> - ret = regmap_update_bits(tmdev->map, CONFIG_ADDR,
> + ret = regmap_update_bits(tmdev->tm_map, CONFIG_ADDR,
> CONFIG_MASK, CONFIG);
> } else {
> reg_cntl |= SLP_CLK_ENA_8660 | (MEASURE_PERIOD << 16);
> @@ -200,12 +200,12 @@ static int init_8960(struct tsens_device *tmdev)
> }
>
> reg_cntl |= GENMASK(tmdev->num_sensors - 1, 0) << SENSOR0_SHIFT;
> - ret = regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
> + ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
> if (ret)
> return ret;
>
> reg_cntl |= EN;
> - ret = regmap_write(tmdev->map, CNTL_ADDR, reg_cntl);
> + ret = regmap_write(tmdev->tm_map, CNTL_ADDR, reg_cntl);
> if (ret)
> return ret;
>
> @@ -252,12 +252,12 @@ static int get_temp_8960(struct tsens_device *tmdev, int id, int *temp)
>
> timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
> do {
> - ret = regmap_read(tmdev->map, INT_STATUS_ADDR, &trdy);
> + ret = regmap_read(tmdev->tm_map, INT_STATUS_ADDR, &trdy);
> if (ret)
> return ret;
> if (!(trdy & TRDY_MASK))
> continue;
> - ret = regmap_read(tmdev->map, s->status, &code);
> + ret = regmap_read(tmdev->tm_map, s->status, &code);
> if (ret)
> return ret;
> *temp = code_to_mdegC(code, s);
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 303e3fdaca98..0585084630b3 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -99,8 +99,7 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
> int last_temp = 0, ret;
>
> status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * SN_ADDR_OFFSET;
> - ret = regmap_read(tmdev->map, status_reg, &code);
> -
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> if (ret)
> return ret;
> last_temp = code & SN_ST_TEMP_MASK;
> @@ -118,7 +117,7 @@ static const struct regmap_config tsens_config = {
>
> int __init init_common(struct tsens_device *tmdev)
> {
> - void __iomem *base;
> + void __iomem *tm_base;
> struct resource *res;
> struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
>
> @@ -134,13 +133,13 @@ int __init init_common(struct tsens_device *tmdev)
> }
>
> res = platform_get_resource(op, IORESOURCE_MEM, 0);
> - base = devm_ioremap_resource(&op->dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> + tm_base = devm_ioremap_resource(&op->dev, res);
> + if (IS_ERR(tm_base))
> + return PTR_ERR(tm_base);
>
> - tmdev->map = devm_regmap_init_mmio(tmdev->dev, base, &tsens_config);
> - if (IS_ERR(tmdev->map))
> - return PTR_ERR(tmdev->map);
> + tmdev->tm_map = devm_regmap_init_mmio(tmdev->dev, tm_base, &tsens_config);
> + if (IS_ERR(tmdev->tm_map))
> + return PTR_ERR(tmdev->tm_map);
>
> return 0;
> }
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 44da02f594ac..1bdef92e4521 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -21,7 +21,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
> int ret;
>
> status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> - ret = regmap_read(tmdev->map, status_reg, &code);
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> if (ret)
> return ret;
> last_temp = code & LAST_TEMP_MASK;
> @@ -29,7 +29,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, status_reg, &code);
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> if (ret)
> return ret;
> if (code & STATUS_VALID_BIT) {
> @@ -40,7 +40,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, status_reg, &code);
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> if (ret)
> return ret;
> if (code & STATUS_VALID_BIT) {
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 8207610f326a..58e98c4d3a8b 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -69,7 +69,7 @@ struct tsens_context {
> struct tsens_device {
> struct device *dev;
> u32 num_sensors;
> - struct regmap *map;
> + struct regmap *tm_map;
> u32 tm_offset;
> struct tsens_context ctx;
> const struct tsens_ops *ops;
> --
> 2.17.1
>

2018-09-03 20:24:38

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 09/11] thermal: tsens: Get rid of 'id' field

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> The hw_id field in 'struct tsens_sensor' can do the job of tracking
> unique ids for each sensor connected to each tsens device instance. It
> also allows hw_ids to be overridden (e.g. 8916) in cases where some
> sensors in a sequence are disabled on a particular platform.
>
> Use the hw_id field instead of the id field consistently across the
> tsens code.
>
> While we're at it, document the fields of struct tsens_sensor.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/thermal/qcom/tsens.c | 5 ++---
> drivers/thermal/qcom/tsens.h | 10 +++++++++-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 9a8e8f7b4ae1..fb728ec5d77f 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -17,7 +17,7 @@ static int tsens_get_temp(void *data, int *temp)
> const struct tsens_sensor *s = data;
> struct tsens_device *tmdev = s->tmdev;
>
> - return tmdev->ops->get_temp(tmdev, s->id, temp);
> + return tmdev->ops->get_temp(tmdev, s->hw_id, temp);
> }
>
> static int tsens_get_trend(void *p, int trip, enum thermal_trend *trend)
> @@ -26,7 +26,7 @@ static int tsens_get_trend(void *p, int trip, enum thermal_trend *trend)
> struct tsens_device *tmdev = s->tmdev;
>
> if (tmdev->ops->get_trend)
> - return tmdev->ops->get_trend(tmdev, s->id, trend);
> + return tmdev->ops->get_trend(tmdev, s->hw_id, trend);
>
> return -ENOTSUPP;
> }
> @@ -83,7 +83,6 @@ static int tsens_register(struct tsens_device *tmdev)
>
> for (i = 0; i < tmdev->num_sensors; i++) {
> tmdev->sensor[i].tmdev = tmdev;
> - tmdev->sensor[i].id = i;
> tzd = devm_thermal_zone_of_sensor_register(tmdev->dev, i,
> &tmdev->sensor[i],
> &tsens_of_ops);
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index b9c4bcf255fa..2a3174dfc1a9 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -14,11 +14,19 @@
>
> struct tsens_device;
>
> +/**
> + * struct tsens_sensor - sensor-specific data
> + * @tmdev: tsens device instance this sensor is connected to
> + * @tzd: thermal zone corresponding to this sensor
> + * @offset: offset from calibration data to convert ADC data to degrees
> + * @hw_id: unique sensor ID for each sensor connected to tsens device instance
> + * @slope: slope from calibration data to convert ADC data to degrees
> + * @status: 8960-specific status register addresses
> + */
> struct tsens_sensor {
> struct tsens_device *tmdev;
> struct thermal_zone_device *tzd;
> int offset;
> - int id;
> int hw_id;
> int slope;
> u32 status;
> --
> 2.17.1
>

2018-09-03 20:25:50

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 07/11] thermal: tsens: Add the SROT address map

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> On platforms whose device trees specify two address spaces for TSENS, the
> second one points to the SROT registers. Initialise the SROT map on those
> platforms.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

Reviewed-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> drivers/thermal/qcom/tsens-common.c | 14 ++++++++++++--
> drivers/thermal/qcom/tsens.h | 1 +
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 0585084630b3..0b8a793f15f4 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -117,16 +117,26 @@ static const struct regmap_config tsens_config = {
>
> int __init init_common(struct tsens_device *tmdev)
> {
> - void __iomem *tm_base;
> + void __iomem *tm_base, *srot_base;
> struct resource *res;
> struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
>
> if (!op)
> return -EINVAL;
>
> - /* The driver only uses the TM register address space for now */
> if (op->num_resources > 1) {
> + /* DT with separate SROT and TM address space */
> tmdev->tm_offset = 0;
> + res = platform_get_resource(op, IORESOURCE_MEM, 1);
> + srot_base = devm_ioremap_resource(&op->dev, res);
> + if (IS_ERR(srot_base))
> + return PTR_ERR(srot_base);
> +
> + tmdev->srot_map = devm_regmap_init_mmio(tmdev->dev,
> + srot_base, &tsens_config);
> + if (IS_ERR(tmdev->srot_map))
> + return PTR_ERR(tmdev->srot_map);
> +
> } else {
> /* old DTs where SROT and TM were in a contiguous 2K block */
> tmdev->tm_offset = 0x1000;
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 58e98c4d3a8b..b9c4bcf255fa 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -70,6 +70,7 @@ struct tsens_device {
> struct device *dev;
> u32 num_sensors;
> struct regmap *tm_map;
> + struct regmap *srot_map;
> u32 tm_offset;
> struct tsens_context ctx;
> const struct tsens_ops *ops;
> --
> 2.17.1
>

2018-09-03 20:31:39

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 10/11] arm64: dts: qcom: Add reg-names for all tsens nodes

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> Instead of showing up as thermal-sensor@<addr>, the nodes will show up as
> tsens0_tm, tsen1_tm, tsens1_srot, etc. in /proc/iomem making it easier to
> read.
>
> IOW,
>
> 0c222000-0c2221fe : thermal-sensor@c263000
> 0c223000-0c2231fe : thermal-sensor@c265000
> 0c263000-0c2631fe : thermal-sensor@c263000
> 0c265000-0c2651fe : thermal-sensor@c265000
>
> becomes
>
> 0c222000-0c2221fe : tsens0_srot
> 0c223000-0c2231fe : tsens1_srot
> 0c263000-0c2631fe : tsens0_tm
> 0c265000-0c2651fe : tsens1_tm
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++
> 4 files changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 3c4b81c29798..64c9f81ddd90 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -431,6 +431,7 @@
> compatible = "qcom,msm8974-tsens";
> reg = <0xfc4a9000 0x1000>, /* TM */
> <0xfc4a8000 0x1000>; /* SROT */
> + reg-names = "tsens_tm", "tsens_srot";

While the iomem output seems more convenient this way, these register
names are a contract between the DT binding and the particular tsens
instance.

As such this is a good idea, but with the names should not include the
instance information. They should be "tm", "srot".

> nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> nvmem-cell-names = "calib", "calib_backup";
> #qcom,sensors = <11>;
[..]
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa6a1b5..f1a36d6829cf 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -966,6 +966,7 @@
> compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> reg = <0xc263000 0x1ff>, /* TM */
> <0xc222000 0x1ff>; /* SROT */
> + reg-names = "tsens0_tm", "tsens0_srot";
> #qcom,sensors = <13>;
> #thermal-sensor-cells = <1>;
> };
> @@ -974,6 +975,7 @@
> compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> reg = <0xc265000 0x1ff>, /* TM */
> <0xc223000 0x1ff>; /* SROT */
> + reg-names = "tsens1_tm", "tsens1_srot";

And I do recognize that in this case iomem won't show which one is
tsens0 and which is tsens1...


As with previous patches, please split arm and arm64 in separate
patches.

Regards,
Bjorn

2018-09-03 20:44:09

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 11/11] MAINTAINERS: Add entry for Qualcomm TSENS thermal drivers

On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:

> Create an entry for the TSENS drivers and mark them as maintained
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Matthias Kaehlcke <[email protected]>
> Acked-by: Rajendra Nayak <[email protected]>

Acked-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

> ---
> MAINTAINERS | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a5b256b25905..b8c96e0699c9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12076,6 +12076,13 @@ L: [email protected]
> S: Maintained
> F: drivers/iommu/qcom_iommu.c
>
> +QUALCOMM TSENS THERMAL DRIVER
> +M: Amit Kucheria <[email protected]>
> +L: [email protected]
> +L: [email protected]
> +S: Maintained
> +F: drivers/thermal/qcom/
> +
> QUALCOMM VENUS VIDEO ACCELERATOR DRIVER
> M: Stanimir Varbanov <[email protected]>
> L: [email protected]
> --
> 2.17.1
>

2018-09-03 20:46:22

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v2 08/11] thermal: tsens: Check if the IP is correctly enabled by firmware

On Thu 30 Aug 12:23 PDT 2018, Amit Kucheria wrote:

> On Tue, Aug 28, 2018 at 7:08 PM, Amit Kucheria <[email protected]> wrote:
> > The SROT registers are initialised by the secure firmware at boot. We
> > don't have write access to the registers. Check if the block is enabled
> > before continuing.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > Reviewed-by: Matthias Kaehlcke <[email protected]>
> > ---
> > drivers/thermal/qcom/tsens-common.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > index 0b8a793f15f4..d250b757d1f0 100644
> > --- a/drivers/thermal/qcom/tsens-common.c
> > +++ b/drivers/thermal/qcom/tsens-common.c
> > @@ -12,6 +12,11 @@
> > #include <linux/regmap.h>
> > #include "tsens.h"
> >
> > +/* SROT */
> > +#define CTRL_OFFSET 0x4
>
> This OFFSET is not constant across the TSENS family and breaks atleast
> one platform in the test below. I have a patch to fix this for all
> platforms, will post it after a bit more testing.
>

Afaict the other 10 patches can be merged independently of this patch.

So please make the minor updates and resend them as a separate series,
with the acks/reviews, to make it easier for Eduardo to just pick them.

Regards,
Bjorn

>
> > +#define TSENS_EN BIT(0)
> > +
> > +/* TM */
> > #define STATUS_OFFSET 0x30
> > #define SN_ADDR_OFFSET 0x4
> > #define SN_ST_TEMP_MASK 0x3ff
> > @@ -119,6 +124,8 @@ int __init init_common(struct tsens_device *tmdev)
> > {
> > void __iomem *tm_base, *srot_base;
> > struct resource *res;
> > + u32 code;
> > + int ret;
> > struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
> >
> > if (!op)
> > @@ -151,5 +158,15 @@ int __init init_common(struct tsens_device *tmdev)
> > if (IS_ERR(tmdev->tm_map))
> > return PTR_ERR(tmdev->tm_map);
> >
> > + if (tmdev->srot_map) {
> > + ret = regmap_read(tmdev->srot_map, CTRL_OFFSET, &code);
> > + if (ret)
> > + return ret;
> > + if (!(code & TSENS_EN)) {
> > + dev_err(tmdev->dev, "tsens device is not enabled\n");
> > + return -ENODEV;
> > + }
> > + }
> > +
> > return 0;
> > }
> > --
> > 2.17.1
> >

2018-09-04 23:44:03

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v2 10/11] arm64: dts: qcom: Add reg-names for all tsens nodes

Hi,

On Mon, Sep 3, 2018 at 1:34 PM, Bjorn Andersson
<[email protected]> wrote:
> On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:
>
>> Instead of showing up as thermal-sensor@<addr>, the nodes will show up as
>> tsens0_tm, tsen1_tm, tsens1_srot, etc. in /proc/iomem making it easier to
>> read.
>>
>> IOW,
>>
>> 0c222000-0c2221fe : thermal-sensor@c263000
>> 0c223000-0c2231fe : thermal-sensor@c265000
>> 0c263000-0c2631fe : thermal-sensor@c263000
>> 0c265000-0c2651fe : thermal-sensor@c265000
>>
>> becomes
>>
>> 0c222000-0c2221fe : tsens0_srot
>> 0c223000-0c2231fe : tsens1_srot
>> 0c263000-0c2631fe : tsens0_tm
>> 0c265000-0c2651fe : tsens1_tm
>>
>> Signed-off-by: Amit Kucheria <[email protected]>
>> Reviewed-by: Matthias Kaehlcke <[email protected]>
>> ---
>> arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
>> arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 ++
>> 4 files changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> index 3c4b81c29798..64c9f81ddd90 100644
>> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
>> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
>> @@ -431,6 +431,7 @@
>> compatible = "qcom,msm8974-tsens";
>> reg = <0xfc4a9000 0x1000>, /* TM */
>> <0xfc4a8000 0x1000>; /* SROT */
>> + reg-names = "tsens_tm", "tsens_srot";
>
> While the iomem output seems more convenient this way, these register
> names are a contract between the DT binding and the particular tsens
> instance.
>
> As such this is a good idea, but with the names should not include the
> instance information. They should be "tm", "srot".

Rob Herring doesn't seem to think so. As per
<http://lkml.kernel.org/r/CAL_Jsq+MMunmVWqeW9v2RyzsMKP+=kMzeTHNMG4JDHM7Fy0HBg@mail.gmail.com>

I said:

> From what you're saying the _only_ reason you'd ever want to use
> reg-names is if there is an optional register range. Is that right?

Rob said:

> IMO, yes.

It sounds like Rob won't NAK a change that adds reg-names if there is
more than one reg, but in general he's not a fan. I'd vote to keep
things consistent with Rob's worldview and just drop this change.


-Doug

2018-09-06 09:25:54

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 01/11] arm/arm64: dts: msm8974/msm8916: thermal: Split address space into two

On Tue, Sep 4, 2018 at 1:29 AM Bjorn Andersson
<[email protected]> wrote:
>
> On Tue 28 Aug 06:38 PDT 2018, Amit Kucheria wrote:
>
> > We've earlier added support to split the register address space into TM
> > and SROT regions.
> >
> > Split up the regmap address space into two for the remaining platforms
> > that have a similar register layout and make corresponding changes to
> > the get_temp_common() function used by these platforms.
> >
> > Since tsens-common.c/init_common() currently only registers one address
> > space, the order is important (TM before SROT). This is OK since the
> > code doesn't really use the SROT functionality yet.
> >
>
> Having a single patch touching both code and dts will cause merge issues
> as this patch travel upstream. Even more arm-soc expects arm and arm64
> dts changes to come in different pull requests.


> Please split it so that the three pieces can be picked up by respective
> maintainer.

Will do.

> > Signed-off-by: Amit Kucheria <[email protected]>
> > Reviewed-by: Matthias Kaehlcke <[email protected]>
> > ---
> > arch/arm/boot/dts/qcom-msm8974.dtsi | 5 +++--
> > arch/arm64/boot/dts/qcom/msm8916.dtsi | 5 +++--
> > drivers/thermal/qcom/tsens-common.c | 5 +++--
> > 3 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> > index d9019a49b292..56dbbf788d15 100644
> > --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> > +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> > @@ -427,9 +427,10 @@
> > };
> > };
> >
> > - tsens: thermal-sensor@fc4a8000 {
> > + tsens: thermal-sensor@fc4a9000 {
> > compatible = "qcom,msm8974-tsens";
> > - reg = <0xfc4a8000 0x2000>;
> > + reg = <0xfc4a9000 0x1000>, /* TM */
> > + <0xfc4a8000 0x1000>; /* SROT */
> > nvmem-cells = <&tsens_calib>, <&tsens_backup>;
> > nvmem-cell-names = "calib", "calib_backup";
> > #thermal-sensor-cells = <1>;
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > index 7b32b8990d62..6a277fce3333 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > @@ -761,9 +761,10 @@
> > };
> > };
> >
> > - tsens: thermal-sensor@4a8000 {
> > + tsens: thermal-sensor@4a9000 {
> > compatible = "qcom,msm8916-tsens";
> > - reg = <0x4a8000 0x2000>;
> > + reg = <0x4a9000 0x1000>, /* TM */
> > + <0x4a8000 0x1000>; /* SROT */
> > nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
> > nvmem-cell-names = "calib", "calib_sel";
> > #thermal-sensor-cells = <1>;
> > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > index 6207d8d92351..478739543bbc 100644
> > --- a/drivers/thermal/qcom/tsens-common.c
> > +++ b/drivers/thermal/qcom/tsens-common.c
> > @@ -21,7 +21,7 @@
> > #include <linux/regmap.h>
> > #include "tsens.h"
> >
> > -#define S0_ST_ADDR 0x1030
> > +#define STATUS_OFFSET 0x30
> > #define SN_ADDR_OFFSET 0x4
> > #define SN_ST_TEMP_MASK 0x3ff
> > #define CAL_DEGC_PT1 30
> > @@ -107,8 +107,9 @@ int get_temp_common(struct tsens_device *tmdev, int id, int *temp)
> > unsigned int status_reg;
> > int last_temp = 0, ret;
> >
> > - status_reg = S0_ST_ADDR + s->hw_id * SN_ADDR_OFFSET;
> > + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * SN_ADDR_OFFSET;
>
> Wasn't this change part of the previous set that introduced the
> tm_offset? If not how did we handle the fact that tmdev->map is already
> indented 0x1000 bytes?

It was a similar change 5b1283984fa3 ("thermal: tsens: Add support to
split up register address space into two") for the get_temp_8996()
function.

This patch converts over the remaining users.

> Both changes looks good, but I'm worries about the order of things.

We're OK.

2018-09-06 10:18:07

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 08/11] thermal: tsens: Check if the IP is correctly enabled by firmware

On Tue, Sep 4, 2018 at 2:15 AM Bjorn Andersson
<[email protected]> wrote:
>
> On Thu 30 Aug 12:23 PDT 2018, Amit Kucheria wrote:
>
> > On Tue, Aug 28, 2018 at 7:08 PM, Amit Kucheria <[email protected]> wrote:
> > > The SROT registers are initialised by the secure firmware at boot. We
> > > don't have write access to the registers. Check if the block is enabled
> > > before continuing.
> > >
> > > Signed-off-by: Amit Kucheria <[email protected]>
> > > Reviewed-by: Matthias Kaehlcke <[email protected]>
> > > ---
> > > drivers/thermal/qcom/tsens-common.c | 17 +++++++++++++++++
> > > 1 file changed, 17 insertions(+)
> > >
> > > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > > index 0b8a793f15f4..d250b757d1f0 100644
> > > --- a/drivers/thermal/qcom/tsens-common.c
> > > +++ b/drivers/thermal/qcom/tsens-common.c
> > > @@ -12,6 +12,11 @@
> > > #include <linux/regmap.h>
> > > #include "tsens.h"
> > >
> > > +/* SROT */
> > > +#define CTRL_OFFSET 0x4
> >
> > This OFFSET is not constant across the TSENS family and breaks atleast
> > one platform in the test below. I have a patch to fix this for all
> > platforms, will post it after a bit more testing.
> >
>
> Afaict the other 10 patches can be merged independently of this patch.
>
> So please make the minor updates and resend them as a separate series,
> with the acks/reviews, to make it easier for Eduardo to just pick them.
>

Will do.