diff mbox series

[v4,4/6] thermal: tsens: Add support for SDM845

Message ID 43e3ae530334efab3fa0151430879f03731e9daa.1530533998.git.amit.kucheria@linaro.org
State New
Headers show
Series None | expand

Commit Message

Amit Kucheria July 2, 2018, 12:44 p.m. UTC
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(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Amit Kucheria July 4, 2018, 5:26 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) July 5, 2018, 8:37 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

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__ */