[PATCH v7 0/7] thermal: tsens: Refactoring for TSENSv2 IP

[PATCH v7 0/7] thermal: tsens: Refactoring for TSENSv2 IP

[Amit Kucheria]

This series is a mixed bag:
- Some code moves to allow code sharing between different SoCs with v2 of
  the TSENS IP,
- a generic qcom,tsens-v2 property as a fallback compatible for all v2.x.y
  platforms,
- new platform support (sdm845)
- a cleanup patch and
- a DT change to have a common way to deal with the SROT and TM registers
  despite slightly different features across the IP family and different
  register offsets.

Changes since v6:
- Fix comments and patch descriptions as per Doug's review
- Rename tsens to thermal-sensor in DT
- Add various review tags

Changes since v5:
- Actually fix unit addressses for the two tsens blocks as per Stephen's comment.

Changes since v4:
- Revert back to a single fallback bindind qcom,tsens-v2 as per Rob's
  suggestion.
- Rework how old (unsplit SROT and TM address space) DTs are handled by
  needing a 0x1000 offset but still sharing common code in tsens-v2.c
- Remove the patch to added TRDY checks while we investigate Matthias'
  reports
- Fix unit addressses for the two tsens blocks as per Stephen's comment.

Changes since v3:
- Introduce qcom,tsens-v2.4.0 property and make qcom,tsens-v2 a
  fallback, compatible property.
- Rename ops_v2 to ops_generic_v2

Changes since v2:

- Based on review, moved tsens-8996.c to tsens-v2.c and changed
  corresponding function names, struct names to allow for generic tsensv2
  platforms
- All v2 platforms will now only need to use the qcom,tsen-v2
  property
- Added a DT patch to initialize tsens driver on sdm845, now that
  4.18-rc1 will contain an sdm845.dtsi

Changes since v1:
- Move get_temp() from tsens-8996 to tsens-common and rename
- Change 8996 DT entry to allow init_common() to work across
  sdm845 and 8996 due to different offsets

Amit Kucheria (7):
  thermal: tsens: Get rid of unused fields in structure
  thermal: tsens: Add support to split up register address space into
    two
  arm64: dts: msm8996: thermal: Initialise via DT and add second
    controller
  thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
  dt: thermal: tsens: Document the fallback DT property for v2 of TSENS
    IP
  thermal: tsens: Add generic support for TSENS v2 IP
  arm64: dts: sdm845: Add tsens nodes

 .../devicetree/bindings/thermal/qcom-tsens.txt     | 31 +++++++++++++++++----
 arch/arm64/boot/dts/qcom/msm8996.dtsi              | 14 ++++++++--
 arch/arm64/boot/dts/qcom/sdm845.dtsi               | 16 +++++++++++
 drivers/thermal/qcom/Makefile                      |  2 +-
 drivers/thermal/qcom/tsens-common.c                | 12 ++++++++
 drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c}  | 32 ++++++++++------------
 drivers/thermal/qcom/tsens.c                       |  3 ++
 drivers/thermal/qcom/tsens.h                       |  8 ++++--
 8 files changed, 88 insertions(+), 30 deletions(-)
 rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (64%)

-- 
2.7.4

[PATCH v7 1/7] thermal: tsens: Get rid of unused fields in structure

[Amit Kucheria]

status_field and trdy are unused in any of the tsens drivers. Remove them.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Rajendra Nayak <rnayak@codeaurora.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/thermal/qcom/tsens.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 911c197..dc56e1e 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -77,9 +77,7 @@ struct tsens_device {
 	struct device			*dev;
 	u32				num_sensors;
 	struct regmap			*map;
-	struct regmap_field		*status_field;
 	struct tsens_context		ctx;
-	bool				trdy;
 	const struct tsens_ops		*ops;
 	struct tsens_sensor		sensor[0];
 };
-- 
2.7.4

[PATCH v7 2/7] thermal: tsens: Add support to split up register address space into two

[Amit Kucheria]

There are two banks of registers for v2 TSENS IPs: SROT and TM. On older
SoCs these were contiguous, leading to DTs mapping them as one register
address space of size 0x2000. In newer SoCs, these two banks are not
contiguous anymore.

Add logic to init_common() to differentiate between old and new DTs and
adjust associated offsets for the TM register bank so that the old DTs will
continue to function correctly.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/thermal/qcom/tsens-8996.c   |  4 ++--
 drivers/thermal/qcom/tsens-common.c | 12 ++++++++++++
 drivers/thermal/qcom/tsens.h        |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8996.c b/drivers/thermal/qcom/tsens-8996.c
index e1f7781..3e60cec 100644
--- a/drivers/thermal/qcom/tsens-8996.c
+++ b/drivers/thermal/qcom/tsens-8996.c
@@ -16,7 +16,7 @@
 #include <linux/regmap.h>
 #include "tsens.h"
 
-#define STATUS_OFFSET	0x10a0
+#define STATUS_OFFSET	0xa0
 #define LAST_TEMP_MASK	0xfff
 #define STATUS_VALID_BIT	BIT(21)
 #define CODE_SIGN_BIT		BIT(11)
@@ -28,7 +28,7 @@ static int get_temp_8996(struct tsens_device *tmdev, int id, int *temp)
 	unsigned int sensor_addr;
 	int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
 
-	sensor_addr = STATUS_OFFSET + s->hw_id * 4;
+	sensor_addr = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
 	ret = regmap_read(tmdev->map, sensor_addr, &code);
 	if (ret)
 		return ret;
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index b1449ad..c22dc18 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/of_address.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include "tsens.h"
@@ -126,11 +127,22 @@ static const struct regmap_config tsens_config = {
 int __init init_common(struct tsens_device *tmdev)
 {
 	void __iomem *base;
+	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) {
+		tmdev->tm_offset = 0;
+	} else {
+		/* old DTs where SROT and TM were in a contiguous 2K block */
+		tmdev->tm_offset = 0x1000;
+	}
+
 	tmdev->map = devm_regmap_init_mmio(tmdev->dev, base, &tsens_config);
 	if (IS_ERR(tmdev->map)) {
 		iounmap(base);
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index dc56e1e..d785b37 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -77,6 +77,7 @@ struct tsens_device {
 	struct device			*dev;
 	u32				num_sensors;
 	struct regmap			*map;
+	u32				tm_offset;
 	struct tsens_context		ctx;
 	const struct tsens_ops		*ops;
 	struct tsens_sensor		sensor[0];
-- 
2.7.4

[PATCH v7 3/7] arm64: dts: msm8996: thermal: Initialise via DT and add second controller

[Amit Kucheria]

We also split up the regmap address space into two, for the TM and SROT
registers. This was required to deal with different address offsets for the
TM and SROT registers across different SoC families.

8996 has two TSENS IP blocks, initialise the second one too.

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 <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 8c7f9ca..688e752 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -459,9 +459,19 @@
 			status = "disabled";
 		};
 
-		tsens0: thermal-sensor@4a8000 {
+		tsens0: thermal-sensor@4a9000 {
 			compatible = "qcom,msm8996-tsens";
-			reg = <0x4a8000 0x2000>;
+			reg = <0x4a9000 0x1000>, /* TM */
+			      <0x4a8000 0x1000>; /* SROT */
+			#qcom,sensors = <13>;
+			#thermal-sensor-cells = <1>;
+		};
+
+		tsens1: thermal-sensor@4ad000 {
+			compatible = "qcom,msm8996-tsens";
+			reg = <0x4ad000 0x1000>, /* TM */
+			      <0x4ac000 0x1000>; /* SROT */
+			#qcom,sensors = <8>;
 			#thermal-sensor-cells = <1>;
 		};
 
-- 
2.7.4

[PATCH v7 4/7] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse

[Amit Kucheria]

The TSENS block inside the 8996 is internally classified as version 2 of
the IP. Several other SoC families use this block and can share this code.

We rename get_temp() to reflect that it can be used across the v2 family.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/thermal/qcom/Makefile                     |  2 +-
 drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 25 ++++++++---------------
 2 files changed, 9 insertions(+), 18 deletions(-)
 rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%)

diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
index 2cc2193..a821929 100644
--- a/drivers/thermal/qcom/Makefile
+++ b/drivers/thermal/qcom/Makefile
@@ -1,2 +1,2 @@
 obj-$(CONFIG_QCOM_TSENS)	+= qcom_tsens.o
-qcom_tsens-y			+= tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-8996.o
+qcom_tsens-y			+= tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o
diff --git a/drivers/thermal/qcom/tsens-8996.c b/drivers/thermal/qcom/tsens-v2.c
similarity index 66%
rename from drivers/thermal/qcom/tsens-8996.c
rename to drivers/thermal/qcom/tsens-v2.c
index 3e60cec..44d3736 100644
--- a/drivers/thermal/qcom/tsens-8996.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -1,27 +1,18 @@
+// 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.
- *
+ * Copyright (c) 2018, Linaro Limited
  */
 
-#include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include "tsens.h"
 
-#define STATUS_OFFSET	0xa0
-#define LAST_TEMP_MASK	0xfff
+#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_8996(struct tsens_device *tmdev, int id, int *temp)
+static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
 {
 	struct tsens_sensor *s = &tmdev->sensor[id];
 	u32 code;
@@ -73,12 +64,12 @@ static int get_temp_8996(struct tsens_device *tmdev, int id, int *temp)
 	return 0;
 }
 
-static const struct tsens_ops ops_8996 = {
+static const struct tsens_ops ops_generic_v2 = {
 	.init		= init_common,
-	.get_temp	= get_temp_8996,
+	.get_temp	= get_temp_tsens_v2,
 };
 
 const struct tsens_data data_8996 = {
 	.num_sensors	= 13,
-	.ops		= &ops_8996,
+	.ops		= &ops_generic_v2,
 };
-- 
2.7.4

[PATCH v7 5/7] dt: thermal: tsens: Document the fallback DT property for v2 of TSENS IP

[Amit Kucheria]

We want to create common code for v2 of the TSENS IP block that is used in
a large number of Qualcomm SoCs. "qcom,tsens-v2" should be able to handle
most of the common functionality start with a common get_temp() function.

It is also necessary to split out the memory regions for the TM and SROT
register banks because their offsets are not constant across SoC families.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 .../devicetree/bindings/thermal/qcom-tsens.txt     | 31 +++++++++++++++++-----
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 06195e8..b5312a8 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -1,18 +1,28 @@
 * QCOM SoC Temperature Sensor (TSENS)
 
 Required properties:
-- compatible :
- - "qcom,msm8916-tsens" : For 8916 Family of SoCs
- - "qcom,msm8974-tsens" : For 8974 Family of SoCs
- - "qcom,msm8996-tsens" : For 8996 Family of SoCs
+- compatible:
+  Must be one of the following:
+    - "qcom,msm8916-tsens" (MSM8916)
+    - "qcom,msm8974-tsens" (MSM8974)
+    - "qcom,msm8996-tsens" (MSM8996)
+    - "qcom,msm8998-tsens", "qcom,tsens-v2" (MSM8998)
+    - "qcom,sdm845-tsens", "qcom,tsens-v2" (SDM845)
+  The generic "qcom,tsens-v2" property must be used as a fallback for any SoC
+  with version 2 of the TSENS IP. MSM8996 is the only exception beacause the
+  generic property did not exist when support was added.
+
+- reg: Address range of the thermal registers.
+  New platforms containing v2.x.y of the TSENS IP must specify the SROT and TM
+  register spaces separately, with order being TM before SROT.
+  See Example 2, below.
 
-- reg: Address range of the thermal registers
 - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
 - #qcom,sensors: Number of sensors in tsens block
 - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
 nvmem cells
 
-Example:
+Example 1 (legacy support before a fallback tsens-v2 propoerty was introduced):
 tsens: thermal-sensor@900000 {
 		compatible = "qcom,msm8916-tsens";
 		reg = <0x4a8000 0x2000>;
@@ -20,3 +30,12 @@ tsens: thermal-sensor@900000 {
 		nvmem-cell-names = "caldata", "calsel";
 		#thermal-sensor-cells = <1>;
 	};
+
+Example 2 (for any platform containing v2 of the TSENS IP):
+tsens0: thermal-sensor@c263000 {
+		compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+		reg = <0xc263000 0x1ff>, /* TM */
+			<0xc222000 0x1ff>; /* SROT */
+		#qcom,sensors = <13>;
+		#thermal-sensor-cells = <1>;
+	};
-- 
2.7.4

[PATCH v7 6/7] thermal: tsens: Add generic support for TSENS v2 IP

[Amit Kucheria]

SDM845 uses v2 of the TSENS IP block but the get_temp() function appears to
be identical across v2.x.y in code seen so far. We use the generic
get_temp() function defined as part of ops_generic_v2.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/thermal/qcom/tsens-v2.c | 5 +++++
 drivers/thermal/qcom/tsens.c    | 3 +++
 drivers/thermal/qcom/tsens.h    | 5 ++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 44d3736..f40150f 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -69,6 +69,11 @@ static const struct tsens_ops ops_generic_v2 = {
 	.get_temp	= get_temp_tsens_v2,
 };
 
+const struct tsens_data data_tsens_v2 = {
+	.ops            = &ops_generic_v2,
+};
+
+/* Kept around for backward compatibility with old msm8996.dtsi */
 const struct tsens_data data_8996 = {
 	.num_sensors	= 13,
 	.ops		= &ops_generic_v2,
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 3440166c..a2c9bfa 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -72,6 +72,9 @@ static const struct of_device_id tsens_table[] = {
 	}, {
 		.compatible = "qcom,msm8996-tsens",
 		.data = &data_8996,
+	}, {
+		.compatible = "qcom,tsens-v2",
+		.data = &data_tsens_v2,
 	},
 	{}
 };
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index d785b37..14331eb 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -88,6 +88,9 @@ void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32);
 int init_common(struct tsens_device *);
 int get_temp_common(struct tsens_device *, int, int *);
 
-extern const struct tsens_data data_8916, data_8974, data_8960, data_8996;
+/* TSENS v1 targets */
+extern const struct tsens_data data_8916, data_8974, data_8960;
+/* TSENS v2 targets */
+extern const struct tsens_data data_8996, data_tsens_v2;
 
 #endif /* __QCOM_TSENS_H__ */
-- 
2.7.4

[PATCH v7 7/7] arm64: dts: sdm845: Add tsens nodes

[Amit Kucheria]

SDM845 has two tsens blocks, one with 13 sensors and the other with 8
sensors. It uses version 2 of the TSENS IP, so use the fallback property to
allow more common code.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index cdaabeb..01ff146 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -221,6 +221,22 @@
 			#interrupt-cells = <2>;
 		};
 
+		tsens0: thermal-sensor@c263000 {
+			compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+			reg = <0xc263000 0x1ff>, /* TM */
+			      <0xc222000 0x1ff>; /* SROT */
+			#qcom,sensors = <13>;
+			#thermal-sensor-cells = <1>;
+		};
+
+		tsens1: thermal-sensor@c265000 {
+			compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
+			reg = <0xc265000 0x1ff>, /* TM */
+			      <0xc223000 0x1ff>; /* SROT */
+			#qcom,sensors = <8>;
+			#thermal-sensor-cells = <1>;
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0xc440000 0x1100>,
-- 
2.7.4

Re: [PATCH v7 2/7] thermal: tsens: Add support to split up register address space into two

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> There are two banks of registers for v2 TSENS IPs: SROT and TM. On older
> SoCs these were contiguous, leading to DTs mapping them as one register
> address space of size 0x2000. In newer SoCs, these two banks are not
> contiguous anymore.
>
> Add logic to init_common() to differentiate between old and new DTs and
> adjust associated offsets for the TM register bank so that the old DTs will
> continue to function correctly.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/thermal/qcom/tsens-8996.c   |  4 ++--
>  drivers/thermal/qcom/tsens-common.c | 12 ++++++++++++
>  drivers/thermal/qcom/tsens.h        |  1 +
>  3 files changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 3/7] arm64: dts: msm8996: thermal: Initialise via DT and add second controller

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> We also split up the regmap address space into two, for the TM and SROT
> registers. This was required to deal with different address offsets for the
> TM and SROT registers across different SoC families.
>
> 8996 has two TSENS IP blocks, initialise the second one too.
>
> 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 <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 5/7] dt: thermal: tsens: Document the fallback DT property for v2 of TSENS IP

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> We want to create common code for v2 of the TSENS IP block that is used in
> a large number of Qualcomm SoCs. "qcom,tsens-v2" should be able to handle
> most of the common functionality start with a common get_temp() function.
>
> It is also necessary to split out the memory regions for the TM and SROT
> register banks because their offsets are not constant across SoC families.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  .../devicetree/bindings/thermal/qcom-tsens.txt     | 31 +++++++++++++++++-----
>  1 file changed, 25 insertions(+), 6 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 6/7] thermal: tsens: Add generic support for TSENS v2 IP

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> SDM845 uses v2 of the TSENS IP block but the get_temp() function appears to
> be identical across v2.x.y in code seen so far. We use the generic
> get_temp() function defined as part of ops_generic_v2.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/thermal/qcom/tsens-v2.c | 5 +++++
>  drivers/thermal/qcom/tsens.c    | 3 +++
>  drivers/thermal/qcom/tsens.h    | 5 ++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 7/7] arm64: dts: sdm845: Add tsens nodes

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> sensors. It uses version 2 of the TSENS IP, so use the fallback property to
> allow more common code.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 4/7] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse

[Doug Anderson]

Hi,

On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> The TSENS block inside the 8996 is internally classified as version 2 of
> the IP. Several other SoC families use this block and can share this code.
>
> We rename get_temp() to reflect that it can be used across the v2 family.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/thermal/qcom/Makefile                     |  2 +-
>  drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 25 ++++++++---------------
>  2 files changed, 9 insertions(+), 18 deletions(-)
>  rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: [PATCH v7 2/7] thermal: tsens: Add support to split up register address space into two

[Matthias Kaehlcke]

On Thu, Jul 12, 2018 at 02:09:03PM +0530, Amit Kucheria wrote:
> There are two banks of registers for v2 TSENS IPs: SROT and TM. On older
> SoCs these were contiguous, leading to DTs mapping them as one register
> address space of size 0x2000. In newer SoCs, these two banks are not
> contiguous anymore.
> 
> Add logic to init_common() to differentiate between old and new DTs and
> adjust associated offsets for the TM register bank so that the old DTs will
> continue to function correctly.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/thermal/qcom/tsens-8996.c   |  4 ++--
>  drivers/thermal/qcom/tsens-common.c | 12 ++++++++++++
>  drivers/thermal/qcom/tsens.h        |  1 +
>  3 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-8996.c b/drivers/thermal/qcom/tsens-8996.c
> index e1f7781..3e60cec 100644
> --- a/drivers/thermal/qcom/tsens-8996.c
> +++ b/drivers/thermal/qcom/tsens-8996.c
> @@ -16,7 +16,7 @@
>  #include <linux/regmap.h>
>  #include "tsens.h"
>  
> -#define STATUS_OFFSET	0x10a0
> +#define STATUS_OFFSET	0xa0
>  #define LAST_TEMP_MASK	0xfff
>  #define STATUS_VALID_BIT	BIT(21)
>  #define CODE_SIGN_BIT		BIT(11)
> @@ -28,7 +28,7 @@ static int get_temp_8996(struct tsens_device *tmdev, int id, int *temp)
>  	unsigned int sensor_addr;
>  	int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
>  
> -	sensor_addr = STATUS_OFFSET + s->hw_id * 4;
> +	sensor_addr = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
>  	ret = regmap_read(tmdev->map, sensor_addr, &code);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index b1449ad..c22dc18 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -16,6 +16,7 @@
>  #include <linux/io.h>
>  #include <linux/nvmem-consumer.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include "tsens.h"
> @@ -126,11 +127,22 @@ static const struct regmap_config tsens_config = {
>  int __init init_common(struct tsens_device *tmdev)
>  {
>  	void __iomem *base;
> +	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) {
> +		tmdev->tm_offset = 0;
> +	} else {
> +		/* old DTs where SROT and TM were in a contiguous 2K block */
> +		tmdev->tm_offset = 0x1000;
> +	}

nit: no curly braces for conditionals with a single statement. There
is probably no need to respin just for this though.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 3/7] arm64: dts: msm8996: thermal: Initialise via DT and add second controller

[Matthias Kaehlcke]

On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote:
> We also split up the regmap address space into two, for the TM and SROT
> registers. This was required to deal with different address offsets for the
> TM and SROT registers across different SoC families.
> 
> 8996 has two TSENS IP blocks, initialise the second one too.
> 
> 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 <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 8c7f9ca..688e752 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -459,9 +459,19 @@
>  			status = "disabled";
>  		};
>  
> -		tsens0: thermal-sensor@4a8000 {
> +		tsens0: thermal-sensor@4a9000 {
                                       ~~~~~~

I suppose the address of the TM block is used here instead of the SROT
address (which is lower) since SROT functionality is currently not
used. Would/should this change if/when the driver uses SROT?

>  			compatible = "qcom,msm8996-tsens";
> -			reg = <0x4a8000 0x2000>;
> +			reg = <0x4a9000 0x1000>, /* TM */
> +			      <0x4a8000 0x1000>; /* SROT */
> +			#qcom,sensors = <13>;
> +			#thermal-sensor-cells = <1>;
> +		};
> +
> +		tsens1: thermal-sensor@4ad000 {
> +			compatible = "qcom,msm8996-tsens";
> +			reg = <0x4ad000 0x1000>, /* TM */
> +			      <0x4ac000 0x1000>; /* SROT */
> +			#qcom,sensors = <8>;
>  			#thermal-sensor-cells = <1>;
>  		};

Re: [PATCH v7 4/7] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse

[Matthias Kaehlcke]

On Thu, Jul 12, 2018 at 02:09:05PM +0530, Amit Kucheria wrote:
> The TSENS block inside the 8996 is internally classified as version 2 of
> the IP. Several other SoC families use this block and can share this code.
> 
> We rename get_temp() to reflect that it can be used across the v2 family.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/thermal/qcom/Makefile                     |  2 +-
>  drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 25 ++++++++---------------
>  2 files changed, 9 insertions(+), 18 deletions(-)
>  rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%)
> 
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index 2cc2193..a821929 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -1,2 +1,2 @@
>  obj-$(CONFIG_QCOM_TSENS)	+= qcom_tsens.o
> -qcom_tsens-y			+= tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-8996.o
> +qcom_tsens-y			+= tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o
> diff --git a/drivers/thermal/qcom/tsens-8996.c b/drivers/thermal/qcom/tsens-v2.c
> similarity index 66%
> rename from drivers/thermal/qcom/tsens-8996.c
> rename to drivers/thermal/qcom/tsens-v2.c
> index 3e60cec..44d3736 100644
> --- a/drivers/thermal/qcom/tsens-8996.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -1,27 +1,18 @@
> +// 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.
> - *
> + * Copyright (c) 2018, Linaro Limited
>   */
>  
> -#include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include "tsens.h"
>  
> -#define STATUS_OFFSET	0xa0
> -#define LAST_TEMP_MASK	0xfff
> +#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_8996(struct tsens_device *tmdev, int id, int *temp)
> +static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
>  {
>  	struct tsens_sensor *s = &tmdev->sensor[id];
>  	u32 code;
> @@ -73,12 +64,12 @@ static int get_temp_8996(struct tsens_device *tmdev, int id, int *temp)
>  	return 0;
>  }
>  
> -static const struct tsens_ops ops_8996 = {
> +static const struct tsens_ops ops_generic_v2 = {
>  	.init		= init_common,
> -	.get_temp	= get_temp_8996,
> +	.get_temp	= get_temp_tsens_v2,
>  };
>  
>  const struct tsens_data data_8996 = {
>  	.num_sensors	= 13,
> -	.ops		= &ops_8996,
> +	.ops		= &ops_generic_v2,
>  };

nit: the unrelated cleanup (SPDX identifier, removal of header,
indendation fixes) could have been done in a separate patch. This
patch is small enough though that it doesn't add too much
distraction and one could argue that this patch creates a new
file ;-)

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 3/7] arm64: dts: msm8996: thermal: Initialise via DT and add second controller

[Doug Anderson]

Hi,

On Tue, Jul 17, 2018 at 4:42 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote:
>> We also split up the regmap address space into two, for the TM and SROT
>> registers. This was required to deal with different address offsets for the
>> TM and SROT registers across different SoC families.
>>
>> 8996 has two TSENS IP blocks, initialise the second one too.
>>
>> 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 <amit.kucheria@linaro.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> Tested-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> index 8c7f9ca..688e752 100644
>> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
>> @@ -459,9 +459,19 @@
>>                       status = "disabled";
>>               };
>>
>> -             tsens0: thermal-sensor@4a8000 {
>> +             tsens0: thermal-sensor@4a9000 {
>                                        ~~~~~~
>
> I suppose the address of the TM block is used here instead of the SROT
> address (which is lower) since SROT functionality is currently not
> used. Would/should this change if/when the driver uses SROT?

For device tree you're always supposed to use the address of the first
"reg" listed as the unit address in the node name.  It doesn't matter
if it's bigger or smaller as long as it's the first one listed.

The bindings indicate that the TM block should be listed as the first
register.  This won't change even if you start using SROT.


-Doug

Re: [PATCH v7 3/7] arm64: dts: msm8996: thermal: Initialise via DT and add second controller

[Matthias Kaehlcke]

On Tue, Jul 17, 2018 at 04:55:10PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 17, 2018 at 4:42 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote:
> >> We also split up the regmap address space into two, for the TM and SROT
> >> registers. This was required to deal with different address offsets for the
> >> TM and SROT registers across different SoC families.
> >>
> >> 8996 has two TSENS IP blocks, initialise the second one too.
> >>
> >> 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 <amit.kucheria@linaro.org>
> >> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> >> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> >> ---
> >>  arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++--
> >>  1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >> index 8c7f9ca..688e752 100644
> >> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> >> @@ -459,9 +459,19 @@
> >>                       status = "disabled";
> >>               };
> >>
> >> -             tsens0: thermal-sensor@4a8000 {
> >> +             tsens0: thermal-sensor@4a9000 {
> >                                        ~~~~~~
> >
> > I suppose the address of the TM block is used here instead of the SROT
> > address (which is lower) since SROT functionality is currently not
> > used. Would/should this change if/when the driver uses SROT?
> 
> For device tree you're always supposed to use the address of the first
> "reg" listed as the unit address in the node name.  It doesn't matter
> if it's bigger or smaller as long as it's the first one listed.
> 
> The bindings indicate that the TM block should be listed as the first
> register.  This won't change even if you start using SROT.

Thanks for the clarification, there is always something more to learn!

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 5/7] dt: thermal: tsens: Document the fallback DT property for v2 of TSENS IP

[Matthias Kaehlcke]

On Thu, Jul 12, 2018 at 02:09:06PM +0530, Amit Kucheria wrote:
> We want to create common code for v2 of the TSENS IP block that is used in
> a large number of Qualcomm SoCs. "qcom,tsens-v2" should be able to handle
> most of the common functionality start with a common get_temp() function.
> 
> It is also necessary to split out the memory regions for the TM and SROT
> register banks because their offsets are not constant across SoC families.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  .../devicetree/bindings/thermal/qcom-tsens.txt     | 31 +++++++++++++++++-----
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> index 06195e8..b5312a8 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> @@ -1,18 +1,28 @@
>  * QCOM SoC Temperature Sensor (TSENS)
>  
>  Required properties:
> -- compatible :
> - - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> - - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> - - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> +- compatible:
> +  Must be one of the following:
> +    - "qcom,msm8916-tsens" (MSM8916)
> +    - "qcom,msm8974-tsens" (MSM8974)
> +    - "qcom,msm8996-tsens" (MSM8996)
> +    - "qcom,msm8998-tsens", "qcom,tsens-v2" (MSM8998)
> +    - "qcom,sdm845-tsens", "qcom,tsens-v2" (SDM845)
> +  The generic "qcom,tsens-v2" property must be used as a fallback for any SoC
> +  with version 2 of the TSENS IP. MSM8996 is the only exception beacause the

s/beacause/because/

> +  generic property did not exist when support was added.
> +
> +- reg: Address range of the thermal registers.
> +  New platforms containing v2.x.y of the TSENS IP must specify the SROT and TM
> +  register spaces separately, with order being TM before SROT.
> +  See Example 2, below.
>  
> -- reg: Address range of the thermal registers
>  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
>  - #qcom,sensors: Number of sensors in tsens block
>  - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
>  nvmem cells
>  
> -Example:
> +Example 1 (legacy support before a fallback tsens-v2 propoerty was introduced):

s/propoerty/property/

>  tsens: thermal-sensor@900000 {
>  		compatible = "qcom,msm8916-tsens";
>  		reg = <0x4a8000 0x2000>;
> @@ -20,3 +30,12 @@ tsens: thermal-sensor@900000 {
>  		nvmem-cell-names = "caldata", "calsel";
>  		#thermal-sensor-cells = <1>;
>  	};
> +
> +Example 2 (for any platform containing v2 of the TSENS IP):
> +tsens0: thermal-sensor@c263000 {
> +		compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +		reg = <0xc263000 0x1ff>, /* TM */
> +			<0xc222000 0x1ff>; /* SROT */
> +		#qcom,sensors = <13>;
> +		#thermal-sensor-cells = <1>;
> +	};

Besides the typos:

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Oh, and you also might want to reorder the patches as suggested by
Doug on v6 to put the changes in the binding before the code changes.

Re: [PATCH v7 7/7] arm64: dts: sdm845: Add tsens nodes

[Matthias Kaehlcke]

On Thu, Jul 12, 2018 at 02:09:08PM +0530, Amit Kucheria wrote:
> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> sensors. It uses version 2 of the TSENS IP, so use the fallback property to
> allow more common code.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index cdaabeb..01ff146 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -221,6 +221,22 @@
>  			#interrupt-cells = <2>;
>  		};
>  
> +		tsens0: thermal-sensor@c263000 {
> +			compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +			reg = <0xc263000 0x1ff>, /* TM */
> +			      <0xc222000 0x1ff>; /* SROT */
> +			#qcom,sensors = <13>;
> +			#thermal-sensor-cells = <1>;
> +		};
> +
> +		tsens1: thermal-sensor@c265000 {
> +			compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
> +			reg = <0xc265000 0x1ff>, /* TM */
> +			      <0xc223000 0x1ff>; /* SROT */
> +			#qcom,sensors = <8>;
> +			#thermal-sensor-cells = <1>;
> +		};
> +
>  		spmi_bus: spmi@c440000 {
>  			compatible = "qcom,spmi-pmic-arb";
>  			reg = <0xc440000 0x1100>,

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 2/7] thermal: tsens: Add support to split up register address space into two

[Amit Kucheria]

On Wed, Jul 18, 2018 at 4:59 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Thu, Jul 12, 2018 at 02:09:03PM +0530, Amit Kucheria wrote:
> > There are two banks of registers for v2 TSENS IPs: SROT and TM. On older
> > SoCs these were contiguous, leading to DTs mapping them as one register
> > address space of size 0x2000. In newer SoCs, these two banks are not
> > contiguous anymore.
> >
> > Add logic to init_common() to differentiate between old and new DTs and
> > adjust associated offsets for the TM register bank so that the old DTs will
> > continue to function correctly.
> >
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > Tested-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/thermal/qcom/tsens-8996.c   |  4 ++--
> >  drivers/thermal/qcom/tsens-common.c | 12 ++++++++++++
> >  drivers/thermal/qcom/tsens.h        |  1 +
> >  3 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/qcom/tsens-8996.c b/drivers/thermal/qcom/tsens-8996.c
> > index e1f7781..3e60cec 100644
> > --- a/drivers/thermal/qcom/tsens-8996.c
> > +++ b/drivers/thermal/qcom/tsens-8996.c
> > @@ -16,7 +16,7 @@
> >  #include <linux/regmap.h>
> >  #include "tsens.h"
> >
> > -#define STATUS_OFFSET        0x10a0
> > +#define STATUS_OFFSET        0xa0
> >  #define LAST_TEMP_MASK       0xfff
> >  #define STATUS_VALID_BIT     BIT(21)
> >  #define CODE_SIGN_BIT                BIT(11)
> > @@ -28,7 +28,7 @@ static int get_temp_8996(struct tsens_device *tmdev, int id, int *temp)
> >       unsigned int sensor_addr;
> >       int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
> >
> > -     sensor_addr = STATUS_OFFSET + s->hw_id * 4;
> > +     sensor_addr = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> >       ret = regmap_read(tmdev->map, sensor_addr, &code);
> >       if (ret)
> >               return ret;
> > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > index b1449ad..c22dc18 100644
> > --- a/drivers/thermal/qcom/tsens-common.c
> > +++ b/drivers/thermal/qcom/tsens-common.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/io.h>
> >  #include <linux/nvmem-consumer.h>
> >  #include <linux/of_address.h>
> > +#include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> >  #include "tsens.h"
> > @@ -126,11 +127,22 @@ static const struct regmap_config tsens_config = {
> >  int __init init_common(struct tsens_device *tmdev)
> >  {
> >       void __iomem *base;
> > +     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) {
> > +             tmdev->tm_offset = 0;
> > +     } else {
> > +             /* old DTs where SROT and TM were in a contiguous 2K block */
> > +             tmdev->tm_offset = 0x1000;
> > +     }
>
> nit: no curly braces for conditionals with a single statement. There
> is probably no need to respin just for this though.

Yeah, that was leftover from refactoring because I have an upcoming
patch that adds SROT mapping inside those braces. I'm about to post it
soon.

> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v7 5/7] dt: thermal: tsens: Document the fallback DT property for v2 of TSENS IP

[Amit Kucheria]

On Wed, Jul 18, 2018 at 5:39 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> On Thu, Jul 12, 2018 at 02:09:06PM +0530, Amit Kucheria wrote:
>> We want to create common code for v2 of the TSENS IP block that is used in
>> a large number of Qualcomm SoCs. "qcom,tsens-v2" should be able to handle
>> most of the common functionality start with a common get_temp() function.
>>
>> It is also necessary to split out the memory regions for the TM and SROT
>> register banks because their offsets are not constant across SoC families.
>>
>> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> Tested-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>>  .../devicetree/bindings/thermal/qcom-tsens.txt     | 31 +++++++++++++++++-----
>>  1 file changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> index 06195e8..b5312a8 100644
>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> @@ -1,18 +1,28 @@
>>  * QCOM SoC Temperature Sensor (TSENS)
>>
>>  Required properties:
>> -- compatible :
>> - - "qcom,msm8916-tsens" : For 8916 Family of SoCs
>> - - "qcom,msm8974-tsens" : For 8974 Family of SoCs
>> - - "qcom,msm8996-tsens" : For 8996 Family of SoCs
>> +- compatible:
>> +  Must be one of the following:
>> +    - "qcom,msm8916-tsens" (MSM8916)
>> +    - "qcom,msm8974-tsens" (MSM8974)
>> +    - "qcom,msm8996-tsens" (MSM8996)
>> +    - "qcom,msm8998-tsens", "qcom,tsens-v2" (MSM8998)
>> +    - "qcom,sdm845-tsens", "qcom,tsens-v2" (SDM845)
>> +  The generic "qcom,tsens-v2" property must be used as a fallback for any SoC
>> +  with version 2 of the TSENS IP. MSM8996 is the only exception beacause the
>
> s/beacause/because/
>
>> +  generic property did not exist when support was added.
>> +
>> +- reg: Address range of the thermal registers.
>> +  New platforms containing v2.x.y of the TSENS IP must specify the SROT and TM
>> +  register spaces separately, with order being TM before SROT.
>> +  See Example 2, below.
>>
>> -- reg: Address range of the thermal registers
>>  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
>>  - #qcom,sensors: Number of sensors in tsens block
>>  - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
>>  nvmem cells
>>
>> -Example:
>> +Example 1 (legacy support before a fallback tsens-v2 propoerty was introduced):
>
> s/propoerty/property/
>
>>  tsens: thermal-sensor@900000 {
>>               compatible = "qcom,msm8916-tsens";
>>               reg = <0x4a8000 0x2000>;
>> @@ -20,3 +30,12 @@ tsens: thermal-sensor@900000 {
>>               nvmem-cell-names = "caldata", "calsel";
>>               #thermal-sensor-cells = <1>;
>>       };
>> +
>> +Example 2 (for any platform containing v2 of the TSENS IP):
>> +tsens0: thermal-sensor@c263000 {
>> +             compatible = "qcom,sdm845-tsens", "qcom,tsens-v2";
>> +             reg = <0xc263000 0x1ff>, /* TM */
>> +                     <0xc222000 0x1ff>; /* SROT */
>> +             #qcom,sensors = <13>;
>> +             #thermal-sensor-cells = <1>;
>> +     };
>
> Besides the typos:
>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>
> Oh, and you also might want to reorder the patches as suggested by
> Doug on v6 to put the changes in the binding before the code changes.

Ahh, sorry I missed that reordering.