diff mbox series

[v2,07/15] dt: thermal: tsens: Document interrupt support in tsens driver

Message ID 66ac3d3707d6296ef85bf1fa321f7f1ee0c02131.1566907161.git.amit.kucheria@linaro.org
State New
Headers show
Series None | expand

Commit Message

Amit Kucheria Aug. 27, 2019, 12:14 p.m. UTC
Define two new required properties to define interrupts and
interrupt-names for tsens.

Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

---
 Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
2.17.1

Comments

Stephen Boyd Aug. 28, 2019, 12:33 a.m. UTC | #1
Quoting Amit Kucheria (2019-08-27 05:14:03)
> Define two new required properties to define interrupts and

> interrupt-names for tsens.

> 

> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---

>  Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++

>  1 file changed, 8 insertions(+)

> 

> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> index 673cc1831ee9d..686bede72f846 100644

> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> @@ -22,6 +22,8 @@ Required properties:

>  

>  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.

>  - #qcom,sensors: Number of sensors in tsens block

> +- interrupts: Interrupts generated from Always-On subsystem (AOSS)


Is it always one? interrupt-names makes it sound like it.

> +- interrupt-names: Must be one of the following: "uplow", "critical"

>  - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify

>  nvmem cells

>
Amit Kucheria Aug. 29, 2019, 4:34 p.m. UTC | #2
On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <swboyd@chromium.org> wrote:
>

> Quoting Amit Kucheria (2019-08-29 01:48:27)

> > On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <swboyd@chromium.org> wrote:

> > >

> > > Quoting Amit Kucheria (2019-08-27 05:14:03)

> > > > Define two new required properties to define interrupts and

> > > > interrupt-names for tsens.

> > > >

> > > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

> > > > ---

> > > >  Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++

> > > >  1 file changed, 8 insertions(+)

> > > >

> > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > index 673cc1831ee9d..686bede72f846 100644

> > > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > @@ -22,6 +22,8 @@ Required properties:

> > > >

> > > >  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.

> > > >  - #qcom,sensors: Number of sensors in tsens block

> > > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)

> > >

> > > Is it always one? interrupt-names makes it sound like it.

> > >

> > > > +- interrupt-names: Must be one of the following: "uplow", "critical"

> >

> > Will fix to "one or more of the following"

> >

>

> Can we get a known quantity of interrupts for a particular compatible

> string instead? Let's be as specific as possible. The index matters too,

> so please list them in the order that is desired.


I *think* we can predict what platforms have uplow and critical
interrupts based on IP version currently[1]. For newer interrupt
types, we might need more fine-grained platform compatibles.

[1] Caveat: this is based only on the list of platforms I've currently
looked at, there might be something internally that breaks these
rules.
Amit Kucheria Aug. 30, 2019, 11:32 a.m. UTC | #3
On Thu, Aug 29, 2019 at 10:04 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>

> On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <swboyd@chromium.org> wrote:

> >

> > Quoting Amit Kucheria (2019-08-29 01:48:27)

> > > On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <swboyd@chromium.org> wrote:

> > > >

> > > > Quoting Amit Kucheria (2019-08-27 05:14:03)

> > > > > Define two new required properties to define interrupts and

> > > > > interrupt-names for tsens.

> > > > >

> > > > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

> > > > > ---

> > > > >  Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++

> > > > >  1 file changed, 8 insertions(+)

> > > > >

> > > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > > index 673cc1831ee9d..686bede72f846 100644

> > > > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt

> > > > > @@ -22,6 +22,8 @@ Required properties:

> > > > >

> > > > >  - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.

> > > > >  - #qcom,sensors: Number of sensors in tsens block

> > > > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)

> > > >

> > > > Is it always one? interrupt-names makes it sound like it.

> > > >

> > > > > +- interrupt-names: Must be one of the following: "uplow", "critical"

> > >

> > > Will fix to "one or more of the following"

> > >

> >

> > Can we get a known quantity of interrupts for a particular compatible

> > string instead? Let's be as specific as possible. The index matters too,

> > so please list them in the order that is desired.

>

> I *think* we can predict what platforms have uplow and critical

> interrupts based on IP version currently[1]. For newer interrupt

> types, we might need more fine-grained platform compatibles.

>

> [1] Caveat: this is based only on the list of platforms I've currently

> looked at, there might be something internally that breaks these

> rules.


What do you think if we changed the wording to something like the following,

- interrupt-names: Must be one of the following depending on IP version:
   For compatibles qcom,msm8916-tsens, qcom,msm8974-tsens,
qcom,qcs404-tsens, qcom,tsens-v1, use
              interrupt-names = "uplow";
   For compatibles qcom,msm8996-tsens, qcom,msm8998-tsens,
qcom,sdm845-tsens, qcom,tsens-v2, use
              interrupt-names = "uplow", "critical";
Stephen Boyd Aug. 30, 2019, 3:55 p.m. UTC | #4
Quoting Amit Kucheria (2019-08-30 04:32:54)
> On Thu, Aug 29, 2019 at 10:04 PM Amit Kucheria <amit.kucheria@linaro.org> wrote:

> >

> > On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <swboyd@chromium.org> wrote:

> > >

> > > Can we get a known quantity of interrupts for a particular compatible

> > > string instead? Let's be as specific as possible. The index matters too,

> > > so please list them in the order that is desired.

> >

> > I *think* we can predict what platforms have uplow and critical

> > interrupts based on IP version currently[1]. For newer interrupt

> > types, we might need more fine-grained platform compatibles.

> >

> > [1] Caveat: this is based only on the list of platforms I've currently

> > looked at, there might be something internally that breaks these

> > rules.

> 

> What do you think if we changed the wording to something like the following,

> 

> - interrupt-names: Must be one of the following depending on IP version:

>    For compatibles qcom,msm8916-tsens, qcom,msm8974-tsens,

> qcom,qcs404-tsens, qcom,tsens-v1, use

>               interrupt-names = "uplow";

>    For compatibles qcom,msm8996-tsens, qcom,msm8998-tsens,

> qcom,sdm845-tsens, qcom,tsens-v2, use

>               interrupt-names = "uplow", "critical";


Ok. I would still prefer YAML/JSON schema for this binding so that it's
much more explicit about numbers and the order of interrupts, etc.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 673cc1831ee9d..686bede72f846 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -22,6 +22,8 @@  Required properties:
 
 - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
 - #qcom,sensors: Number of sensors in tsens block
+- interrupts: Interrupts generated from Always-On subsystem (AOSS)
+- interrupt-names: Must be one of the following: "uplow", "critical"
 - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
 nvmem cells
 
@@ -40,6 +42,9 @@  tsens0: thermal-sensor@c263000 {
 		reg = <0xc263000 0x1ff>, /* TM */
 			<0xc222000 0x1ff>; /* SROT */
 		#qcom,sensors = <13>;
+		interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "uplow";
+
 		#thermal-sensor-cells = <1>;
 	};
 
@@ -51,5 +56,8 @@  tsens: thermal-sensor@4a9000 {
 		nvmem-cells = <&tsens_caldata>;
 		nvmem-cell-names = "calib";
 		#qcom,sensors = <10>;
+		interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "uplow";
+
 		#thermal-sensor-cells = <1>;
 	};