* [PATCH v4 1/6] thermal: tsens: Get rid of unused fields in structure
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 12:44 ` [PATCH v4 2/6] dt: qcom: 8996: thermal: Move to DT initialisation Amit Kucheria
` (6 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, linux-pm
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>
---
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v4 2/6] dt: qcom: 8996: thermal: Move to DT initialisation
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
2018-07-02 12:44 ` [PATCH v4 1/6] thermal: tsens: Get rid of unused fields in structure Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 18:48 ` Bjorn Andersson
2018-07-02 12:44 ` [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse Amit Kucheria
` (5 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, linux-soc, devicetree,
linux-arm-kernel
We also split up the regmap address space into two, one for the TM
registers, the other for the SROT registers. This was required to deal with
different address offsets for the TM and SROT registers across different
SoC families.
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>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 8c7f9ca..6c8a857 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -461,7 +461,17 @@
tsens0: thermal-sensor@4a8000 {
compatible = "qcom,msm8996-tsens";
- reg = <0x4a8000 0x2000>;
+ reg = <0x4a9000 0x1000>, /* TM */
+ <0x4a8000 0x1000>; /* SROT */
+ #qcom,sensors = <13>;
+ #thermal-sensor-cells = <1>;
+ };
+
+ tsens1: thermal-sensor@4ac000 {
+ compatible = "qcom,msm8996-tsens";
+ reg = <0x4ad000 0x1000>, /* TM */
+ <0x4ac000 0x1000>; /* SROT */
+ #qcom,sensors = <8>;
#thermal-sensor-cells = <1>;
};
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v4 2/6] dt: qcom: 8996: thermal: Move to DT initialisation
2018-07-02 12:44 ` [PATCH v4 2/6] dt: qcom: 8996: thermal: Move to DT initialisation Amit Kucheria
@ 2018-07-02 18:48 ` Bjorn Andersson
0 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2018-07-02 18:48 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, edubezval, smohanad,
vivek.gautam, andy.gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, linux-soc, devicetree,
linux-arm-kernel
On Mon 02 Jul 05:44 PDT 2018, Amit Kucheria wrote:
> We also split up the regmap address space into two, one for the TM
> registers, the other for the SROT registers. This was required to deal with
> different address offsets for the TM and SROT registers across different
> SoC families.
>
> 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>
Regards,
Bjorn
> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 8c7f9ca..6c8a857 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -461,7 +461,17 @@
>
> tsens0: thermal-sensor@4a8000 {
> compatible = "qcom,msm8996-tsens";
> - reg = <0x4a8000 0x2000>;
> + reg = <0x4a9000 0x1000>, /* TM */
> + <0x4a8000 0x1000>; /* SROT */
> + #qcom,sensors = <13>;
> + #thermal-sensor-cells = <1>;
> + };
> +
> + tsens1: thermal-sensor@4ac000 {
> + compatible = "qcom,msm8996-tsens";
> + reg = <0x4ad000 0x1000>, /* TM */
> + <0x4ac000 0x1000>; /* SROT */
> + #qcom,sensors = <8>;
> #thermal-sensor-cells = <1>;
> };
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
2018-07-02 12:44 ` [PATCH v4 1/6] thermal: tsens: Get rid of unused fields in structure Amit Kucheria
2018-07-02 12:44 ` [PATCH v4 2/6] dt: qcom: 8996: thermal: Move to DT initialisation Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 18:55 ` Bjorn Andersson
2018-07-02 12:44 ` [PATCH v4 4/6] thermal: tsens: Add support for SDM845 Amit Kucheria
` (4 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, linux-pm
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>
---
drivers/thermal/qcom/Makefile | 2 +-
drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 26 ++++++++---------------
2 files changed, 10 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 e1f7781..2eca7ff 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 0x10a0
-#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,13 @@ 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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
2018-07-02 12:44 ` [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse Amit Kucheria
@ 2018-07-02 18:55 ` Bjorn Andersson
2018-07-04 12:38 ` Amit Kucheria
0 siblings, 1 reply; 28+ messages in thread
From: Bjorn Andersson @ 2018-07-02 18:55 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, linux-pm
On Mon 02 Jul 05:44 PDT 2018, 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>
> ---
> drivers/thermal/qcom/Makefile | 2 +-
> drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 26 ++++++++---------------
> 2 files changed, 10 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 e1f7781..2eca7ff 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 0x10a0
> -#define LAST_TEMP_MASK 0xfff
> +#define STATUS_OFFSET 0xa0
This is not backwards compatible with present day dts files, you need to
keep this effectively 0x10a0 when the memory region isn't split in two.
Perhaps you can just offset the ioremap by 4k when there's only one
region?
Apart from this I like the change!
Regards,
Bjorn
> +#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,13 @@ 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
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
2018-07-02 18:55 ` Bjorn Andersson
@ 2018-07-04 12:38 ` Amit Kucheria
0 siblings, 0 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-04 12:38 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Linux Kernel Mailing List, Rajendra Nayak, linux-arm-msm,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross, Zhang Rui,
Linux PM list
[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]
On Tue, Jul 3, 2018 at 12:23 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 02 Jul 05:44 PDT 2018, 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>
> > ---
> > drivers/thermal/qcom/Makefile | 2 +-
> > drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 26 ++++++++---------------
> > 2 files changed, 10 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 e1f7781..2eca7ff 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 0x10a0
> > -#define LAST_TEMP_MASK 0xfff
> > +#define STATUS_OFFSET 0xa0
>
> This is not backwards compatible with present day dts files, you need to
> keep this effectively 0x10a0 when the memory region isn't split in two.
Argh!! Good catch.
> Perhaps you can just offset the ioremap by 4k when there's only one
> region?
That'll cause problems when we want to access some features exposed in
the other register bank through the common get_temp_tsens_v2 function.
The SW reset bit is in there that I'd like to add support for, for
example.
I can see a few other ways to detect this at runtime. None of them
look pretty but option 3 looks least ugly at least to me. Any
preferences?
1. Don't try to combine get_temp for 8996 with other v2 platforms.
This would mean retaining a separate ops_8996, pointing to a custom
get_temp(). We end up with two copies of the function.
2. In get_temp_tsens_v2(), check for number of regions but this has an
overhead for each temperature read.
struct platform_device *op = of_find_device_by_node(tmdev->dev->of_node);
if (op->num_resources > 1)
status_offset = 0xa0;
else
status_offset = 0x10a0;
Read temperature(status_offset);
3. Restrict to init time (ideal) by storing the status_offset to
struct tsens_device. This is then used directly in
get_temp_generic_v2() to determine the actual address. This should
work for 8996, 8916 and 8974 which all have a single memory region
iomapp'ed. The last two don't even need this value since they have
their own get_temp functions. See diff attached.
I have to ask: Are there really devices in the field that rely on the
existing *upstream* code and DT? If not, could we consider deprecating
the "qcom,msm8996-tsens" property immediately or at least mark it for
future removal?
Regards,
Amit
[-- Attachment #2: 3.diff.txt --]
[-- Type: text/plain, Size: 1942 bytes --]
diff --git i/drivers/thermal/qcom/tsens-common.c w/drivers/thermal/qcom/tsens-common.c
index b1449ad..7982867 100644
--- i/drivers/thermal/qcom/tsens-common.c
+++ w/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,6 +127,12 @@ 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->num_resources > 1)
+ tmdev->status_offset = 0xa0;
+ else
+ tmdev->status_offset = 0x10a0;
base = of_iomap(tmdev->dev->of_node, 0);
if (!base)
diff --git i/drivers/thermal/qcom/tsens-v2.c w/drivers/thermal/qcom/tsens-v2.c
index 0473f33..8400290 100644
--- i/drivers/thermal/qcom/tsens-v2.c
+++ w/drivers/thermal/qcom/tsens-v2.c
@@ -10,7 +10,6 @@
#define TRDY_OFFSET 0xe4
#define TRDY_READY_BIT BIT(0)
-#define STATUS_OFFSET 0xa0
#define LAST_TEMP_MASK 0xfff
#define STATUS_VALID_BIT BIT(21)
#define CODE_SIGN_BIT BIT(11)
@@ -28,7 +27,7 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
if (code & TRDY_READY_BIT)
return -ENODATA;
- sensor_addr = STATUS_OFFSET + s->hw_id * 4;
+ sensor_addr = tmdev->status_offset + s->hw_id * 4;
ret = regmap_read(tmdev->map, sensor_addr, &code);
if (ret)
return ret;
diff --git i/drivers/thermal/qcom/tsens.h w/drivers/thermal/qcom/tsens.h
index 69212cb..8a42a70 100644
--- i/drivers/thermal/qcom/tsens.h
+++ w/drivers/thermal/qcom/tsens.h
@@ -77,6 +77,7 @@ struct tsens_device {
struct device *dev;
u32 num_sensors;
struct regmap *map;
+ u32 status_offset;
struct tsens_context ctx;
const struct tsens_ops *ops;
struct tsens_sensor sensor[0];
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
` (2 preceding siblings ...)
2018-07-02 12:44 ` [PATCH v4 3/6] thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 18:51 ` Bjorn Andersson
2018-07-03 16:26 ` Rob Herring
2018-07-02 12:44 ` [PATCH v4 5/6] thermal: tsens: Check if we have valid data before reading Amit Kucheria
` (3 subsequent siblings)
7 siblings, 2 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, Rob Herring, Mark Rutland,
Kees Cook, linux-pm, devicetree
SDM845 uses v2.4.0 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.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
drivers/thermal/qcom/tsens-v2.c | 6 +++++-
drivers/thermal/qcom/tsens.c | 6 ++++++
drivers/thermal/qcom/tsens.h | 5 ++++-
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 06195e8..075182e 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -5,6 +5,8 @@ Required properties:
- "qcom,msm8916-tsens" : For 8916 Family of SoCs
- "qcom,msm8974-tsens" : For 8974 Family of SoCs
- "qcom,msm8996-tsens" : For 8996 Family of SoCs
+ - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
+ - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
- reg: Address range of the thermal registers
- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 2eca7ff..1871945 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -69,8 +69,12 @@ 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..992ed17 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -72,6 +72,12 @@ static const struct of_device_id tsens_table[] = {
}, {
.compatible = "qcom,msm8996-tsens",
.data = &data_8996,
+ }, {
+ .compatible = "qcom,tsens-v2.4.0",
+ .data = &data_tsens_v2,
+ }, {
+ .compatible = "qcom,tsens-v2",
+ .data = &data_tsens_v2,
},
{}
};
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index dc56e1e..69212cb 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -87,6 +87,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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-02 12:44 ` [PATCH v4 4/6] thermal: tsens: Add support for SDM845 Amit Kucheria
@ 2018-07-02 18:51 ` Bjorn Andersson
2018-07-03 16:26 ` Rob Herring
1 sibling, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2018-07-02 18:51 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, Rob Herring, Mark Rutland,
Kees Cook, linux-pm, devicetree
On Mon 02 Jul 05:44 PDT 2018, Amit Kucheria wrote:
> SDM845 uses v2.4.0 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.
>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
> drivers/thermal/qcom/tsens.c | 6 ++++++
> drivers/thermal/qcom/tsens.h | 5 ++++-
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> index 06195e8..075182e 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> @@ -5,6 +5,8 @@ Required properties:
> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
>
> - reg: Address range of the thermal registers
> - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 2eca7ff..1871945 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -69,8 +69,12 @@ 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..992ed17 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -72,6 +72,12 @@ static const struct of_device_id tsens_table[] = {
> }, {
> .compatible = "qcom,msm8996-tsens",
> .data = &data_8996,
> + }, {
> + .compatible = "qcom,tsens-v2.4.0",
> + .data = &data_tsens_v2,
> + }, {
> + .compatible = "qcom,tsens-v2",
> + .data = &data_tsens_v2,
> },
> {}
> };
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index dc56e1e..69212cb 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -87,6 +87,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
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-02 12:44 ` [PATCH v4 4/6] thermal: tsens: Add support for SDM845 Amit Kucheria
2018-07-02 18:51 ` Bjorn Andersson
@ 2018-07-03 16:26 ` Rob Herring
2018-07-04 17:26 ` Amit Kucheria
1 sibling, 1 reply; 28+ messages in thread
From: Rob Herring @ 2018-07-03 16:26 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, bjorn.andersson, edubezval,
smohanad, vivek.gautam, andy.gross, Zhang Rui, Mark Rutland,
Kees Cook, linux-pm, devicetree
On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote:
> SDM845 uses v2.4.0 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.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
> drivers/thermal/qcom/tsens.c | 6 ++++++
> drivers/thermal/qcom/tsens.h | 5 ++++-
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> index 06195e8..075182e 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> @@ -5,6 +5,8 @@ Required properties:
> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
You need to show what are valid combinations of compatibles. Does v2
apply to 8996? One valid combination per line.
Rob
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-03 16:26 ` Rob Herring
@ 2018-07-04 17:26 ` Amit Kucheria
2018-07-05 20:37 ` Rob Herring
0 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-04 17:26 UTC (permalink / raw)
To: Rob Herring
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross, Zhang Rui,
Mark Rutland, Kees Cook, Linux PM list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Tue, Jul 3, 2018 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote:
>> SDM845 uses v2.4.0 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.
>>
>> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
>> ---
>> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
>> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
>> drivers/thermal/qcom/tsens.c | 6 ++++++
>> drivers/thermal/qcom/tsens.h | 5 ++++-
>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> index 06195e8..075182e 100644
>> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
>> @@ -5,6 +5,8 @@ Required properties:
>> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
>> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
>> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
>> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
>> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
>
> You need to show what are valid combinations of compatibles. Does v2
> apply to 8996? One valid combination per line.
I've restructured qcom-tsens.txt to look like this:
-----%<-------
* QCOM SoC Temperature Sensor (TSENS)
Required properties:
- compatible: must be one of the following:
- "qcom,msm8916-tsens" (MSM8916)
- "qcom,msm8974-tsens" (MSM8974)
- "qcom,msm8996-tsens" (MSM8996)
- "qcom,tsens-<ip_version>", "qcom,tsens-v2" (TSENS IP version and a
generic v2 property as fallback except for MSM8996)
Examples with ip_version are:
- "qcom,tsens-v2.4.0", "qcom,tsens-v2" (SDM845)
- "qcom,tsens-v2.2.1", "qcom,tsens-v2" (MSM8998)
-----%<-------
8996 would end up being something like this if needed, though we're
stuck with "qcom,msm8996-tsens":
"qcom,msm8996-tsens", "qcom,tsens-v2.1.0", "qcom,tsens-v2" (MSM8996)
If this looks good, I'll respin the patches.
Regards,
Amit
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-04 17:26 ` Amit Kucheria
@ 2018-07-05 20:37 ` Rob Herring
2018-07-06 5:13 ` Amit Kucheria
0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2018-07-05 20:37 UTC (permalink / raw)
To: Amit Kucheria
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross, Zhang Rui,
Mark Rutland, Kees Cook, Linux PM list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Wed, Jul 04, 2018 at 10:56:26PM +0530, Amit Kucheria wrote:
> On Tue, Jul 3, 2018 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote:
> >> SDM845 uses v2.4.0 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.
> >>
> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> >> ---
> >> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> >> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
> >> drivers/thermal/qcom/tsens.c | 6 ++++++
> >> drivers/thermal/qcom/tsens.h | 5 ++++-
> >> 4 files changed, 17 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> >> index 06195e8..075182e 100644
> >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> >> @@ -5,6 +5,8 @@ Required properties:
> >> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> >> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> >> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> >> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
> >> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
> >
> > You need to show what are valid combinations of compatibles. Does v2
> > apply to 8996? One valid combination per line.
>
> I've restructured qcom-tsens.txt to look like this:
>
> -----%<-------
>
> * QCOM SoC Temperature Sensor (TSENS)
>
> Required properties:
> - compatible: must be one of the following:
> - "qcom,msm8916-tsens" (MSM8916)
> - "qcom,msm8974-tsens" (MSM8974)
> - "qcom,msm8996-tsens" (MSM8996)
> - "qcom,tsens-<ip_version>", "qcom,tsens-v2" (TSENS IP version and a
> generic v2 property as fallback except for MSM8996)
>
> Examples with ip_version are:
> - "qcom,tsens-v2.4.0", "qcom,tsens-v2" (SDM845)
> - "qcom,tsens-v2.2.1", "qcom,tsens-v2" (MSM8998)
>
> -----%<-------
>
> 8996 would end up being something like this if needed, though we're
> stuck with "qcom,msm8996-tsens":
> "qcom,msm8996-tsens", "qcom,tsens-v2.1.0", "qcom,tsens-v2" (MSM8996)
3 versions here for 3 SoCs. I'm not getting that convinced version
numbers really are better. I would assume that other QCom IP blocks
have versions too, but pretty much *every* *other* binding uses SoC names.
Why is this one special? The other problem with versions is the mapping
of versions to SoCs most likely can't be validated outside of QCom
unless there's a version register.
So, sorry to go in circles, but can you go back to qcom,<soc>-tsens. You
can keep qcom,tsens-v2 as a fallback.
Yes, it's annoying to have to update bindings for new SoCs. But it's
trivial one line patches. Look at Renesas bindings. Maybe adding new
ones will be scriptable once we move to json-schema binding docs.
Rob
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-05 20:37 ` Rob Herring
@ 2018-07-06 5:13 ` Amit Kucheria
2018-07-06 17:47 ` Rob Herring
0 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-06 5:13 UTC (permalink / raw)
To: Rob Herring
Cc: Linux Kernel Mailing List, Rajendra Nayak, linux-arm-msm,
Bjorn Andersson, Eduardo Valentin, smohanad, Vivek Gautam,
Andy Gross, Zhang Rui, Mark Rutland, Kees Cook, Linux PM list,
DTML
On Fri, Jul 6, 2018 at 2:07 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Jul 04, 2018 at 10:56:26PM +0530, Amit Kucheria wrote:
> > On Tue, Jul 3, 2018 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> > > On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote:
> > >> SDM845 uses v2.4.0 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.
> > >>
> > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > >> ---
> > >> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> > >> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
> > >> drivers/thermal/qcom/tsens.c | 6 ++++++
> > >> drivers/thermal/qcom/tsens.h | 5 ++++-
> > >> 4 files changed, 17 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > >> index 06195e8..075182e 100644
> > >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > >> @@ -5,6 +5,8 @@ Required properties:
> > >> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> > >> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> > >> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> > >> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
> > >> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
> > >
> > > You need to show what are valid combinations of compatibles. Does v2
> > > apply to 8996? One valid combination per line.
> >
> > I've restructured qcom-tsens.txt to look like this:
> >
> > -----%<-------
> >
> > * QCOM SoC Temperature Sensor (TSENS)
> >
> > Required properties:
> > - compatible: must be one of the following:
> > - "qcom,msm8916-tsens" (MSM8916)
> > - "qcom,msm8974-tsens" (MSM8974)
> > - "qcom,msm8996-tsens" (MSM8996)
> > - "qcom,tsens-<ip_version>", "qcom,tsens-v2" (TSENS IP version and a
> > generic v2 property as fallback except for MSM8996)
> >
> > Examples with ip_version are:
> > - "qcom,tsens-v2.4.0", "qcom,tsens-v2" (SDM845)
> > - "qcom,tsens-v2.2.1", "qcom,tsens-v2" (MSM8998)
> >
> > -----%<-------
> >
> > 8996 would end up being something like this if needed, though we're
> > stuck with "qcom,msm8996-tsens":
> > "qcom,msm8996-tsens", "qcom,tsens-v2.1.0", "qcom,tsens-v2" (MSM8996)
>
> 3 versions here for 3 SoCs. I'm not getting that convinced version
> numbers really are better. I would assume that other QCom IP blocks
Yeah, it is a bit unfortunate that the 3-4 SoCs we're focusing on
getting supported upstream have different versions of the TSENS IP.
The other goal of this work was to make the upstream driver
feature-complete so we can make a case to switch to it in the
downstream trees, even on platforms that aren't being active
upstreamed. They'll still need to keep around those SoC-specific
one-liner patches in the downstream trees.
> have versions too, but pretty much *every* *other* binding uses SoC names.
> Why is this one special?
I'm not an expert on all QC IPs, but this one _seems_ to be reused a
lot more than others.
> The other problem with versions is the mapping
> of versions to SoCs most likely can't be validated outside of QCom
> unless there's a version register.
There is in fact a HW version register that I was hoping to add
support for later.
> So, sorry to go in circles, but can you go back to qcom,<soc>-tsens. You
> can keep qcom,tsens-v2 as a fallback.
OK.
> Yes, it's annoying to have to update bindings for new SoCs. But it's
> trivial one line patches. Look at Renesas bindings. Maybe adding new
> ones will be scriptable once we move to json-schema binding docs.
I did look at the Renesas RCar bindings when restructuring the
documentation. They seem to have settled upon a 3-level fallback:
"soc-specific", "generic-no TZ", "generic-TZ". But
drivers/thermal/rcar_thermal.c seems to be compatible with only one
soc-specific property (thermal-r8a77995) and all other SoCs seem to be
just relying the fallbacks.
Anyways, I'll respin.
Thanks.
Regards,
Amit
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 4/6] thermal: tsens: Add support for SDM845
2018-07-06 5:13 ` Amit Kucheria
@ 2018-07-06 17:47 ` Rob Herring
0 siblings, 0 replies; 28+ messages in thread
From: Rob Herring @ 2018-07-06 17:47 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, Siddartha Mohanadoss, Vivek Gautam, Andy Gross,
Zhang Rui, Mark Rutland, Kees Cook, open list:THERMAL,
devicetree
On Thu, Jul 5, 2018 at 11:13 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> On Fri, Jul 6, 2018 at 2:07 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Jul 04, 2018 at 10:56:26PM +0530, Amit Kucheria wrote:
> > > On Tue, Jul 3, 2018 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
> > > > On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote:
> > > >> SDM845 uses v2.4.0 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.
> > > >>
> > > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > > >> ---
> > > >> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> > > >> drivers/thermal/qcom/tsens-v2.c | 6 +++++-
> > > >> drivers/thermal/qcom/tsens.c | 6 ++++++
> > > >> drivers/thermal/qcom/tsens.h | 5 ++++-
> > > >> 4 files changed, 17 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > >> index 06195e8..075182e 100644
> > > >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > >> @@ -5,6 +5,8 @@ Required properties:
> > > >> - "qcom,msm8916-tsens" : For 8916 Family of SoCs
> > > >> - "qcom,msm8974-tsens" : For 8974 Family of SoCs
> > > >> - "qcom,msm8996-tsens" : For 8996 Family of SoCs
> > > >> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs
> > > >> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP
> > > >
> > > > You need to show what are valid combinations of compatibles. Does v2
> > > > apply to 8996? One valid combination per line.
> > >
> > > I've restructured qcom-tsens.txt to look like this:
> > >
> > > -----%<-------
> > >
> > > * QCOM SoC Temperature Sensor (TSENS)
> > >
> > > Required properties:
> > > - compatible: must be one of the following:
> > > - "qcom,msm8916-tsens" (MSM8916)
> > > - "qcom,msm8974-tsens" (MSM8974)
> > > - "qcom,msm8996-tsens" (MSM8996)
> > > - "qcom,tsens-<ip_version>", "qcom,tsens-v2" (TSENS IP version and a
> > > generic v2 property as fallback except for MSM8996)
> > >
> > > Examples with ip_version are:
> > > - "qcom,tsens-v2.4.0", "qcom,tsens-v2" (SDM845)
> > > - "qcom,tsens-v2.2.1", "qcom,tsens-v2" (MSM8998)
> > >
> > > -----%<-------
> > >
> > > 8996 would end up being something like this if needed, though we're
> > > stuck with "qcom,msm8996-tsens":
> > > "qcom,msm8996-tsens", "qcom,tsens-v2.1.0", "qcom,tsens-v2" (MSM8996)
> >
> > 3 versions here for 3 SoCs. I'm not getting that convinced version
> > numbers really are better. I would assume that other QCom IP blocks
>
> Yeah, it is a bit unfortunate that the 3-4 SoCs we're focusing on
> getting supported upstream have different versions of the TSENS IP.
> The other goal of this work was to make the upstream driver
> feature-complete so we can make a case to switch to it in the
> downstream trees, even on platforms that aren't being active
> upstreamed. They'll still need to keep around those SoC-specific
> one-liner patches in the downstream trees.
Why? They can just use the "v2" fallback (which was sufficient for
your original version). That's not my recommendation, but what do I
care for downstream. Only upstream needs the specific strings to
appease the annoying DT maintainers.
Plus, if it's not upstream, it doesn't exist. :)
> > have versions too, but pretty much *every* *other* binding uses SoC names.
> > Why is this one special?
>
> I'm not an expert on all QC IPs, but this one _seems_ to be reused a
> lot more than others.
>
> > The other problem with versions is the mapping
> > of versions to SoCs most likely can't be validated outside of QCom
> > unless there's a version register.
>
> There is in fact a HW version register that I was hoping to add
> support for later.
Yes, you should rely on that as much as possible. But I have seen h/w
designers forget to update revision registers or there can be
integration differences even if the IP version is the same.
> > So, sorry to go in circles, but can you go back to qcom,<soc>-tsens. You
> > can keep qcom,tsens-v2 as a fallback.
>
> OK.
>
> > Yes, it's annoying to have to update bindings for new SoCs. But it's
> > trivial one line patches. Look at Renesas bindings. Maybe adding new
> > ones will be scriptable once we move to json-schema binding docs.
>
> I did look at the Renesas RCar bindings when restructuring the
> documentation. They seem to have settled upon a 3-level fallback:
> "soc-specific", "generic-no TZ", "generic-TZ". But
> drivers/thermal/rcar_thermal.c seems to be compatible with only one
> soc-specific property (thermal-r8a77995) and all other SoCs seem to be
> just relying the fallbacks.
>
> Anyways, I'll respin.
>
> Thanks.
>
> Regards,
> Amit
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 5/6] thermal: tsens: Check if we have valid data before reading
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
` (3 preceding siblings ...)
2018-07-02 12:44 ` [PATCH v4 4/6] thermal: tsens: Add support for SDM845 Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 17:47 ` Eduardo Valentin
2018-07-02 12:44 ` [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes Amit Kucheria
` (2 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, Zhang Rui, linux-pm
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/thermal/qcom/tsens-v2.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 1871945..0473f33 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -7,6 +7,9 @@
#include <linux/regmap.h>
#include "tsens.h"
+#define TRDY_OFFSET 0xe4
+#define TRDY_READY_BIT BIT(0)
+
#define STATUS_OFFSET 0xa0
#define LAST_TEMP_MASK 0xfff
#define STATUS_VALID_BIT BIT(21)
@@ -19,6 +22,12 @@ static int get_temp_tsens_v2(struct tsens_device *tmdev, int id, int *temp)
unsigned int sensor_addr;
int last_temp = 0, last_temp2 = 0, last_temp3 = 0, ret;
+ ret = regmap_read(tmdev->map, TRDY_OFFSET, &code);
+ if (ret)
+ return ret;
+ if (code & TRDY_READY_BIT)
+ return -ENODATA;
+
sensor_addr = STATUS_OFFSET + s->hw_id * 4;
ret = regmap_read(tmdev->map, sensor_addr, &code);
if (ret)
--
2.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v4 5/6] thermal: tsens: Check if we have valid data before reading
2018-07-02 12:44 ` [PATCH v4 5/6] thermal: tsens: Check if we have valid data before reading Amit Kucheria
@ 2018-07-02 17:47 ` Eduardo Valentin
0 siblings, 0 replies; 28+ messages in thread
From: Eduardo Valentin @ 2018-07-02 17:47 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, bjorn.andersson, smohanad,
vivek.gautam, andy.gross, Zhang Rui, linux-pm
On Mon, Jul 02, 2018 at 06:14:08PM +0530, Amit Kucheria wrote:
Empty description, cannot apply. Please describe your patch.
Thanks
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
` (4 preceding siblings ...)
2018-07-02 12:44 ` [PATCH v4 5/6] thermal: tsens: Check if we have valid data before reading Amit Kucheria
@ 2018-07-02 12:44 ` Amit Kucheria
2018-07-02 18:50 ` Bjorn Andersson
` (2 more replies)
2018-07-02 14:00 ` [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
2018-07-02 17:53 ` Eduardo Valentin
7 siblings, 3 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 12:44 UTC (permalink / raw)
To: linux-kernel
Cc: rnayak, linux-arm-msm, bjorn.andersson, edubezval, smohanad,
vivek.gautam, andy.gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, linux-soc, devicetree,
linux-arm-kernel
SDM845 has two tsens blocks, one with 13 sensors and the other with 8
sensors.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
+ compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
+ reg = <0xc263000 0x1ff>, /* TM */
+ <0xc222000 0x1ff>; /* SROT */
+ #qcom,sensors = <13>;
+ #thermal-sensor-cells = <1>;
+ };
+
+ tsens1: tsens@c223000 {
+ compatible = "qcom,tsens-v2.4.0", "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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-02 12:44 ` [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes Amit Kucheria
@ 2018-07-02 18:50 ` Bjorn Andersson
2018-07-03 21:48 ` Matthias Kaehlcke
2018-07-06 17:31 ` Stephen Boyd
2 siblings, 0 replies; 28+ messages in thread
From: Bjorn Andersson @ 2018-07-02 18:50 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, edubezval, smohanad,
vivek.gautam, andy.gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon, linux-soc, devicetree,
linux-arm-kernel
On Mon 02 Jul 05:44 PDT 2018, Amit Kucheria wrote:
> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> sensors.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> 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..edf8505 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: tsens@c222000 {
> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> + reg = <0xc263000 0x1ff>, /* TM */
> + <0xc222000 0x1ff>; /* SROT */
> + #qcom,sensors = <13>;
> + #thermal-sensor-cells = <1>;
> + };
> +
> + tsens1: tsens@c223000 {
> + compatible = "qcom,tsens-v2.4.0", "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
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-02 12:44 ` [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes Amit Kucheria
2018-07-02 18:50 ` Bjorn Andersson
@ 2018-07-03 21:48 ` Matthias Kaehlcke
2018-07-09 11:26 ` Amit Kucheria
2018-07-06 17:31 ` Stephen Boyd
2 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2018-07-03 21:48 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, bjorn.andersson, edubezval,
smohanad, vivek.gautam, andy.gross, David Brown, Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon, linux-soc,
devicetree, linux-arm-kernel, Douglas Anderson
Hi,
On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote:
> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> sensors.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> + reg = <0xc263000 0x1ff>, /* TM */
> + <0xc222000 0x1ff>; /* SROT */
> + #qcom,sensors = <13>;
> + #thermal-sensor-cells = <1>;
> + };
> +
> + tsens1: tsens@c223000 {
> + compatible = "qcom,tsens-v2.4.0", "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>,
I tested this series on an SDM845 board. Temperatures can be read from
the tsens0 sensors, however for tsens1 sensors I get:
cat temp
cat: temp: No data available
TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2().
Unfortunately I currently have no documentation of the registers or the
thermal sensors in general and couldn't investigate further.
Any idea what could be the problem?
Thanks
Matthias
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-03 21:48 ` Matthias Kaehlcke
@ 2018-07-09 11:26 ` Amit Kucheria
2018-07-09 16:01 ` Matthias Kaehlcke
0 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-09 11:26 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross,
David Brown, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Douglas Anderson
On Wed, Jul 4, 2018 at 3:18 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Hi,
>
> On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote:
>> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
>> sensors.
>>
>> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
>> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
>> + reg = <0xc263000 0x1ff>, /* TM */
>> + <0xc222000 0x1ff>; /* SROT */
>> + #qcom,sensors = <13>;
>> + #thermal-sensor-cells = <1>;
>> + };
>> +
>> + tsens1: tsens@c223000 {
>> + compatible = "qcom,tsens-v2.4.0", "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>,
>
> I tested this series on an SDM845 board. Temperatures can be read from
> the tsens0 sensors, however for tsens1 sensors I get:
>
> cat temp
> cat: temp: No data available
>
> TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2().
> Unfortunately I currently have no documentation of the registers or the
> thermal sensors in general and couldn't investigate further.
>
> Any idea what could be the problem?
Strangely enough, I'm seeing this now after rebuilding my integration
branch to test some other bits.
I've sent another version with this patch removed while I investigate
this. Could you check if you're able to read all 8 sensors with v5?
Regards,
Amit
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-09 11:26 ` Amit Kucheria
@ 2018-07-09 16:01 ` Matthias Kaehlcke
2018-07-09 16:04 ` Amit Kucheria
0 siblings, 1 reply; 28+ messages in thread
From: Matthias Kaehlcke @ 2018-07-09 16:01 UTC (permalink / raw)
To: Amit Kucheria
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross,
David Brown, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Douglas Anderson
On Mon, Jul 09, 2018 at 04:56:51PM +0530, Amit Kucheria wrote:
> On Wed, Jul 4, 2018 at 3:18 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > Hi,
> >
> > On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote:
> >> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> >> sensors.
> >>
> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
> >> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> >> + reg = <0xc263000 0x1ff>, /* TM */
> >> + <0xc222000 0x1ff>; /* SROT */
> >> + #qcom,sensors = <13>;
> >> + #thermal-sensor-cells = <1>;
> >> + };
> >> +
> >> + tsens1: tsens@c223000 {
> >> + compatible = "qcom,tsens-v2.4.0", "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>,
> >
> > I tested this series on an SDM845 board. Temperatures can be read from
> > the tsens0 sensors, however for tsens1 sensors I get:
> >
> > cat temp
> > cat: temp: No data available
> >
> > TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2().
> > Unfortunately I currently have no documentation of the registers or the
> > thermal sensors in general and couldn't investigate further.
> >
> > Any idea what could be the problem?
>
> Strangely enough, I'm seeing this now after rebuilding my integration
> branch to test some other bits.
>
> I've sent another version with this patch removed while I investigate
> this. Could you check if you're able to read all 8 sensors with v5?
With v6 all sensors can be read, thanks!
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-09 16:01 ` Matthias Kaehlcke
@ 2018-07-09 16:04 ` Amit Kucheria
2018-07-09 16:10 ` Matthias Kaehlcke
0 siblings, 1 reply; 28+ messages in thread
From: Amit Kucheria @ 2018-07-09 16:04 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross,
David Brown, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Douglas Anderson
On Mon, Jul 9, 2018 at 9:31 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> On Mon, Jul 09, 2018 at 04:56:51PM +0530, Amit Kucheria wrote:
>> On Wed, Jul 4, 2018 at 3:18 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
>> > Hi,
>> >
>> > On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote:
>> >> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
>> >> sensors.
>> >>
>> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
>> >> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
>> >> + reg = <0xc263000 0x1ff>, /* TM */
>> >> + <0xc222000 0x1ff>; /* SROT */
>> >> + #qcom,sensors = <13>;
>> >> + #thermal-sensor-cells = <1>;
>> >> + };
>> >> +
>> >> + tsens1: tsens@c223000 {
>> >> + compatible = "qcom,tsens-v2.4.0", "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>,
>> >
>> > I tested this series on an SDM845 board. Temperatures can be read from
>> > the tsens0 sensors, however for tsens1 sensors I get:
>> >
>> > cat temp
>> > cat: temp: No data available
>> >
>> > TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2().
>> > Unfortunately I currently have no documentation of the registers or the
>> > thermal sensors in general and couldn't investigate further.
>> >
>> > Any idea what could be the problem?
>>
>> Strangely enough, I'm seeing this now after rebuilding my integration
>> branch to test some other bits.
>>
>> I've sent another version with this patch removed while I investigate
>> this. Could you check if you're able to read all 8 sensors with v5?
>
> With v6 all sensors can be read, thanks!
Great! Would it be OK to add your Tested-by tag?
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-09 16:04 ` Amit Kucheria
@ 2018-07-09 16:10 ` Matthias Kaehlcke
0 siblings, 0 replies; 28+ messages in thread
From: Matthias Kaehlcke @ 2018-07-09 16:10 UTC (permalink / raw)
To: Amit Kucheria
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson,
Eduardo Valentin, smohanad, Vivek Gautam, Andy Gross,
David Brown, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Douglas Anderson
On Mon, Jul 09, 2018 at 09:34:55PM +0530, Amit Kucheria wrote:
> On Mon, Jul 9, 2018 at 9:31 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > On Mon, Jul 09, 2018 at 04:56:51PM +0530, Amit Kucheria wrote:
> >> On Wed, Jul 4, 2018 at 3:18 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> >> > Hi,
> >> >
> >> > On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote:
> >> >> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> >> >> sensors.
> >> >>
> >> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
> >> >> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> >> >> + reg = <0xc263000 0x1ff>, /* TM */
> >> >> + <0xc222000 0x1ff>; /* SROT */
> >> >> + #qcom,sensors = <13>;
> >> >> + #thermal-sensor-cells = <1>;
> >> >> + };
> >> >> +
> >> >> + tsens1: tsens@c223000 {
> >> >> + compatible = "qcom,tsens-v2.4.0", "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>,
> >> >
> >> > I tested this series on an SDM845 board. Temperatures can be read from
> >> > the tsens0 sensors, however for tsens1 sensors I get:
> >> >
> >> > cat temp
> >> > cat: temp: No data available
> >> >
> >> > TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2().
> >> > Unfortunately I currently have no documentation of the registers or the
> >> > thermal sensors in general and couldn't investigate further.
> >> >
> >> > Any idea what could be the problem?
> >>
> >> Strangely enough, I'm seeing this now after rebuilding my integration
> >> branch to test some other bits.
> >>
> >> I've sent another version with this patch removed while I investigate
> >> this. Could you check if you're able to read all 8 sensors with v5?
> >
> > With v6 all sensors can be read, thanks!
>
> Great! Would it be OK to add your Tested-by tag?
Yep
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-02 12:44 ` [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes Amit Kucheria
2018-07-02 18:50 ` Bjorn Andersson
2018-07-03 21:48 ` Matthias Kaehlcke
@ 2018-07-06 17:31 ` Stephen Boyd
2018-07-09 11:27 ` Amit Kucheria
2 siblings, 1 reply; 28+ messages in thread
From: Stephen Boyd @ 2018-07-06 17:31 UTC (permalink / raw)
To: Amit Kucheria, linux-kernel
Cc: Mark Rutland, devicetree, rnayak, linux-arm-msm, Will Deacon,
smohanad, bjorn.andersson, edubezval, David Brown, Rob Herring,
vivek.gautam, Catalin Marinas, andy.gross, linux-soc,
linux-arm-kernel
Quoting Amit Kucheria (2018-07-02 05:44:09)
> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
> sensors.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
These unit addresses should match the first reg property, not the second
one.
> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> + reg = <0xc263000 0x1ff>, /* TM */
> + <0xc222000 0x1ff>; /* SROT */
> + #qcom,sensors = <13>;
> + #thermal-sensor-cells = <1>;
> + };
> +
> + tsens1: tsens@c223000 {
> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
> + reg = <0xc265000 0x1ff>, /* TM */
> + <0xc223000 0x1ff>; /* SROT */
> + #qcom,sensors = <8>;
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes
2018-07-06 17:31 ` Stephen Boyd
@ 2018-07-09 11:27 ` Amit Kucheria
0 siblings, 0 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-09 11:27 UTC (permalink / raw)
To: Stephen Boyd
Cc: LKML, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, linux-arm-msm, Will Deacon, smohanad,
Bjorn Andersson, Eduardo Valentin, David Brown, Rob Herring,
Vivek Gautam, Catalin Marinas, Andy Gross,
open list:ARM/QUALCOMM SUPPORT, lakml
On Fri, Jul 6, 2018 at 11:01 PM, Stephen Boyd <swboyd@chromium.org> wrote:
> Quoting Amit Kucheria (2018-07-02 05:44:09)
>> SDM845 has two tsens blocks, one with 13 sensors and the other with 8
>> sensors.
>>
>> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 {
>
> These unit addresses should match the first reg property, not the second
> one.
Fixed in v5. Thanks.
>> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
>> + reg = <0xc263000 0x1ff>, /* TM */
>> + <0xc222000 0x1ff>; /* SROT */
>> + #qcom,sensors = <13>;
>> + #thermal-sensor-cells = <1>;
>> + };
>> +
>> + tsens1: tsens@c223000 {
>> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2";
>> + reg = <0xc265000 0x1ff>, /* TM */
>> + <0xc223000 0x1ff>; /* SROT */
>> + #qcom,sensors = <8>;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
` (5 preceding siblings ...)
2018-07-02 12:44 ` [PATCH v4 6/6] arm64: dts: sdm845: Add tsens nodes Amit Kucheria
@ 2018-07-02 14:00 ` Amit Kucheria
2018-07-02 17:53 ` Eduardo Valentin
7 siblings, 0 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-02 14:00 UTC (permalink / raw)
To: LKML
Cc: Rajendra Nayak, linux-arm-msm, Bjorn Andersson, Eduardo Valentin,
smohanad, Vivek Gautam, Andy Gross, Kees Cook, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Linux PM list, open list:ARM/QUALCOMM SUPPORT
On Mon, Jul 2, 2018 at 6:14 PM, Amit Kucheria <amit.kucheria@linaro.org> wrote:
> This series is a mixed bag:
> - Some code moves to allow code sharing between various v2.x.y versions of
> the TSENS IP,
> - new qcom,tsens-v2.4.0 DT property for SDM845 and 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.
>
> Rob mentioned offline that we should expose the full version string of the
> TSENS IP (x.y.z) and have a fallback compatible. I hope patch 4 does what
> you were looking for.
>
> Regards,
> Amit
Forgot to append the changelog. Here it is.
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 (6):
> thermal: tsens: Get rid of unused fields in structure
> dt: qcom: 8996: thermal: Move to DT initialisation
> thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
> thermal: tsens: Add support for SDM845
> thermal: tsens: Check if we have valid data before reading
> arm64: dts: sdm845: Add tsens nodes
>
> .../devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 12 ++++++-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 +++++++++
> drivers/thermal/qcom/Makefile | 2 +-
> drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 39 ++++++++++++----------
> drivers/thermal/qcom/tsens.c | 6 ++++
> drivers/thermal/qcom/tsens.h | 7 ++--
> 7 files changed, 62 insertions(+), 22 deletions(-)
> rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%)
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP
2018-07-02 12:44 [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
` (6 preceding siblings ...)
2018-07-02 14:00 ` [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP Amit Kucheria
@ 2018-07-02 17:53 ` Eduardo Valentin
2018-07-09 11:30 ` Amit Kucheria
7 siblings, 1 reply; 28+ messages in thread
From: Eduardo Valentin @ 2018-07-02 17:53 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, rnayak, linux-arm-msm, bjorn.andersson, smohanad,
vivek.gautam, andy.gross, Kees Cook, Rob Herring, devicetree,
linux-arm-kernel, linux-pm, linux-soc
On Mon, Jul 02, 2018 at 06:14:03PM +0530, Amit Kucheria wrote:
> This series is a mixed bag:
> - Some code moves to allow code sharing between various v2.x.y versions of
> the TSENS IP,
> - new qcom,tsens-v2.4.0 DT property for SDM845 and 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.
>
> Rob mentioned offline that we should expose the full version string of the
> TSENS IP (x.y.z) and have a fallback compatible. I hope patch 4 does what
> you were looking for.
Applied patches 1, 3, and 4. Patch 5 needs a description. Patches 2 and
6 go via your arch tree.
>
> Regards,
> Amit
>
> Amit Kucheria (6):
> thermal: tsens: Get rid of unused fields in structure
> dt: qcom: 8996: thermal: Move to DT initialisation
> thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse
> thermal: tsens: Add support for SDM845
> thermal: tsens: Check if we have valid data before reading
> arm64: dts: sdm845: Add tsens nodes
>
> .../devicetree/bindings/thermal/qcom-tsens.txt | 2 ++
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 12 ++++++-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 +++++++++
> drivers/thermal/qcom/Makefile | 2 +-
> drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 39 ++++++++++++----------
> drivers/thermal/qcom/tsens.c | 6 ++++
> drivers/thermal/qcom/tsens.h | 7 ++--
> 7 files changed, 62 insertions(+), 22 deletions(-)
> rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%)
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v4 0/6] thermal: tsens: Refactoring for TSENSv2 IP
2018-07-02 17:53 ` Eduardo Valentin
@ 2018-07-09 11:30 ` Amit Kucheria
0 siblings, 0 replies; 28+ messages in thread
From: Amit Kucheria @ 2018-07-09 11:30 UTC (permalink / raw)
To: Eduardo Valentin
Cc: LKML, Rajendra Nayak, linux-arm-msm, Bjorn Andersson, smohanad,
Vivek Gautam, Andy Gross, Kees Cook, Rob Herring,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
lakml, Linux PM list, open list:ARM/QUALCOMM SUPPORT
On Mon, Jul 2, 2018 at 11:23 PM, Eduardo Valentin <edubezval@gmail.com> wrote:
> On Mon, Jul 02, 2018 at 06:14:03PM +0530, Amit Kucheria wrote:
>> This series is a mixed bag:
>> - Some code moves to allow code sharing between various v2.x.y versions of
>> the TSENS IP,
>> - new qcom,tsens-v2.4.0 DT property for SDM845 and 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.
>>
>> Rob mentioned offline that we should expose the full version string of the
>> TSENS IP (x.y.z) and have a fallback compatible. I hope patch 4 does what
>> you were looking for.
>
> Applied patches 1, 3, and 4. Patch 5 needs a description. Patches 2 and
> 6 go via your arch tree.
Hi Eduardo,
I've sent v5 for this series. It fixes a breakage for old DTs with new
code. Also, Rob decided he was happier with the earlier single
fallback binding. Please consider applying the new series.
Regards,
Amit
^ permalink raw reply [flat|nested] 28+ messages in thread