| Message ID | 66ac3d3707d6296ef85bf1fa321f7f1ee0c02131.1566907161.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | thermal: qcom: tsens: Add interrupt support | expand |
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 >
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" > > - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify > > nvmem cells > >
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.
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.
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";
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.
On Fri, Aug 30, 2019 at 9:25 PM Stephen Boyd <swboyd@chromium.org> wrote: > > 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. OK, I'll look around for some examples.
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>; };
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(+)