2022-09-12 09:05:14

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 0/4] thermal: Introduce Qualcomm Cooling Driver suppport

This patchset introduces the Qualcomm Cooling Driver (aka Qualcomm Thermal
Mitigation Driver) and the related support code (dt-binding, MAINTAINER
file etc).

Several Qualcomm Snapdragon SoCs have Thermal Mitigation Devices (TMDs)
present on various remote subsystem(s) (for e.g. the Compute DSP, aka cDSP),
which can be used for several mitigations for remote subsystem(s), including
remote processor mitigation, rail voltage restriction etc.

Here we introduce the Qualcomm Cooling Driver which is based on the
kernel thermal driver framework and also employs the kernel QMI interface
to send the message to remote subsystem(s).

At the very top-level, the dts is supposed to describe a TMD node, which
should further represent the remote subsystem(s) present on the SoC and
further each child of a subsystem should represent the separate cooling devices
available on the remote subsystem.

Note that this patchset is targeted for the 'linux-pm' tree and the dts
patchset/changes targeted for 'linux-arm-msm' tree will be sent as a
separate patchset.

This patchset is based on the CONFIG_QCOM_THERMAL related fix sent via
[1]. Otherwise the latest changes from 'linux-next/master' are used to
rebase the patchset.

[1]. https://lore.kernel.org/all/CAA8EJpoM5nW=pVJB4zy4Jh9Q3gE4KOju2QVy_WtmUokKMyXtuw@mail.gmail.com/T/#m4e2b765e68e3123b3c0e28c806409dae4b988432

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

Bhupesh Sharma (4):
thermal: qcom: qmi_cooling: Add skeletal qmi cooling driver
thermal: qcom: Add Kconfig entry & compilation support for qmi cooling
driver
dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml
bindings
MAINTAINERS: Add entry for Qualcomm Cooling Driver

.../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++
.../bindings/thermal/qcom,tmd-device.yaml | 122 ++++
MAINTAINERS | 10 +
drivers/thermal/qcom/Kconfig | 4 +
drivers/thermal/qcom/Makefile | 2 +
drivers/thermal/qcom/qmi_cooling/Kconfig | 14 +
drivers/thermal/qcom/qmi_cooling/Makefile | 3 +
.../qcom/qmi_cooling/qcom_qmi_cooling.c | 632 ++++++++++++++++++
.../qcom/qmi_cooling/qcom_tmd_services.c | 352 ++++++++++
.../qcom/qmi_cooling/qcom_tmd_services.h | 120 ++++
include/dt-bindings/thermal/qcom,tmd.h | 14 +
11 files changed, 1351 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
create mode 100644 drivers/thermal/qcom/qmi_cooling/Kconfig
create mode 100644 drivers/thermal/qcom/qmi_cooling/Makefile
create mode 100644 drivers/thermal/qcom/qmi_cooling/qcom_qmi_cooling.c
create mode 100644 drivers/thermal/qcom/qmi_cooling/qcom_tmd_services.c
create mode 100644 drivers/thermal/qcom/qmi_cooling/qcom_tmd_services.h
create mode 100644 include/dt-bindings/thermal/qcom,tmd.h

--
2.37.1


2022-09-12 09:09:09

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings

Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.

Qualcomm QMI based TMD cooling device(s) are used for various
mitigations for remote subsystem(s) including remote processor
mitigation, rail voltage restriction etc.

Each child node represents one remote subsystem and each child
of this subsystem in-turn represents separate TMD cooling device.

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Bhupesh Sharma <[email protected]>
---
.../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++++++++++
.../bindings/thermal/qcom,tmd-device.yaml | 122 ++++++++++++++++++
include/dt-bindings/thermal/qcom,tmd.h | 14 ++
3 files changed, 214 insertions(+)
create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
create mode 100644 include/dt-bindings/thermal/qcom,tmd.h

diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
new file mode 100644
index 000000000000..dfda5b611a93
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/thermal/qcom,qmi-tmd-device.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
+
+maintainers:
+ - Bhupesh Sharma <[email protected]>
+
+description:
+ Qualcomm QMI based TMD cooling device(s) are used for various
+ mitigations for remote subsystem(s) including remote processor
+ mitigation, rail voltage restriction etc.
+
+properties:
+ $nodename:
+ const: qmi-tmd-devices
+
+ compatible:
+ items:
+ - const: qcom,qmi-tmd-devices
+
+ modem0:
+ $ref: /schemas/thermal/qcom,tmd-device.yaml#
+
+ adsp:
+ $ref: /schemas/thermal/qcom,tmd-device.yaml#
+
+ cdsp:
+ $ref: /schemas/thermal/qcom,tmd-device.yaml#
+
+ slpi:
+ $ref: /schemas/thermal/qcom,tmd-device.yaml#
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/thermal/qcom,tmd.h>
+ qmi-tmd-devices {
+ compatible = "qcom,qmi-tmd-devices";
+
+ modem0 {
+ qcom,instance-id = <MODEM0_INSTANCE_ID>;
+
+ modem0_pa: tmd-device0 {
+ label = "pa";
+ #cooling-cells = <2>;
+ };
+
+ modem0_proc: tmd-device1 {
+ label = "modem";
+ #cooling-cells = <2>;
+ };
+
+ modem0_current: tmd-device2 {
+ label = "modem_current";
+ #cooling-cells = <2>;
+ };
+
+ modem0_skin: tmd-device3 {
+ label = "modem_skin";
+ #cooling-cells = <2>;
+ };
+
+ modem0_vdd: tmd-device4 {
+ label = "cpuv_restriction_cold";
+ #cooling-cells = <2>;
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
new file mode 100644
index 000000000000..38ac62f03376
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/thermal/qcom,tmd-device.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm thermal mitigation (TMD) cooling devices
+
+maintainers:
+ - Bhupesh Sharma <[email protected]>
+
+description:
+ Qualcomm thermal mitigation (TMD) cooling devices. Each child node
+ represents one remote subsystem and each child of this subsystem in-turn
+ represents separate cooling devices.
+
+properties:
+ $nodename:
+ pattern: "^(modem|adsp|cdsp|slpi[0-9])?$"
+
+ qcom,instance-id:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Remote subsystem QMI server instance id to be used for communicating with QMI.
+
+patternProperties:
+ "^tmd-device[0-9]?$":
+ type: object
+ description:
+ Subnodes indicating tmd cooling device of a specific category.
+ properties:
+ label:
+ maxItems: 1
+ description: |
+ Remote subsystem device identifier. Acceptable device names -
+ "pa" -> for pa cooling device,
+ "cpuv_restriction_cold" -> for vdd restriction,
+ "cx_vdd_limit" -> for vdd limit,
+ "modem" -> for processor passive cooling device,
+ "modem_current" -> for current limiting device,
+ "modem_bw" -> for bus bandwidth limiting device,
+ "cpr_cold" -> for cpr restriction.
+
+ "#cooling-cells":
+ const: 2
+
+ required:
+ - label
+ - "#cooling-cells"
+
+ additionalProperties: false
+
+required:
+ - qcom,instance-id
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/thermal/qcom,tmd.h>
+ modem0 {
+ qcom,instance-id = <MODEM0_INSTANCE_ID>;
+
+ modem0_pa: tmd-device0 {
+ label = "pa";
+ #cooling-cells = <2>;
+ };
+
+ modem0_proc: tmd-device1 {
+ label = "modem";
+ #cooling-cells = <2>;
+ };
+
+ modem0_current: tmd-device2 {
+ label = "modem_current";
+ #cooling-cells = <2>;
+ };
+
+ modem0_skin: tmd-device3 {
+ label = "modem_skin";
+ #cooling-cells = <2>;
+ };
+
+ modem0_vdd: tmd-device4 {
+ label = "cpuv_restriction_cold";
+ #cooling-cells = <2>;
+ };
+ };
+
+ - |
+ #include <dt-bindings/thermal/qcom,tmd.h>
+ adsp {
+ qcom,instance-id = <ADSP_INSTANCE_ID>;
+
+ adsp_vdd: tmd-device1 {
+ label = "cpuv_restriction_cold";
+ #cooling-cells = <2>;
+ };
+ };
+
+ - |
+ #include <dt-bindings/thermal/qcom,tmd.h>
+ cdsp {
+ qcom,instance-id = <CDSP_INSTANCE_ID>;
+
+ cdsp_vdd: tmd-device1 {
+ label = "cpuv_restriction_cold";
+ #cooling-cells = <2>;
+ };
+ };
+
+ - |
+ #include <dt-bindings/thermal/qcom,tmd.h>
+ slpi {
+ qcom,instance-id = <SLPI_INSTANCE_ID>;
+
+ slpi_vdd: tmd-device1 {
+ label = "cpuv_restriction_cold";
+ #cooling-cells = <2>;
+ };
+ };
diff --git a/include/dt-bindings/thermal/qcom,tmd.h b/include/dt-bindings/thermal/qcom,tmd.h
new file mode 100644
index 000000000000..5ede4422e04e
--- /dev/null
+++ b/include/dt-bindings/thermal/qcom,tmd.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for the Qualcomm TMD instances.
+ */
+
+#ifndef _DT_BINDINGS_THERMAL_QCOM_TMD_H_
+#define _DT_BINDINGS_THERMAL_QCOM_TMD_H_
+
+#define MODEM0_INSTANCE_ID 0x0
+#define ADSP_INSTANCE_ID 0x1
+#define CDSP_INSTANCE_ID 0x43
+#define SLPI_INSTANCE_ID 0x53
+
+#endif
--
2.37.1

2022-09-12 09:11:59

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 4/4] MAINTAINERS: Add entry for Qualcomm Cooling Driver

Add myself as the maintainer for the Qualcomm Cooling
driver (aka Qualcomm Thermal Mitigation Device Driver).

Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Bhupesh Sharma <[email protected]>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4dbf82d3b775..20f90089bf10 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17048,6 +17048,16 @@ S: Maintained
F: Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
F: drivers/mtd/nand/raw/qcom_nandc.c

+QUALCOMM QMI COOLING DRIVER (THERMAL MITIGATION DEVICE DRIVER)
+M: Bhupesh Sharma <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
+F: Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
+F: drivers/thermal/qcom/qmi_cooling/
+F: include/dt-bindings/thermal/qcom,tmd.h
+
QUALCOMM RMNET DRIVER
M: Subash Abhinov Kasiviswanathan <[email protected]>
M: Sean Tranchetti <[email protected]>
--
2.37.1

2022-09-12 15:42:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings

On Mon, 12 Sep 2022 14:20:48 +0530, Bhupesh Sharma wrote:
> Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.
>
> Qualcomm QMI based TMD cooling device(s) are used for various
> mitigations for remote subsystem(s) including remote processor
> mitigation, rail voltage restriction etc.
>
> Each child node represents one remote subsystem and each child
> of this subsystem in-turn represents separate TMD cooling device.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++++++++++
> .../bindings/thermal/qcom,tmd-device.yaml | 122 ++++++++++++++++++
> include/dt-bindings/thermal/qcom,tmd.h | 14 ++
> 3 files changed, 214 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> create mode 100644 include/dt-bindings/thermal/qcom,tmd.h
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/remoteproc/qcom,adsp.example.dtb: adsp: 'qcom,instance-id' is a required property
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/remoteproc/qcom,adsp.example.dtb: adsp: 'clock-names', 'clocks', 'compatible', 'cx-supply', 'interrupt-names', 'interrupts-extended', 'memory-region', 'qcom,smem-state-names', 'qcom,smem-states', 'smd-edge' do not match any of the regexes: '^tmd-device[0-9]?$', 'pinctrl-[0-9]+'
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

2022-09-12 16:17:36

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings

On Mon, Sep 12, 2022 at 02:20:48PM +0530, Bhupesh Sharma wrote:
> Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.
>
> Qualcomm QMI based TMD cooling device(s) are used for various
> mitigations for remote subsystem(s) including remote processor
> mitigation, rail voltage restriction etc.
>
> Each child node represents one remote subsystem and each child
> of this subsystem in-turn represents separate TMD cooling device.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++++++++++
> .../bindings/thermal/qcom,tmd-device.yaml | 122 ++++++++++++++++++
> include/dt-bindings/thermal/qcom,tmd.h | 14 ++
> 3 files changed, 214 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> create mode 100644 include/dt-bindings/thermal/qcom,tmd.h
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> new file mode 100644
> index 000000000000..dfda5b611a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,qmi-tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm QMI based TMD cooling device(s) are used for various
> + mitigations for remote subsystem(s) including remote processor
> + mitigation, rail voltage restriction etc.
> +
> +properties:
> + $nodename:
> + const: qmi-tmd-devices
> +
> + compatible:
> + items:
> + - const: qcom,qmi-tmd-devices
> +
> + modem0:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + adsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + cdsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + slpi:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + qmi-tmd-devices {
> + compatible = "qcom,qmi-tmd-devices";
> +
> + modem0 {
> + qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> + modem0_pa: tmd-device0 {
> + label = "pa";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_proc: tmd-device1 {
> + label = "modem";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_current: tmd-device2 {
> + label = "modem_current";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_skin: tmd-device3 {
> + label = "modem_skin";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_vdd: tmd-device4 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> new file mode 100644
> index 000000000000..38ac62f03376
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm thermal mitigation (TMD) cooling devices
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm thermal mitigation (TMD) cooling devices. Each child node
> + represents one remote subsystem and each child of this subsystem in-turn
> + represents separate cooling devices.
> +
> +properties:
> + $nodename:
> + pattern: "^(modem|adsp|cdsp|slpi[0-9])?$"

Node names are supposed to reflect the class of device, not instance.

> +
> + qcom,instance-id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Remote subsystem QMI server instance id to be used for communicating with QMI.

In general, we don't do instance indexes in DT. You'll need to explain
where the value comes from.

> +
> +patternProperties:
> + "^tmd-device[0-9]?$":

A cooling provider should have a standard node name. Not sure offhand if
we defined one in the spec or not yet.

> + type: object
> + description:
> + Subnodes indicating tmd cooling device of a specific category.
> + properties:
> + label:
> + maxItems: 1
> + description: |
> + Remote subsystem device identifier. Acceptable device names -
> + "pa" -> for pa cooling device,
> + "cpuv_restriction_cold" -> for vdd restriction,
> + "cx_vdd_limit" -> for vdd limit,
> + "modem" -> for processor passive cooling device,
> + "modem_current" -> for current limiting device,
> + "modem_bw" -> for bus bandwidth limiting device,
> + "cpr_cold" -> for cpr restriction.
> +
> + "#cooling-cells":
> + const: 2
> +
> + required:
> + - label
> + - "#cooling-cells"
> +
> + additionalProperties: false
> +
> +required:
> + - qcom,instance-id
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + modem0 {
> + qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> + modem0_pa: tmd-device0 {
> + label = "pa";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_proc: tmd-device1 {
> + label = "modem";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_current: tmd-device2 {
> + label = "modem_current";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_skin: tmd-device3 {
> + label = "modem_skin";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_vdd: tmd-device4 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + adsp {
> + qcom,instance-id = <ADSP_INSTANCE_ID>;
> +
> + adsp_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + cdsp {
> + qcom,instance-id = <CDSP_INSTANCE_ID>;
> +
> + cdsp_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + slpi {
> + qcom,instance-id = <SLPI_INSTANCE_ID>;
> +
> + slpi_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> diff --git a/include/dt-bindings/thermal/qcom,tmd.h b/include/dt-bindings/thermal/qcom,tmd.h
> new file mode 100644
> index 000000000000..5ede4422e04e
> --- /dev/null
> +++ b/include/dt-bindings/thermal/qcom,tmd.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license.

> +/*
> + * This header provides constants for the Qualcomm TMD instances.
> + */
> +
> +#ifndef _DT_BINDINGS_THERMAL_QCOM_TMD_H_
> +#define _DT_BINDINGS_THERMAL_QCOM_TMD_H_
> +
> +#define MODEM0_INSTANCE_ID 0x0
> +#define ADSP_INSTANCE_ID 0x1
> +#define CDSP_INSTANCE_ID 0x43
> +#define SLPI_INSTANCE_ID 0x53
> +
> +#endif
> --
> 2.37.1
>
>

2022-09-13 19:18:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings

On Mon, Sep 12, 2022 at 02:20:48PM +0530, Bhupesh Sharma wrote:
> Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.

Looks like a duplicate of $subject.

>
> Qualcomm QMI based TMD cooling device(s) are used for various

What is "TMD" an abbreviation of?

> mitigations for remote subsystem(s) including remote processor
> mitigation, rail voltage restriction etc.
>
> Each child node represents one remote subsystem and each child
> of this subsystem in-turn represents separate TMD cooling device.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++++++++++
> .../bindings/thermal/qcom,tmd-device.yaml | 122 ++++++++++++++++++
> include/dt-bindings/thermal/qcom,tmd.h | 14 ++
> 3 files changed, 214 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> create mode 100644 include/dt-bindings/thermal/qcom,tmd.h
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> new file mode 100644
> index 000000000000..dfda5b611a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,qmi-tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm QMI based TMD cooling device(s) are used for various
> + mitigations for remote subsystem(s) including remote processor
> + mitigation, rail voltage restriction etc.
> +
> +properties:
> + $nodename:
> + const: qmi-tmd-devices
> +
> + compatible:
> + items:
> + - const: qcom,qmi-tmd-devices
> +
> + modem0:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + adsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + cdsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + slpi:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + qmi-tmd-devices {

Looking at the implementation I see no relationship between the
individual instances (i.e. between the children of this node).

My suggestion is that you drop this top-level node and just list out
modem, adsp etc individually - which would mean that you can remove one
layer of indirection in the driver, as each instance would just need a
list of cooling-devices.

> + compatible = "qcom,qmi-tmd-devices";
> +
> + modem0 {

So you would move the compatible here.

> + qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> + modem0_pa: tmd-device0 {
> + label = "pa";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_proc: tmd-device1 {
> + label = "modem";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_current: tmd-device2 {
> + label = "modem_current";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_skin: tmd-device3 {
> + label = "modem_skin";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_vdd: tmd-device4 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> new file mode 100644
> index 000000000000..38ac62f03376
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +

I see no reason for splitting this into a separate binding.

> +title: Qualcomm thermal mitigation (TMD) cooling devices
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm thermal mitigation (TMD) cooling devices. Each child node
> + represents one remote subsystem and each child of this subsystem in-turn
> + represents separate cooling devices.
> +
> +properties:
> + $nodename:
> + pattern: "^(modem|adsp|cdsp|slpi[0-9])?$"
> +
> + qcom,instance-id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Remote subsystem QMI server instance id to be used for communicating with QMI.
> +
> +patternProperties:
> + "^tmd-device[0-9]?$":

So max 10 cooling devices per remote?

> + type: object
> + description:
> + Subnodes indicating tmd cooling device of a specific category.
> + properties:
> + label:
> + maxItems: 1
> + description: |
> + Remote subsystem device identifier. Acceptable device names -
> + "pa" -> for pa cooling device,
> + "cpuv_restriction_cold" -> for vdd restriction,
> + "cx_vdd_limit" -> for vdd limit,
> + "modem" -> for processor passive cooling device,
> + "modem_current" -> for current limiting device,
> + "modem_bw" -> for bus bandwidth limiting device,
> + "cpr_cold" -> for cpr restriction.

Afaict there are about 50 valid cooling devices listed in the driver.
Why limit this to these 7 here?

> +
> + "#cooling-cells":
> + const: 2
> +
> + required:
> + - label
> + - "#cooling-cells"
> +
> + additionalProperties: false
> +
> +required:
> + - qcom,instance-id
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + modem0 {

As written here this example is incomplete, as these nodes can't live on
their own.

But this is actually what I propose above.

> + qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> + modem0_pa: tmd-device0 {
> + label = "pa";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_proc: tmd-device1 {
> + label = "modem";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_current: tmd-device2 {
> + label = "modem_current";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_skin: tmd-device3 {
> + label = "modem_skin";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_vdd: tmd-device4 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + adsp {
> + qcom,instance-id = <ADSP_INSTANCE_ID>;
> +
> + adsp_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + cdsp {
> + qcom,instance-id = <CDSP_INSTANCE_ID>;
> +
> + cdsp_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> +
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + slpi {
> + qcom,instance-id = <SLPI_INSTANCE_ID>;
> +
> + slpi_vdd: tmd-device1 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> diff --git a/include/dt-bindings/thermal/qcom,tmd.h b/include/dt-bindings/thermal/qcom,tmd.h
> new file mode 100644
> index 000000000000..5ede4422e04e
> --- /dev/null
> +++ b/include/dt-bindings/thermal/qcom,tmd.h

This is a quite generic name, how about qcom,qmi-cooling.h?

> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides constants for the Qualcomm TMD instances.
> + */
> +
> +#ifndef _DT_BINDINGS_THERMAL_QCOM_TMD_H_
> +#define _DT_BINDINGS_THERMAL_QCOM_TMD_H_
> +
> +#define MODEM0_INSTANCE_ID 0x0
> +#define ADSP_INSTANCE_ID 0x1
> +#define CDSP_INSTANCE_ID 0x43
> +#define SLPI_INSTANCE_ID 0x53

QMI cooling isn't the only thing dealing with "instance id" and all of
them would deal with instances ids of type modem, adsp, cdsp, slpi etc.

As such I think these are too generic, how about

QMI_COOLING_ADSP etc?

Regards,
Bjorn

> +
> +#endif
> --
> 2.37.1
>

2022-10-28 17:28:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/4] dt-bindings: thermal: Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings

On 12/09/2022 04:50, Bhupesh Sharma wrote:
> Add qcom,qmi-tmd-device and qcom,tmd-device yaml bindings.
>
> Qualcomm QMI based TMD cooling device(s) are used for various
> mitigations for remote subsystem(s) including remote processor
> mitigation, rail voltage restriction etc.
>
> Each child node represents one remote subsystem and each child
> of this subsystem in-turn represents separate TMD cooling device.
>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> .../bindings/thermal/qcom,qmi-tmd-device.yaml | 78 +++++++++++
> .../bindings/thermal/qcom,tmd-device.yaml | 122 ++++++++++++++++++
> include/dt-bindings/thermal/qcom,tmd.h | 14 ++

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

> 3 files changed, 214 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> create mode 100644 Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> create mode 100644 include/dt-bindings/thermal/qcom,tmd.h
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> new file mode 100644
> index 000000000000..dfda5b611a93
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-tmd-device.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,qmi-tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes.

> +
> +title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm QMI based TMD cooling device(s) are used for various
> + mitigations for remote subsystem(s) including remote processor
> + mitigation, rail voltage restriction etc.
> +
> +properties:
> + $nodename:
> + const: qmi-tmd-devices

Why enforcing node name?
> +
> + compatible:
> + items:
> + - const: qcom,qmi-tmd-devices
> +
> + modem0:

Why 0? Do you think it will grow for more devices?

> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + adsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + cdsp:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> + slpi:
> + $ref: /schemas/thermal/qcom,tmd-device.yaml#
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/thermal/qcom,tmd.h>
> + qmi-tmd-devices {
> + compatible = "qcom,qmi-tmd-devices";
> +
> + modem0 {
> + qcom,instance-id = <MODEM0_INSTANCE_ID>;
> +
> + modem0_pa: tmd-device0 {
> + label = "pa";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_proc: tmd-device1 {
> + label = "modem";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_current: tmd-device2 {
> + label = "modem_current";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_skin: tmd-device3 {
> + label = "modem_skin";
> + #cooling-cells = <2>;
> + };
> +
> + modem0_vdd: tmd-device4 {
> + label = "cpuv_restriction_cold";
> + #cooling-cells = <2>;
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> new file mode 100644
> index 000000000000..38ac62f03376
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/qcom,tmd-device.yaml
> @@ -0,0 +1,122 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/thermal/qcom,tmd-device.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm thermal mitigation (TMD) cooling devices
> +
> +maintainers:
> + - Bhupesh Sharma <[email protected]>
> +
> +description:
> + Qualcomm thermal mitigation (TMD) cooling devices. Each child node
> + represents one remote subsystem and each child of this subsystem in-turn
> + represents separate cooling devices.
> +
> +properties:
> + $nodename:
> + pattern: "^(modem|adsp|cdsp|slpi[0-9])?$"

No need to enforce node name.

> +
> + qcom,instance-id:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Remote subsystem QMI server instance id to be used for communicating with QMI.
> +
> +patternProperties:
> + "^tmd-device[0-9]?$":
> + type: object
> + description:
> + Subnodes indicating tmd cooling device of a specific category.
> + properties:
> + label:
> + maxItems: 1
> + description: |
> + Remote subsystem device identifier. Acceptable device names -
> + "pa" -> for pa cooling device,
> + "cpuv_restriction_cold" -> for vdd restriction,
> + "cx_vdd_limit" -> for vdd limit,
> + "modem" -> for processor passive cooling device,
> + "modem_current" -> for current limiting device,
> + "modem_bw" -> for bus bandwidth limiting device,
> + "cpr_cold" -> for cpr restriction.

Why these are acceptable and others are not? What does it mean here the
"acceptable"?

Best regards,
Krzysztof