diff mbox series

[v3,6/6] dt-bindings: thermal: Add dt binding for QCOM LMh

Message ID 20210708120656.663851-7-thara.gopinath@linaro.org
State New
Headers show
Series Introduce LMh driver for Qualcomm SoCs | expand

Commit Message

Thara Gopinath July 8, 2021, 12:06 p.m. UTC
Add dt binding documentation to describe Qualcomm
Limits Management Hardware node.

Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 .../devicetree/bindings/thermal/qcom-lmh.yaml | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

Comments

Bjorn Andersson July 10, 2021, 4:21 a.m. UTC | #1
On Thu 08 Jul 07:06 CDT 2021, Thara Gopinath wrote:

> Add dt binding documentation to describe Qualcomm
> Limits Management Hardware node.
> 
> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>  .../devicetree/bindings/thermal/qcom-lmh.yaml | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
> new file mode 100644
> index 000000000000..7f62bd3d543d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2021 Linaro Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/qcom-lmh.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Limits Management Hardware(LMh)
> +
> +maintainers:
> +  - Thara Gopinath <thara.gopinath@linaro.org>
> +
> +description:
> +  Limits Management Hardware(LMh) is a hardware infrastructure on some
> +  Qualcomm SoCs that can enforce temperature and current limits as
> +  programmed by software for certain IPs like CPU.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sdm845-lmh
> +
> +  reg:
> +    items:
> +      - description: core registers
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  '#interrupt-cells':
> +    const: 1
> +
> +  interrupt-controller: true
> +
> +  qcom,lmh-cpu-id:
> +    description:
> +      CPU id of the first cpu in the LMh cluster
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  qcom,lmh-temperature-arm:
> +    description:
> +      An integer expressing temperature threshold in millicelsius at which
> +      the LMh thermal FSM is engaged.

Do we know (by any public source) what "arm", "low" and "high" means
beyond that they somehow pokes the state machine?

> +    $ref: /schemas/types.yaml#/definitions/int32
> +
> +  qcom,lmh-temperature-low:
> +    description:
> +      An integer expressing temperature threshold in millicelsius at which
> +      the LMh thermal FSM is engaged.
> +    $ref: /schemas/types.yaml#/definitions/int32
> +
> +  qcom,lmh-temperature-high:
> +    description:
> +      An integer expressing temperature threshold in millicelsius at which
> +      the LMh thermal FSM is engaged.
> +    $ref: /schemas/types.yaml#/definitions/int32
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - #interrupt-cells
> +  - interrupt-controller
> +  - qcom,lmh-cpu-id
> +  - qcom,lmh-temperature-arm
> +  - qcom,lmh-temperature-low
> +  - qcom,lmh-temperature-high
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/interconnect/qcom,sdm845.h>

I don't see why you need qcom,rpmh.h or the interconnect include in this
example.

> +
> +    lmh_cluster1: lmh@17d70800 {
> +      compatible = "qcom,sdm845-lmh";
> +      reg = <0 0x17d70800 0 0x401>;

#address- and #size-cells are 1 in the wrapper that validates the
examples, so drop the two zeros.

> +      interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +      qcom,lmh-cpu-id = <0x4>;
> +      qcom,lmh-temperature-arm = <65000>;
> +      qcom,lmh-temperature-low = <94500>;
> +      qcom,lmh-temperature-high = <95000>;
> +      interrupt-controller;
> +      #interrupt-cells = <1>;
> +    };
> +  - |

This is a different example from the one above, if you intended that,
don't you need the #include of arm-gic.h here as well?

Regards,
Bjorn

> +    lmh_cluster0: lmh@17d78800 {
> +      compatible = "qcom,sdm845-lmh";
> +      reg = <0 0x17d78800 0 0x401>;
> +      interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +      qcom,lmh-cpu-id = <0x0>;
> +      qcom,lmh-temperature-arm = <65000>;
> +      qcom,lmh-temperature-low = <94500>;
> +      qcom,lmh-temperature-high = <95000>;
> +      interrupt-controller;
> +      #interrupt-cells = <1>;
> +    };
> +  - |
> -- 
> 2.25.1
>
Rob Herring July 12, 2021, 5:32 p.m. UTC | #2
On Thu, Jul 08, 2021 at 08:06:56AM -0400, Thara Gopinath wrote:
> Add dt binding documentation to describe Qualcomm

> Limits Management Hardware node.

> 

> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

> ---

>  .../devicetree/bindings/thermal/qcom-lmh.yaml | 100 ++++++++++++++++++

>  1 file changed, 100 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

> 

> diff --git a/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

> new file mode 100644

> index 000000000000..7f62bd3d543d

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

> @@ -0,0 +1,100 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +# Copyright 2021 Linaro Ltd.

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/thermal/qcom-lmh.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Qualcomm Limits Management Hardware(LMh)

> +

> +maintainers:

> +  - Thara Gopinath <thara.gopinath@linaro.org>

> +

> +description:

> +  Limits Management Hardware(LMh) is a hardware infrastructure on some

> +  Qualcomm SoCs that can enforce temperature and current limits as

> +  programmed by software for certain IPs like CPU.

> +

> +properties:

> +  compatible:

> +    enum:

> +      - qcom,sdm845-lmh

> +

> +  reg:

> +    items:

> +      - description: core registers

> +

> +  interrupts:

> +    maxItems: 1

> +

> +  '#interrupt-cells':

> +    const: 1

> +

> +  interrupt-controller: true

> +

> +  qcom,lmh-cpu-id:

> +    description:

> +      CPU id of the first cpu in the LMh cluster

> +    $ref: /schemas/types.yaml#/definitions/uint32


The way we reference other nodes in DT is phandles. 'cpus' is already 
somewhat established for this case.

> +

> +  qcom,lmh-temperature-arm:

> +    description:

> +      An integer expressing temperature threshold in millicelsius at which


Use unit suffix when you have units.

> +      the LMh thermal FSM is engaged.

> +    $ref: /schemas/types.yaml#/definitions/int32

> +

> +  qcom,lmh-temperature-low:

> +    description:

> +      An integer expressing temperature threshold in millicelsius at which

> +      the LMh thermal FSM is engaged.

> +    $ref: /schemas/types.yaml#/definitions/int32

> +

> +  qcom,lmh-temperature-high:

> +    description:

> +      An integer expressing temperature threshold in millicelsius at which

> +      the LMh thermal FSM is engaged.


What's the difference in the 3 properties because the description is the 
same.

> +    $ref: /schemas/types.yaml#/definitions/int32

> +

> +required:

> +  - compatible

> +  - reg

> +  - interrupts

> +  - #interrupt-cells

> +  - interrupt-controller

> +  - qcom,lmh-cpu-id

> +  - qcom,lmh-temperature-arm

> +  - qcom,lmh-temperature-low

> +  - qcom,lmh-temperature-high

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

> +    #include <dt-bindings/clock/qcom,rpmh.h>

> +    #include <dt-bindings/interconnect/qcom,sdm845.h>

> +

> +    lmh_cluster1: lmh@17d70800 {

> +      compatible = "qcom,sdm845-lmh";

> +      reg = <0 0x17d70800 0 0x401>;

> +      interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;

> +      qcom,lmh-cpu-id = <0x4>;

> +      qcom,lmh-temperature-arm = <65000>;

> +      qcom,lmh-temperature-low = <94500>;

> +      qcom,lmh-temperature-high = <95000>;

> +      interrupt-controller;


What devices is this an interrupt controller for?

> +      #interrupt-cells = <1>;

> +    };

> +  - |

> +    lmh_cluster0: lmh@17d78800 {

> +      compatible = "qcom,sdm845-lmh";

> +      reg = <0 0x17d78800 0 0x401>;

> +      interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;

> +      qcom,lmh-cpu-id = <0x0>;

> +      qcom,lmh-temperature-arm = <65000>;

> +      qcom,lmh-temperature-low = <94500>;

> +      qcom,lmh-temperature-high = <95000>;

> +      interrupt-controller;

> +      #interrupt-cells = <1>;

> +    };

> +  - |

> -- 

> 2.25.1

> 

>
Thara Gopinath July 13, 2021, 12:54 a.m. UTC | #3
On 7/10/21 12:21 AM, Bjorn Andersson wrote:
> On Thu 08 Jul 07:06 CDT 2021, Thara Gopinath wrote:

> 

>> Add dt binding documentation to describe Qualcomm

>> Limits Management Hardware node.

>>

>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org>

>> ---

>>   .../devicetree/bindings/thermal/qcom-lmh.yaml | 100 ++++++++++++++++++

>>   1 file changed, 100 insertions(+)

>>   create mode 100644 Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

>>

>> diff --git a/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

>> new file mode 100644

>> index 000000000000..7f62bd3d543d

>> --- /dev/null

>> +++ b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml

>> @@ -0,0 +1,100 @@

>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

>> +# Copyright 2021 Linaro Ltd.

>> +%YAML 1.2

>> +---

>> +$id: http://devicetree.org/schemas/thermal/qcom-lmh.yaml#

>> +$schema: http://devicetree.org/meta-schemas/core.yaml#

>> +

>> +title: Qualcomm Limits Management Hardware(LMh)

>> +

>> +maintainers:

>> +  - Thara Gopinath <thara.gopinath@linaro.org>

>> +

>> +description:

>> +  Limits Management Hardware(LMh) is a hardware infrastructure on some

>> +  Qualcomm SoCs that can enforce temperature and current limits as

>> +  programmed by software for certain IPs like CPU.

>> +

>> +properties:

>> +  compatible:

>> +    enum:

>> +      - qcom,sdm845-lmh

>> +

>> +  reg:

>> +    items:

>> +      - description: core registers

>> +

>> +  interrupts:

>> +    maxItems: 1

>> +

>> +  '#interrupt-cells':

>> +    const: 1

>> +

>> +  interrupt-controller: true

>> +

>> +  qcom,lmh-cpu-id:

>> +    description:

>> +      CPU id of the first cpu in the LMh cluster

>> +    $ref: /schemas/types.yaml#/definitions/uint32

>> +

>> +  qcom,lmh-temperature-arm:

>> +    description:

>> +      An integer expressing temperature threshold in millicelsius at which

>> +      the LMh thermal FSM is engaged.

> 

> Do we know (by any public source) what "arm", "low" and "high" means

> beyond that they somehow pokes the state machine?


Not from public documentation. I know what these thresholds means, 
atleast to some extent. Though I will never claim to be an expert in 
this! There is an error in description of qcom,lmh-temperature-low and 
qcom,lmh-temperature-high below. I copied
and forgot to change the description. I will fix it.

> 

>> +    $ref: /schemas/types.yaml#/definitions/int32

>> +

>> +  qcom,lmh-temperature-low:

>> +    description:

>> +      An integer expressing temperature threshold in millicelsius at which

>> +      the LMh thermal FSM is engaged.

>> +    $ref: /schemas/types.yaml#/definitions/int32

>> +

>> +  qcom,lmh-temperature-high:

>> +    description:

>> +      An integer expressing temperature threshold in millicelsius at which

>> +      the LMh thermal FSM is engaged.

>> +    $ref: /schemas/types.yaml#/definitions/int32

>> +

>> +required:

>> +  - compatible

>> +  - reg

>> +  - interrupts

>> +  - #interrupt-cells

>> +  - interrupt-controller

>> +  - qcom,lmh-cpu-id

>> +  - qcom,lmh-temperature-arm

>> +  - qcom,lmh-temperature-low

>> +  - qcom,lmh-temperature-high

>> +

>> +additionalProperties: false

>> +

>> +examples:

>> +  - |

>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

>> +    #include <dt-bindings/clock/qcom,rpmh.h>

>> +    #include <dt-bindings/interconnect/qcom,sdm845.h>

> 

> I don't see why you need qcom,rpmh.h or the interconnect include in this

> example.


I could have sworn make dt-bindings check failed. But maybe only The 
first include is needed. I will remove the other two.

> 

>> +

>> +    lmh_cluster1: lmh@17d70800 {

>> +      compatible = "qcom,sdm845-lmh";

>> +      reg = <0 0x17d70800 0 0x401>;

> 

> #address- and #size-cells are 1 in the wrapper that validates the

> examples, so drop the two zeros.


Ok.

> 

>> +      interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;

>> +      qcom,lmh-cpu-id = <0x4>;

>> +      qcom,lmh-temperature-arm = <65000>;

>> +      qcom,lmh-temperature-low = <94500>;

>> +      qcom,lmh-temperature-high = <95000>;

>> +      interrupt-controller;

>> +      #interrupt-cells = <1>;

>> +    };

>> +  - |

> 

> This is a different example from the one above, if you intended that,

> don't you need the #include of arm-gic.h here as well?


Again make dt-bindings check did not fail. It is a different example.
So I am not sure of the norm here. Is one example good enough ?

> 

> Regards,

> Bjorn

> 

>> +    lmh_cluster0: lmh@17d78800 {

>> +      compatible = "qcom,sdm845-lmh";

>> +      reg = <0 0x17d78800 0 0x401>;

>> +      interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;

>> +      qcom,lmh-cpu-id = <0x0>;

>> +      qcom,lmh-temperature-arm = <65000>;

>> +      qcom,lmh-temperature-low = <94500>;

>> +      qcom,lmh-temperature-high = <95000>;

>> +      interrupt-controller;

>> +      #interrupt-cells = <1>;

>> +    };

>> +  - |

>> -- 

>> 2.25.1

>>


-- 
Warm Regards
Thara (She/Her/Hers)
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
new file mode 100644
index 000000000000..7f62bd3d543d
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/qcom-lmh.yaml
@@ -0,0 +1,100 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2021 Linaro Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/qcom-lmh.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Limits Management Hardware(LMh)
+
+maintainers:
+  - Thara Gopinath <thara.gopinath@linaro.org>
+
+description:
+  Limits Management Hardware(LMh) is a hardware infrastructure on some
+  Qualcomm SoCs that can enforce temperature and current limits as
+  programmed by software for certain IPs like CPU.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sdm845-lmh
+
+  reg:
+    items:
+      - description: core registers
+
+  interrupts:
+    maxItems: 1
+
+  '#interrupt-cells':
+    const: 1
+
+  interrupt-controller: true
+
+  qcom,lmh-cpu-id:
+    description:
+      CPU id of the first cpu in the LMh cluster
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,lmh-temperature-arm:
+    description:
+      An integer expressing temperature threshold in millicelsius at which
+      the LMh thermal FSM is engaged.
+    $ref: /schemas/types.yaml#/definitions/int32
+
+  qcom,lmh-temperature-low:
+    description:
+      An integer expressing temperature threshold in millicelsius at which
+      the LMh thermal FSM is engaged.
+    $ref: /schemas/types.yaml#/definitions/int32
+
+  qcom,lmh-temperature-high:
+    description:
+      An integer expressing temperature threshold in millicelsius at which
+      the LMh thermal FSM is engaged.
+    $ref: /schemas/types.yaml#/definitions/int32
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - #interrupt-cells
+  - interrupt-controller
+  - qcom,lmh-cpu-id
+  - qcom,lmh-temperature-arm
+  - qcom,lmh-temperature-low
+  - qcom,lmh-temperature-high
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/interconnect/qcom,sdm845.h>
+
+    lmh_cluster1: lmh@17d70800 {
+      compatible = "qcom,sdm845-lmh";
+      reg = <0 0x17d70800 0 0x401>;
+      interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+      qcom,lmh-cpu-id = <0x4>;
+      qcom,lmh-temperature-arm = <65000>;
+      qcom,lmh-temperature-low = <94500>;
+      qcom,lmh-temperature-high = <95000>;
+      interrupt-controller;
+      #interrupt-cells = <1>;
+    };
+  - |
+    lmh_cluster0: lmh@17d78800 {
+      compatible = "qcom,sdm845-lmh";
+      reg = <0 0x17d78800 0 0x401>;
+      interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+      qcom,lmh-cpu-id = <0x0>;
+      qcom,lmh-temperature-arm = <65000>;
+      qcom,lmh-temperature-low = <94500>;
+      qcom,lmh-temperature-high = <95000>;
+      interrupt-controller;
+      #interrupt-cells = <1>;
+    };
+  - |