[PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Caleb Connolly]

The Thermal Mitigation Device (TMD) Service is a QMI service that runs
on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
It exposes various mitigations including passive thermal controls and
rail voltage restrictions.

This series introduces support for exposing TMDs as cooling devices
in the kernel through the thermal framework, using the QMI interface.

Each TMD client is described as a child of the remoteproc node in
devicetree. With subnodes for each control.

This series is based on previous work by Bhupesh Sharma which can be
found at [1]. I'm sending this as a fresh series as it has been a
year since the original version and I have rewritten most of the driver.

[1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/

---
Caleb Connolly (4):
      remoteproc: qcom: probe all child devices
      dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
      thermal: qcom: add qmi-cooling driver
      MAINTAINERS: Add entry for Qualcomm Cooling Driver

 .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
 .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
 .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
 MAINTAINERS                                        |   8 +
 drivers/remoteproc/qcom_q6v5.c                     |   4 +
 drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
 drivers/thermal/qcom/Kconfig                       |  13 +
 drivers/thermal/qcom/Makefile                      |   1 +
 drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
 drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
 10 files changed, 1161 insertions(+), 8 deletions(-)
---
base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812

// Caleb (they/them)

[PATCH 1/4] remoteproc: qcom: probe all child devices

[Caleb Connolly]

Generalise the qcom,bam-dmux child node support by probing all
remoteproc children with of_platform_populate(). This will be used to
enable support for devices which are best represented as subnodes of the
remoteproc, such as those representing QMI clients.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/remoteproc/qcom_q6v5.c     | 4 ++++
 drivers/remoteproc/qcom_q6v5_mss.c | 8 --------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
index 4ee5e67a9f03..8aa5c7b05429 100644
--- a/drivers/remoteproc/qcom_q6v5.c
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2014 Sony Mobile Communications AB
  * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
  */
+#include <linux/of_platform.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/interconnect.h>
@@ -349,6 +350,8 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
 		return dev_err_probe(&pdev->dev, PTR_ERR(q6v5->path),
 				     "failed to acquire interconnect path\n");
 
+	of_platform_populate(q6v5->dev->of_node, NULL, NULL, q6v5->dev);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
@@ -359,6 +362,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_init);
  */
 void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
 {
+	of_platform_depopulate(q6v5->dev);
 	qmp_put(q6v5->qmp);
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 22fe7b5f5236..ab7e426777c7 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -230,7 +230,6 @@ struct q6v5 {
 	struct qcom_rproc_subdev smd_subdev;
 	struct qcom_rproc_ssr ssr_subdev;
 	struct qcom_sysmon *sysmon;
-	struct platform_device *bam_dmux;
 	bool need_mem_protection;
 	bool has_alt_reset;
 	bool has_mba_logs;
@@ -1969,7 +1968,6 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 static int q6v5_probe(struct platform_device *pdev)
 {
 	const struct rproc_hexagon_res *desc;
-	struct device_node *node;
 	struct q6v5 *qproc;
 	struct rproc *rproc;
 	const char *mba_image;
@@ -2113,10 +2111,6 @@ static int q6v5_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_sysmon_subdev;
 
-	node = of_get_compatible_child(pdev->dev.of_node, "qcom,bam-dmux");
-	qproc->bam_dmux = of_platform_device_create(node, NULL, &pdev->dev);
-	of_node_put(node);
-
 	return 0;
 
 remove_sysmon_subdev:
@@ -2138,8 +2132,6 @@ static void q6v5_remove(struct platform_device *pdev)
 	struct q6v5 *qproc = platform_get_drvdata(pdev);
 	struct rproc *rproc = qproc->rproc;
 
-	if (qproc->bam_dmux)
-		of_platform_device_destroy(&qproc->bam_dmux->dev, NULL);
 	rproc_del(rproc);
 
 	qcom_q6v5_deinit(&qproc->q6v5);

-- 
2.42.0

[PATCH 2/4] dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings

[Caleb Connolly]

The cooling subnode of a remoteproc represents a client of the Thermal
Mitigation Device QMI service running on it. Each subnode of the cooling
node represents a single control exposed by the service.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 ++
 .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
 .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++++++++++++++++
 3 files changed, 187 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml
index 0643faae2c39..9d0398764a31 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8996-mss-pil.yaml
@@ -145,6 +145,12 @@ properties:
       and devices related to the Modem.
     unevaluatedProperties: false
 
+  cooling:
+    $ref: /schemas/thermal/qcom,qmi-cooling.yaml#
+    description:
+      Cooling subnode which represents the cooling devices exposed by the Modem.
+    unevaluatedProperties: false
+
   # Deprecated properties
   mba:
     type: object
@@ -386,6 +392,13 @@ examples:
         qcom,smem-states = <&modem_smp2p_out 0>;
         qcom,smem-state-names = "stop";
 
+        cooling {
+            vdd {
+              label = "cpuv_restriction_cold";
+              #cooling-cells = <2>;
+            };
+        };
+
         glink-edge {
             interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
             label = "modem";
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
index 63a82e7a8bf8..bbc82253f76b 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
@@ -77,6 +77,12 @@ properties:
       and devices related to the ADSP.
     unevaluatedProperties: false
 
+  cooling:
+    $ref: /schemas/thermal/qcom,qmi-cooling.yaml#
+    description:
+      Cooling subnode which represents the cooling devices exposed by the Modem.
+    unevaluatedProperties: false
+
 required:
   - clocks
   - clock-names
diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml
new file mode 100644
index 000000000000..65b1c7b40c1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 (c), Linaro Limited
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/qcom,qmi-cooling.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QMI based thermal mitigation (TMD) cooling devices.
+
+maintainers:
+  - Caleb Connolly <caleb.connolly@linaro.org>
+
+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. Some devices such as "cpuv_restriction_cold" are for warming,
+  (e.g. by raising minimum voltage on core system rails).
+
+  Each subnode represents a control for a single instance of the TMD service running
+  on a remote subsystem.
+
+definitions:
+  tmd:
+    type: object
+    description: |
+      A single Thermal Mitigation Device exposed by a remote subsystem.
+    properties:
+      label:
+        maxItems: 1
+      "#cooling-cells":
+        $ref: /schemas/thermal/thermal-cooling-devices.yaml#/properties/#cooling-cells
+
+    required:
+      - label
+      - "#cooling-cells"
+
+    additionalProperties: false
+
+properties:
+  compatible:
+    enum:
+      - qcom,qmi-cooling-modem
+      - qcom,qmi-cooling-adsp
+      - qcom,qmi-cooling-cdsp
+      - qcom,qmi-cooling-slpi
+
+  vdd:
+    $ref: "#/definitions/tmd"
+    description:
+      Restrict primary rail minimum voltage to "nominal" setting.
+    properties:
+      label:
+        const: cpuv_restriction_cold
+
+required:
+  - compatible
+  - vdd
+
+# Modem has additional TMDs
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,qmi-cooling-modem
+    then:
+      properties:
+        pa:
+          $ref: "#/definitions/tmd"
+          description:
+            Power Amplifier TMD
+          properties:
+            label:
+              const: pa
+
+        proc:
+          $ref: "#/definitions/tmd"
+          description:
+            Modem processor temperature TMD
+          properties:
+            label:
+              const: modem
+
+        current:
+          $ref: "#/definitions/tmd"
+          description:
+            Modem peak current TMD
+          properties:
+            label:
+              const: modem_current
+
+        skin:
+          $ref: "#/definitions/tmd"
+          description:
+            Modem skin temperature TMD
+          properties:
+            label:
+              const: modem_skin
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    remoteproc-modem {
+        cooling {
+            compatible = "qcom,qmi-cooling-modem";
+
+            modem_pa: pa {
+              label = "pa";
+              #cooling-cells = <2>;
+            };
+
+            modem_proc: proc {
+              label = "modem";
+              #cooling-cells = <2>;
+            };
+
+            modem_current: current {
+              label = "modem_current";
+              #cooling-cells = <2>;
+            };
+
+            modem_skin: skin {
+              label = "modem_skin";
+              #cooling-cells = <2>;
+            };
+
+            modem_vdd: vdd {
+              label = "cpuv_restriction_cold";
+              #cooling-cells = <2>;
+            };
+        };
+    };
+
+    remoteproc-adsp {
+        cooling {
+            compatible = "qcom,qmi-cooling-adsp";
+
+            adsp_vdd: vdd {
+              label = "cpuv_restriction_cold";
+              #cooling-cells = <2>;
+            };
+        };
+    };
+
+    remoteproc-cdsp {
+        cooling {
+            compatible = "qcom,qmi-cooling-cdsp";
+
+            cdsp_vdd: vdd {
+              label = "cpuv_restriction_cold";
+              #cooling-cells = <2>;
+            };
+        };
+    };
+
+    remoteproc-slpi {
+        cooling {
+            compatible = "qcom,qmi-cooling-slpi";
+
+            slpi_vdd: vdd {
+              label = "cpuv_restriction_cold";
+              #cooling-cells = <2>;
+            };
+        };
+    };
+...

-- 
2.42.0

Re: [PATCH 2/4] dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings

[Rob Herring]

On Fri, 29 Sep 2023 17:16:18 +0100, Caleb Connolly wrote:
> The cooling subnode of a remoteproc represents a client of the Thermal
> Mitigation Device QMI service running on it. Each subnode of the cooling
> node represents a single control exposed by the service.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>  .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 ++
>  .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
>  .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++++++++++++++++
>  3 files changed, 187 insertions(+)
> 

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,msm8996-mss-pil.example.dtb: remoteproc@4080000: cooling: 'compatible' is a required property
	from schema $id: http://devicetree.org/schemas/remoteproc/qcom,msm8996-mss-pil.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230905-caleb-qmi_cooling-v1-2-5aa39d4164a7@linaro.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/4] dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings

[Bjorn Andersson]

On Fri, Sep 29, 2023 at 05:16:18PM +0100, Caleb Connolly wrote:
> diff --git a/Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml b/Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml
> +definitions:
> +  tmd:
> +    type: object
> +    description: |

No need to preserve formatting (which is what '|' denotes).

> +      A single Thermal Mitigation Device exposed by a remote subsystem.
> +    properties:
> +      label:
> +        maxItems: 1
> +      "#cooling-cells":
> +        $ref: /schemas/thermal/thermal-cooling-devices.yaml#/properties/#cooling-cells
> +
> +    required:
> +      - label
> +      - "#cooling-cells"
> +
> +    additionalProperties: false
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,qmi-cooling-modem
> +      - qcom,qmi-cooling-adsp
> +      - qcom,qmi-cooling-cdsp
> +      - qcom,qmi-cooling-slpi
> +
> +  vdd:
> +    $ref: "#/definitions/tmd"
> +    description:
> +      Restrict primary rail minimum voltage to "nominal" setting.

Isn't this one of the "heating" thermal mitigations? (I.e. something
being tripped when the temperature goes below some level) Which afaik
the framework doesn't support still?

Regards,
Bjorn

[PATCH 3/4] thermal: qcom: add qmi-cooling driver

[Caleb Connolly]

The Thermal Mitigation Device (TMD) service exposes various platform
specific thermal mitigations available on remote subsystems (ie the
modem, adsp, cdsp, and sdsp). The service is exposed via the QMI messaging
interface, usually over the QRTR transport.

These controls affect things like the power limits of the modem power
amplifier and cpu core, skin temperature mitigations, as well as rail
voltage restrictions under cold conditions.

Each remote subsystem has its own TMD instance, where each child node
represents a single thermal control.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/thermal/qcom/Kconfig       |  13 +
 drivers/thermal/qcom/Makefile      |   1 +
 drivers/thermal/qcom/qmi-cooling.c | 520 +++++++++++++++++++++++++++++++++++++
 drivers/thermal/qcom/qmi-cooling.h | 428 ++++++++++++++++++++++++++++++
 4 files changed, 962 insertions(+)

diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
index 2c7f3f9a26eb..a24c840b78b3 100644
--- a/drivers/thermal/qcom/Kconfig
+++ b/drivers/thermal/qcom/Kconfig
@@ -41,3 +41,16 @@ config QCOM_LMH
 	  input from temperature and current sensors.  On many newer Qualcomm SoCs
 	  LMh is configured in the firmware and this feature need not be enabled.
 	  However, on certain SoCs like sdm845 LMh has to be configured from kernel.
+
+config QCOM_QMI_COOLING
+	tristate "Qualcomm QMI cooling drivers"
+	depends on QCOM_RPROC_COMMON
+	depends on ARCH_QCOM || COMPILE_TEST
+	select QCOM_QMI_HELPERS
+	help
+	   This enables the remote subsystem cooling devices. These cooling
+	   devices will be used by Qualcomm chipset to place various remote
+	   subsystem mitigations like remote processor passive mitigation,
+	   remote subsystem voltage restriction at low temperatures etc.
+	   The QMI cooling device will interface with remote subsystem
+	   using Qualcomm remoteproc interface.
diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
index 0fa2512042e7..94dd98e89af9 100644
--- a/drivers/thermal/qcom/Makefile
+++ b/drivers/thermal/qcom/Makefile
@@ -6,3 +6,4 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
 obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_QCOM_LMH)		+= lmh.o
+obj-$(CONFIG_QCOM_QMI_COOLING) += qmi-cooling.o
diff --git a/drivers/thermal/qcom/qmi-cooling.c b/drivers/thermal/qcom/qmi-cooling.c
new file mode 100644
index 000000000000..c34fecd7eefa
--- /dev/null
+++ b/drivers/thermal/qcom/qmi-cooling.c
@@ -0,0 +1,520 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017, The Linux Foundation
+ * Copyright (c) 2023, Linaro Limited
+ *
+ * QMI Thermal Mitigation Device (TMD) client driver.
+ * This driver provides an in-kernel client to handle hot and cold thermal
+ * mitigations for remote subsystems (modem and DSPs) running the TMD service.
+ * It doesn't implement any handling of reports from remote subsystems.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc/qcom_rproc.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/qmi.h>
+#include <linux/thermal.h>
+
+#include "qmi-cooling.h"
+
+#define MODEM0_INSTANCE_ID	0x0
+#define ADSP_INSTANCE_ID	0x1
+#define CDSP_INSTANCE_ID	0x43
+#define SLPI_INSTANCE_ID	0x53
+
+#define QMI_TMD_RESP_TIMEOUT msecs_to_jiffies(100)
+
+/**
+ * struct qmi_instance_id - QMI instance ID and name
+ * @id:		The QMI instance ID
+ * @name:	Friendly name for this instance
+ */
+struct qmi_instance_id {
+	u32 id;
+	const char *name;
+};
+
+/**
+ * struct qmi_tmd_client - TMD client state
+ * @dev:	Device associated with this client
+ * @name:	Friendly name for the remote TMD service
+ * @handle:	QMI connection handle
+ * @mutex:	Lock to synchronise QMI communication
+ * @id:		The QMI TMD service instance ID
+ * @cdev_list:	The list of cooling devices (controls) enabled for this instance
+ * @svc_arrive_work: Work item for initialising the client when the TMD service
+ *		     starts.
+ * @connection_active: Whether or not we're connected to the QMI TMD service
+ */
+struct qmi_tmd_client {
+	struct device *dev;
+	const char *name;
+	struct qmi_handle handle;
+	struct mutex mutex;
+	u32 id;
+	struct list_head cdev_list;
+	struct work_struct svc_arrive_work;
+	bool connection_active;
+};
+
+/**
+ * struct qmi_tmd - A TMD cooling device
+ * @np:		OF node associated with this control
+ * @type:	The control type (exposed via sysfs)
+ * @qmi_name:	The common name of this control shared by the remote subsystem
+ * @cdev:	Thermal framework cooling device handle
+ * @cur_state:	The current cooling/warming/mitigation state
+ * @max_state:	The maximum state
+ * @client:	The TMD client instance this control is associated with
+ */
+struct qmi_tmd {
+	struct device_node *np;
+	const char *type;
+	char qmi_name[QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1];
+	struct list_head node;
+	struct thermal_cooling_device *cdev;
+	unsigned int cur_state;
+	unsigned int max_state;
+	struct qmi_tmd_client *client;
+};
+
+/* Notify the remote subsystem of the requested cooling state */
+static int qmi_tmd_send_state_request(struct qmi_tmd *tmd)
+{
+	struct tmd_set_mitigation_level_resp_msg_v01 tmd_resp;
+	struct tmd_set_mitigation_level_req_msg_v01 req;
+	struct qmi_tmd_client *client;
+	struct qmi_txn txn;
+	int ret = 0;
+
+	client = tmd->client;
+
+	if (!client->connection_active)
+		return 0;
+
+	memset(&req, 0, sizeof(req));
+	memset(&tmd_resp, 0, sizeof(tmd_resp));
+
+	strscpy(req.mitigation_dev_id.mitigation_dev_id, tmd->qmi_name,
+		QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
+	req.mitigation_level = tmd->cur_state;
+
+	mutex_lock(&client->mutex);
+
+	ret = qmi_txn_init(&client->handle, &txn,
+			   tmd_set_mitigation_level_resp_msg_v01_ei, &tmd_resp);
+	if (ret < 0) {
+		dev_err(client->dev, "qmi set state %d txn init failed for %s ret %d\n",
+			tmd->cur_state, tmd->type, ret);
+		goto qmi_send_exit;
+	}
+
+	ret = qmi_send_request(&client->handle, NULL, &txn,
+			       QMI_TMD_SET_MITIGATION_LEVEL_REQ_V01,
+			       TMD_SET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN,
+			       tmd_set_mitigation_level_req_msg_v01_ei, &req);
+	if (ret < 0) {
+		dev_err(client->dev, "qmi set state %d txn send failed for %s ret %d\n",
+			tmd->cur_state, tmd->type, ret);
+		qmi_txn_cancel(&txn);
+		goto qmi_send_exit;
+	}
+
+	ret = qmi_txn_wait(&txn, QMI_TMD_RESP_TIMEOUT);
+	if (ret < 0) {
+		dev_err(client->dev, "qmi set state %d txn wait failed for %s ret %d\n",
+			tmd->cur_state, tmd->type, ret);
+		goto qmi_send_exit;
+	}
+	ret = 0;
+
+	if (tmd_resp.resp.result != QMI_RESULT_SUCCESS_V01) {
+		ret = tmd_resp.resp.result;
+		dev_err(client->dev, "qmi set state %d NOT success for %s ret %d\n",
+			tmd->cur_state, tmd->type, ret);
+		goto qmi_send_exit;
+	}
+
+	dev_dbg(client->dev, "Requested state %d/%d for %s\n", tmd->cur_state,
+		tmd->max_state, tmd->type);
+
+qmi_send_exit:
+	mutex_unlock(&client->mutex);
+	return ret;
+}
+
+static int qmi_get_max_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	struct qmi_tmd *tmd = cdev->devdata;
+
+	if (!tmd)
+		return -EINVAL;
+
+	*state = tmd->max_state;
+
+	return 0;
+}
+
+static int qmi_get_cur_state(struct thermal_cooling_device *cdev, unsigned long *state)
+{
+	struct qmi_tmd *tmd = cdev->devdata;
+
+	if (!tmd)
+		return -EINVAL;
+
+	*state = tmd->cur_state;
+
+	return 0;
+}
+
+static int qmi_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
+{
+	struct qmi_tmd *tmd = cdev->devdata;
+
+	if (!tmd)
+		return -EINVAL;
+
+	if (state > tmd->max_state)
+		return -EINVAL;
+
+	if (tmd->cur_state == state)
+		return 0;
+
+	tmd->cur_state = state;
+
+	return qmi_tmd_send_state_request(tmd);
+}
+
+static struct thermal_cooling_device_ops qmi_device_ops = {
+	.get_max_state = qmi_get_max_state,
+	.get_cur_state = qmi_get_cur_state,
+	.set_cur_state = qmi_set_cur_state,
+};
+
+static int qmi_register_cooling_device(struct qmi_tmd *tmd)
+{
+	struct thermal_cooling_device *cdev;
+
+	cdev = thermal_of_cooling_device_register(tmd->np, tmd->type, tmd,
+						  &qmi_device_ops);
+
+	if (IS_ERR(cdev))
+		return dev_err_probe(tmd->client->dev, PTR_ERR(tmd->cdev),
+				     "Failed to register cooling device %s\n",
+				     tmd->qmi_name);
+
+	tmd->cdev = cdev;
+	return 0;
+}
+
+/*
+ * Init a single TMD control by registering a cooling device for it, or
+ * synchronising state with the remote subsystem if recovering from a service
+ * restart. This is called when the TMD service starts up.
+ */
+static int qmi_tmd_init_control(struct qmi_tmd_client *client, const char *label,
+				u8 max_state)
+{
+	struct qmi_tmd *tmd = NULL;
+
+	list_for_each_entry(tmd, &client->cdev_list, node)
+		if (!strncasecmp(tmd->qmi_name, label,
+				 QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1))
+			goto found;
+
+	dev_dbg(client->dev,
+		"TMD '%s' available in firmware but not specified in DT\n",
+		label);
+	return 0;
+
+found:
+	tmd->max_state = max_state;
+	/*
+	 * If the cooling device already exists then the QMI service went away and
+	 * came back. So just make sure the current cooling device state is
+	 * reflected on the remote side and then return.
+	 */
+	if (tmd->cdev)
+		return qmi_tmd_send_state_request(tmd);
+
+	return qmi_register_cooling_device(tmd);
+}
+
+/*
+ * When the QMI service starts up on a remote subsystem this function will fetch
+ * the list of TMDs on the subsystem, match it to the TMDs specified in devicetree
+ * and call qmi_tmd_init_control() for each
+ */
+static void qmi_tmd_svc_arrive(struct work_struct *work)
+{
+	struct qmi_tmd_client *client =
+		container_of(work, struct qmi_tmd_client, svc_arrive_work);
+
+	struct tmd_get_mitigation_device_list_req_msg_v01 req;
+	struct tmd_get_mitigation_device_list_resp_msg_v01 *resp;
+	int ret = 0, i;
+	struct qmi_txn txn;
+
+	memset(&req, 0, sizeof(req));
+	/* resp struct is 1.1kB, allocate it on the heap. */
+	resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+	if (!resp)
+		return;
+
+	/* Get a list of TMDs supported by the remoteproc */
+	mutex_lock(&client->mutex);
+	ret = qmi_txn_init(&client->handle, &txn,
+			   tmd_get_mitigation_device_list_resp_msg_v01_ei, resp);
+	if (ret < 0) {
+		dev_err(client->dev,
+			"Transaction init error for instance_id: %#x ret %d\n",
+			client->id, ret);
+		goto reg_exit;
+	}
+
+	ret = qmi_send_request(&client->handle, NULL, &txn,
+			       QMI_TMD_GET_MITIGATION_DEVICE_LIST_REQ_V01,
+			       TMD_GET_MITIGATION_DEVICE_LIST_REQ_MSG_V01_MAX_MSG_LEN,
+			       tmd_get_mitigation_device_list_req_msg_v01_ei, &req);
+	if (ret < 0) {
+		qmi_txn_cancel(&txn);
+		goto reg_exit;
+	}
+
+	ret = qmi_txn_wait(&txn, QMI_TMD_RESP_TIMEOUT);
+	if (ret < 0) {
+		dev_err(client->dev, "Transaction wait error for client %#x ret:%d\n",
+			client->id, ret);
+		goto reg_exit;
+	}
+	if (resp->resp.result != QMI_RESULT_SUCCESS_V01) {
+		ret = resp->resp.result;
+		dev_err(client->dev, "Failed to get device list for client %#x ret:%d\n",
+			client->id, ret);
+		goto reg_exit;
+	}
+	mutex_unlock(&client->mutex);
+
+	client->connection_active = true;
+
+	for (i = 0; i < resp->mitigation_device_list_len; i++) {
+		struct tmd_mitigation_dev_list_type_v01 *device =
+			&resp->mitigation_device_list[i];
+
+		ret = qmi_tmd_init_control(client,
+					   device->mitigation_dev_id.mitigation_dev_id,
+					   device->max_mitigation_level);
+		if (ret)
+			break;
+	}
+
+	kfree(resp);
+	return;
+
+reg_exit:
+	mutex_unlock(&client->mutex);
+	kfree(resp);
+}
+
+static void thermal_qmi_net_reset(struct qmi_handle *qmi)
+{
+	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
+	struct qmi_tmd *tmd = NULL;
+
+	pr_info("QMI TMD service reset %s\n", client->name);
+
+	list_for_each_entry(tmd, &client->cdev_list, node) {
+		qmi_tmd_send_state_request(tmd);
+	}
+}
+
+static void thermal_qmi_del_server(struct qmi_handle *qmi, struct qmi_service *service)
+{
+	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
+
+	pr_info("QMI TMD service stop %s\n", client->name);
+
+	client->connection_active = false;
+}
+
+static int thermal_qmi_new_server(struct qmi_handle *qmi, struct qmi_service *service)
+{
+	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
+	struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port };
+
+	pr_info("QMI TMD service start %s\n", client->name);
+
+	mutex_lock(&client->mutex);
+	kernel_connect(qmi->sock, (struct sockaddr *)&sq, sizeof(sq), 0);
+	mutex_unlock(&client->mutex);
+	queue_work(system_highpri_wq, &client->svc_arrive_work);
+
+	return 0;
+}
+
+static struct qmi_ops thermal_qmi_event_ops = {
+	.new_server = thermal_qmi_new_server,
+	.del_server = thermal_qmi_del_server,
+	.net_reset = thermal_qmi_net_reset,
+};
+
+static void qmi_tmd_cleanup(struct qmi_tmd_client *client)
+{
+	struct qmi_tmd *tmd, *c_next;
+
+	client->connection_active = false;
+
+	mutex_lock(&client->mutex);
+	qmi_handle_release(&client->handle);
+	cancel_work(&client->svc_arrive_work);
+	list_for_each_entry_safe(tmd, c_next, &client->cdev_list, node) {
+		if (tmd->cdev)
+			thermal_cooling_device_unregister(tmd->cdev);
+
+		list_del(&tmd->node);
+	}
+	mutex_unlock(&client->mutex);
+}
+
+/* Parse the controls and allocate a qmi_tmd for each of them */
+static int qmi_tmd_alloc_cdevs(struct qmi_tmd_client *client)
+{
+	struct device *dev = client->dev;
+	struct qmi_tmd *tmd;
+	struct device_node *subnode, *node = dev->of_node;
+	int ret;
+
+	for_each_available_child_of_node(node, subnode) {
+		const char *name;
+
+		tmd = devm_kzalloc(dev, sizeof(*tmd), GFP_KERNEL);
+		if (!tmd)
+			return dev_err_probe(client->dev, -ENOMEM,
+					     "Couldn't allocate tmd\n");
+
+		tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s",
+						client->name, subnode->name);
+		if (!tmd->type)
+			return dev_err_probe(dev, -ENOMEM, "Cooling device name\n");
+
+		if (of_property_read_string(subnode, "label", &name)) {
+			return dev_err_probe(client->dev, -EINVAL,
+					     "Fail to parse dev name for %s\n",
+					     subnode->name);
+		}
+
+		ret = strscpy(tmd->qmi_name, name,
+			      QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
+		if (ret == -E2BIG) {
+			return dev_err_probe(dev, -EINVAL, "TMD label %s is too long\n",
+					     name);
+		}
+
+		tmd->client = client;
+		tmd->np = subnode;
+		tmd->cur_state = 0;
+		list_add(&tmd->node, &client->cdev_list);
+	}
+
+	if (list_empty(&client->cdev_list))
+		return dev_err_probe(client->dev, -EINVAL,
+				     "No cooling devices specified for client %s (%#x)\n",
+				     client->name, client->id);
+
+	return 0;
+}
+
+static int qmi_tmd_client_probe(struct platform_device *pdev)
+{
+	struct device *dev;
+	struct qmi_tmd_client *client;
+	const struct qmi_instance_id *match;
+	int ret;
+
+	dev = &pdev->dev;
+
+	client = devm_kzalloc(dev, sizeof(*client), GFP_KERNEL);
+	if (!client)
+		return -ENOMEM;
+
+	client->dev = dev;
+
+	match = of_device_get_match_data(dev);
+	if (!match)
+		return dev_err_probe(dev, -EINVAL, "No match data\n");
+
+	client->id = match->id;
+	client->name = match->name;
+
+	mutex_init(&client->mutex);
+	INIT_LIST_HEAD(&client->cdev_list);
+	INIT_WORK(&client->svc_arrive_work, qmi_tmd_svc_arrive);
+
+	ret = qmi_tmd_alloc_cdevs(client);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, client);
+
+	ret = qmi_handle_init(&client->handle,
+			      TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN,
+			      &thermal_qmi_event_ops, NULL);
+	if (ret < 0)
+		return dev_err_probe(client->dev, ret, "QMI handle init failed for client %#x\n",
+			      client->id);
+
+	ret = qmi_add_lookup(&client->handle, TMD_SERVICE_ID_V01, TMD_SERVICE_VERS_V01,
+			     client->id);
+	if (ret < 0) {
+		qmi_handle_release(&client->handle);
+		return dev_err_probe(client->dev, ret, "QMI register failed for client 0x%x\n",
+			      client->id);
+	}
+
+	return 0;
+}
+
+static int qmi_tmd_client_remove(struct platform_device *pdev)
+{
+	struct qmi_tmd_client *client = platform_get_drvdata(pdev);
+
+	qmi_tmd_cleanup(client);
+
+	return 0;
+}
+
+static const struct of_device_id qmi_tmd_device_table[] = {
+	{
+		.compatible = "qcom,qmi-cooling-modem",
+		.data = &((struct qmi_instance_id) { MODEM0_INSTANCE_ID, "modem" }),
+	}, {
+		.compatible = "qcom,qmi-cooling-adsp",
+		.data = &((struct qmi_instance_id) { ADSP_INSTANCE_ID, "adsp" }),
+	}, {
+		.compatible = "qcom,qmi-cooling-cdsp",
+		.data = &((struct qmi_instance_id) { CDSP_INSTANCE_ID, "cdsp" }),
+	}, {
+		.compatible = "qcom,qmi-cooling-slpi",
+		.data = &((struct qmi_instance_id) { SLPI_INSTANCE_ID, "slpi" }),
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, qmi_tmd_device_table);
+
+static struct platform_driver qmi_tmd_device_driver = {
+	.probe          = qmi_tmd_client_probe,
+	.remove         = qmi_tmd_client_remove,
+	.driver         = {
+		.name   = "qcom-qmi-cooling",
+		.of_match_table = qmi_tmd_device_table,
+	},
+};
+
+module_platform_driver(qmi_tmd_device_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Qualcomm QMI Thermal Mitigation Device driver");
diff --git a/drivers/thermal/qcom/qmi-cooling.h b/drivers/thermal/qcom/qmi-cooling.h
new file mode 100644
index 000000000000..f46b827b4ce6
--- /dev/null
+++ b/drivers/thermal/qcom/qmi-cooling.h
@@ -0,0 +1,428 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2017, The Linux Foundation
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#ifndef __QCOM_COOLING_H__
+#define __QCOM_COOLING_H__
+
+#include <linux/soc/qcom/qmi.h>
+
+#define TMD_SERVICE_ID_V01 0x18
+#define TMD_SERVICE_VERS_V01 0x01
+
+#define QMI_TMD_GET_MITIGATION_DEVICE_LIST_RESP_V01 0x0020
+#define QMI_TMD_GET_MITIGATION_LEVEL_REQ_V01 0x0022
+#define QMI_TMD_GET_SUPPORTED_MSGS_REQ_V01 0x001E
+#define QMI_TMD_SET_MITIGATION_LEVEL_REQ_V01 0x0021
+#define QMI_TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_V01 0x0023
+#define QMI_TMD_GET_SUPPORTED_MSGS_RESP_V01 0x001E
+#define QMI_TMD_SET_MITIGATION_LEVEL_RESP_V01 0x0021
+#define QMI_TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_V01 0x0024
+#define QMI_TMD_MITIGATION_LEVEL_REPORT_IND_V01 0x0025
+#define QMI_TMD_GET_MITIGATION_LEVEL_RESP_V01 0x0022
+#define QMI_TMD_GET_SUPPORTED_FIELDS_REQ_V01 0x001F
+#define QMI_TMD_GET_MITIGATION_DEVICE_LIST_REQ_V01 0x0020
+#define QMI_TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_V01 0x0023
+#define QMI_TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_V01 0x0024
+#define QMI_TMD_GET_SUPPORTED_FIELDS_RESP_V01 0x001F
+
+#define QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 32
+#define QMI_TMD_MITIGATION_DEV_LIST_MAX_V01 32
+
+struct tmd_mitigation_dev_id_type_v01 {
+	char mitigation_dev_id[QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1];
+};
+
+static const struct qmi_elem_info tmd_mitigation_dev_id_type_v01_ei[] = {
+	{
+		.data_type = QMI_STRING,
+		.elem_len = QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1,
+		.elem_size = sizeof(char),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0,
+		.offset = offsetof(struct tmd_mitigation_dev_id_type_v01,
+				   mitigation_dev_id),
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_mitigation_dev_list_type_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_dev_id;
+	uint8_t max_mitigation_level;
+};
+
+static const struct qmi_elem_info tmd_mitigation_dev_list_type_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0,
+		.offset = offsetof(struct tmd_mitigation_dev_list_type_v01,
+				   mitigation_dev_id),
+		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0,
+		.offset = offsetof(struct tmd_mitigation_dev_list_type_v01,
+				   max_mitigation_level),
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_get_mitigation_device_list_req_msg_v01 {
+	char placeholder;
+};
+
+#define TMD_GET_MITIGATION_DEVICE_LIST_REQ_MSG_V01_MAX_MSG_LEN 0
+const struct qmi_elem_info tmd_get_mitigation_device_list_req_msg_v01_ei[] = {
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_get_mitigation_device_list_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+	uint8_t mitigation_device_list_valid;
+	uint32_t mitigation_device_list_len;
+	struct tmd_mitigation_dev_list_type_v01
+		mitigation_device_list[QMI_TMD_MITIGATION_DEV_LIST_MAX_V01];
+};
+
+#define TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN 1099
+static const struct qmi_elem_info tmd_get_mitigation_device_list_resp_msg_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct qmi_response_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
+				   resp),
+		.ei_array = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type = QMI_OPT_FLAG,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
+				   mitigation_device_list_valid),
+	},
+	{
+		.data_type = QMI_DATA_LEN,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
+				   mitigation_device_list_len),
+	},
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = QMI_TMD_MITIGATION_DEV_LIST_MAX_V01,
+		.elem_size = sizeof(struct tmd_mitigation_dev_list_type_v01),
+		.array_type = VAR_LEN_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
+				   mitigation_device_list),
+		.ei_array = tmd_mitigation_dev_list_type_v01_ei,
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_set_mitigation_level_req_msg_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_dev_id;
+	uint8_t mitigation_level;
+};
+
+#define TMD_SET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 40
+static const struct qmi_elem_info tmd_set_mitigation_level_req_msg_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct tmd_set_mitigation_level_req_msg_v01,
+				   mitigation_dev_id),
+		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct tmd_set_mitigation_level_req_msg_v01,
+				   mitigation_level),
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_set_mitigation_level_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+};
+
+#define TMD_SET_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
+static const struct qmi_elem_info tmd_set_mitigation_level_resp_msg_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct qmi_response_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct tmd_set_mitigation_level_resp_msg_v01, resp),
+		.ei_array = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_get_mitigation_level_req_msg_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
+};
+#define TMD_GET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
+
+static const struct qmi_elem_info tmd_get_mitigation_level_req_msg_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct tmd_get_mitigation_level_req_msg_v01,
+				   mitigation_device),
+		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_get_mitigation_level_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+	uint8_t current_mitigation_level_valid;
+	uint8_t current_mitigation_level;
+	uint8_t requested_mitigation_level_valid;
+	uint8_t requested_mitigation_level;
+};
+
+#define TMD_GET_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 15
+static const struct qmi_elem_info tmd_get_mitigation_level_resp_msg_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct qmi_response_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01, resp),
+		.ei_array = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type = QMI_OPT_FLAG,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
+				   current_mitigation_level_valid),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x10,
+		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
+				   current_mitigation_level),
+	},
+	{
+		.data_type = QMI_OPT_FLAG,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x11,
+		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
+				   requested_mitigation_level_valid),
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x11,
+		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
+				   requested_mitigation_level),
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+struct tmd_register_notification_mitigation_level_req_msg_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
+};
+
+#define TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
+static const struct qmi_elem_info
+	tmd_register_notification_mitigation_level_req_msg_v01_ei[] = {
+		{
+			.data_type = QMI_STRUCT,
+			.elem_len = 1,
+			.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+			.array_type = NO_ARRAY,
+			.tlv_type = 0x01,
+			.offset = offsetof(
+				struct tmd_register_notification_mitigation_level_req_msg_v01,
+				mitigation_device),
+			.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+		},
+		{
+			.data_type = QMI_EOTI,
+			.array_type = NO_ARRAY,
+			.tlv_type = QMI_COMMON_TLV_TYPE,
+		},
+	};
+
+struct tmd_register_notification_mitigation_level_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+};
+
+#define TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
+static const struct qmi_elem_info
+	tmd_register_notification_mitigation_level_resp_msg_v01_ei[] = {
+		{
+			.data_type = QMI_STRUCT,
+			.elem_len = 1,
+			.elem_size = sizeof(struct qmi_response_type_v01),
+			.array_type = NO_ARRAY,
+			.tlv_type = 0x02,
+			.offset = offsetof(
+				struct tmd_register_notification_mitigation_level_resp_msg_v01,
+				resp),
+			.ei_array = qmi_response_type_v01_ei,
+		},
+		{
+			.data_type = QMI_EOTI,
+			.array_type = NO_ARRAY,
+			.tlv_type = QMI_COMMON_TLV_TYPE,
+		},
+	};
+
+struct tmd_deregister_notification_mitigation_level_req_msg_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
+};
+
+#define TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
+static const struct qmi_elem_info
+	tmd_deregister_notification_mitigation_level_req_msg_v01_ei[] = {
+		{
+			.data_type = QMI_STRUCT,
+			.elem_len = 1,
+			.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+			.array_type = NO_ARRAY,
+			.tlv_type = 0x01,
+			.offset = offsetof(
+				struct tmd_deregister_notification_mitigation_level_req_msg_v01,
+				mitigation_device),
+			.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+		},
+		{
+			.data_type = QMI_EOTI,
+			.array_type = NO_ARRAY,
+			.tlv_type = QMI_COMMON_TLV_TYPE,
+		},
+	};
+
+struct tmd_deregister_notification_mitigation_level_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+};
+
+#define TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
+static const struct qmi_elem_info
+	tmd_deregister_notification_mitigation_level_resp_msg_v01_ei[] = {
+		{
+			.data_type = QMI_STRUCT,
+			.elem_len = 1,
+			.elem_size = sizeof(struct qmi_response_type_v01),
+			.array_type = NO_ARRAY,
+			.tlv_type = 0x02,
+			.offset = offsetof(
+				struct tmd_deregister_notification_mitigation_level_resp_msg_v01,
+				resp),
+			.ei_array = qmi_response_type_v01_ei,
+		},
+		{
+			.data_type = QMI_EOTI,
+			.array_type = NO_ARRAY,
+			.tlv_type = QMI_COMMON_TLV_TYPE,
+		},
+	};
+
+struct tmd_mitigation_level_report_ind_msg_v01 {
+	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
+	uint8_t current_mitigation_level;
+};
+
+#define TMD_MITIGATION_LEVEL_REPORT_IND_MSG_V01_MAX_MSG_LEN 40
+static const struct qmi_elem_info tmd_mitigation_level_report_ind_msg_v01_ei[] = {
+	{
+		.data_type = QMI_STRUCT,
+		.elem_len = 1,
+		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x01,
+		.offset = offsetof(struct tmd_mitigation_level_report_ind_msg_v01,
+				   mitigation_device),
+		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
+	},
+	{
+		.data_type = QMI_UNSIGNED_1_BYTE,
+		.elem_len = 1,
+		.elem_size = sizeof(uint8_t),
+		.array_type = NO_ARRAY,
+		.tlv_type = 0x02,
+		.offset = offsetof(struct tmd_mitigation_level_report_ind_msg_v01,
+				   current_mitigation_level),
+	},
+	{
+		.data_type = QMI_EOTI,
+		.array_type = NO_ARRAY,
+		.tlv_type = QMI_COMMON_TLV_TYPE,
+	},
+};
+
+#endif /* __QMI_COOLING_INTERNAL_H__ */

-- 
2.42.0

Re: [PATCH 3/4] thermal: qcom: add qmi-cooling driver

[Konrad Dybcio]

On 9/29/23 18:16, Caleb Connolly wrote:
> The Thermal Mitigation Device (TMD) service exposes various platform
> specific thermal mitigations available on remote subsystems (ie the
> modem, adsp, cdsp, and sdsp). The service is exposed via the QMI messaging
> interface, usually over the QRTR transport.
> 
> These controls affect things like the power limits of the modem power
> amplifier and cpu core, skin temperature mitigations, as well as rail
> voltage restrictions under cold conditions.
> 
> Each remote subsystem has its own TMD instance, where each child node
> represents a single thermal control.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
[...]

> +/* Notify the remote subsystem of the requested cooling state */
> +static int qmi_tmd_send_state_request(struct qmi_tmd *tmd)
> +{
> +	struct tmd_set_mitigation_level_resp_msg_v01 tmd_resp;
> +	struct tmd_set_mitigation_level_req_msg_v01 req;
> +	struct qmi_tmd_client *client;
> +	struct qmi_txn txn;
> +	int ret = 0;
> +
> +	client = tmd->client;
You can assign it at declaration time
> +
> +	if (!client->connection_active)
> +		return 0;
Is that an expected scenario?

> +
> +	memset(&req, 0, sizeof(req));
> +	memset(&tmd_resp, 0, sizeof(tmd_resp));
Since you're declaring them above, you can do = { 0 }; instead, which
will be faster

> +
> +	strscpy(req.mitigation_dev_id.mitigation_dev_id, tmd->qmi_name,
> +		QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
> +	req.mitigation_level = tmd->cur_state;
> +
> +	mutex_lock(&client->mutex);
Look into fancy scoped mutexes https://lwn.net/Articles/934679/

[...]

> +static int qmi_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
> +{
> +	struct qmi_tmd *tmd = cdev->devdata;
> +
> +	if (!tmd)
> +		return -EINVAL;
> +
> +	if (state > tmd->max_state)
> +		return -EINVAL;Would it be useful if this threw an error in dmesg?


> + * When the QMI service starts up on a remote subsystem this function will fetch
> + * the list of TMDs on the subsystem, match it to the TMDs specified in devicetree
> + * and call qmi_tmd_init_control() for each
> + */
> +static void qmi_tmd_svc_arrive(struct work_struct *work)
> +{
> +	struct qmi_tmd_client *client =
> +		container_of(work, struct qmi_tmd_client, svc_arrive_work);
> +
> +	struct tmd_get_mitigation_device_list_req_msg_v01 req;
> +	struct tmd_get_mitigation_device_list_resp_msg_v01 *resp;
> +	int ret = 0, i;
> +	struct qmi_txn txn;
> +
> +	memset(&req, 0, sizeof(req));
= { 0 }

[...]

> +
> +	pr_info("QMI TMD service reset %s\n", client->name);
Is it useful to the user? pr_debug?

> +
> +	list_for_each_entry(tmd, &client->cdev_list, node) {
> +		qmi_tmd_send_state_request(tmd);
> +	}
> +}
> +
> +static void thermal_qmi_del_server(struct qmi_handle *qmi, struct qmi_service *service)
> +{
> +	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
> +
> +	pr_info("QMI TMD service stop %s\n", client->name);
Ditto

> +
> +	client->connection_active = false;
> +}
> +
> +static int thermal_qmi_new_server(struct qmi_handle *qmi, struct qmi_service *service)
> +{
> +	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
> +	struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port };
> +
> +	pr_info("QMI TMD service start %s\n", client->name);
Ditto

> +		tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s",
> +						client->name, subnode->name);
> +		if (!tmd->type)
> +			return dev_err_probe(dev, -ENOMEM, "Cooling device name\n");
Cooling device name-what? :D

> +
> +		if (of_property_read_string(subnode, "label", &name)) {
> +			return dev_err_probe(client->dev, -EINVAL,
> +					     "Fail to parse dev name for %s\n",
Failed

[...]

> +static int qmi_tmd_client_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct qmi_tmd_client *client;
> +	const struct qmi_instance_id *match;
> +	int ret;
> +
> +	dev = &pdev->dev;
Initialize at declaration

> +
> +	client = devm_kzalloc(dev, sizeof(*client), GFP_KERNEL);
> +	if (!client)
> +		return -ENOMEM;
> +
> +	client->dev = dev;
> +
> +	match = of_device_get_match_data(dev);
> +	if (!match)
> +		return dev_err_probe(dev, -EINVAL, "No match data\n");
> +
> +	client->id = match->id;
> +	client->name = match->name;
> +
> +	mutex_init(&client->mutex);
> +	INIT_LIST_HEAD(&client->cdev_list);
> +	INIT_WORK(&client->svc_arrive_work, qmi_tmd_svc_arrive);
> +
> +	ret = qmi_tmd_alloc_cdevs(client);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, client);
> +
> +	ret = qmi_handle_init(&client->handle,
> +			      TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN,
> +			      &thermal_qmi_event_ops, NULL);
> +	if (ret < 0)
> +		return dev_err_probe(client->dev, ret, "QMI handle init failed for client %#x\n",
> +			      client->id);
> +
> +	ret = qmi_add_lookup(&client->handle, TMD_SERVICE_ID_V01, TMD_SERVICE_VERS_V01,
> +			     client->id);
> +	if (ret < 0) {
> +		qmi_handle_release(&client->handle);
> +		return dev_err_probe(client->dev, ret, "QMI register failed for client 0x%x\n",
> +			      client->id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int qmi_tmd_client_remove(struct platform_device *pdev)
void + .remove_new

Konrad

Re: [PATCH 3/4] thermal: qcom: add qmi-cooling driver

[Caleb Connolly]

On 29/09/2023 17:28, Konrad Dybcio wrote:
> 
> 
> On 9/29/23 18:16, Caleb Connolly wrote:
>> The Thermal Mitigation Device (TMD) service exposes various platform
>> specific thermal mitigations available on remote subsystems (ie the
>> modem, adsp, cdsp, and sdsp). The service is exposed via the QMI
>> messaging
>> interface, usually over the QRTR transport.
>>
>> These controls affect things like the power limits of the modem power
>> amplifier and cpu core, skin temperature mitigations, as well as rail
>> voltage restrictions under cold conditions.
>>
>> Each remote subsystem has its own TMD instance, where each child node
>> represents a single thermal control.
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
> [...]
> 
>> +/* Notify the remote subsystem of the requested cooling state */
>> +static int qmi_tmd_send_state_request(struct qmi_tmd *tmd)
>> +{
>> +    struct tmd_set_mitigation_level_resp_msg_v01 tmd_resp;
>> +    struct tmd_set_mitigation_level_req_msg_v01 req;
>> +    struct qmi_tmd_client *client;
>> +    struct qmi_txn txn;
>> +    int ret = 0;
>> +
>> +    client = tmd->client;
> You can assign it at declaration time

Is this just personal preference or is it subsystem rules? I generally
prefer to avoid non-const assignments at declaration time, it's just
easier to read this way.
>> +
>> +    if (!client->connection_active)
>> +        return 0;
> Is that an expected scenario?

Yes, this function is called by the cooling device set_cur_state op. The
cooling device itself is always registered, even if the remoteproc is
offline (in which case the cached state will be sent to it when it
starts up).
> 
>> +
>> +    memset(&req, 0, sizeof(req));
>> +    memset(&tmd_resp, 0, sizeof(tmd_resp));
> Since you're declaring them above, you can do = { 0 }; instead, which
> will be faster

Thanks, will do (though fwiw the compiler almost definitely optimises
this out).
> 
>> +
>> +    strscpy(req.mitigation_dev_id.mitigation_dev_id, tmd->qmi_name,
>> +        QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
>> +    req.mitigation_level = tmd->cur_state;
>> +
>> +    mutex_lock(&client->mutex);
> Look into fancy scoped mutexes https://lwn.net/Articles/934679/

ooh nice, that actually improves things a lot :D
> 
> [...]
> 
>> +static int qmi_set_cur_state(struct thermal_cooling_device *cdev,
>> unsigned long state)
>> +{
>> +    struct qmi_tmd *tmd = cdev->devdata;
>> +
>> +    if (!tmd)
>> +        return -EINVAL;
>> +
>> +    if (state > tmd->max_state)
>> +        return -EINVAL;Would it be useful if this threw an error in
>> dmesg?

I may be mistaken but I think the only place this will be hit is when
userspace attempts to write to it via sysfs, in that case the invalid
argument is likely enough.
> 
> 
>> + * When the QMI service starts up on a remote subsystem this function
>> will fetch
>> + * the list of TMDs on the subsystem, match it to the TMDs specified
>> in devicetree
>> + * and call qmi_tmd_init_control() for each
>> + */
>> +static void qmi_tmd_svc_arrive(struct work_struct *work)
>> +{
>> +    struct qmi_tmd_client *client =
>> +        container_of(work, struct qmi_tmd_client, svc_arrive_work);
>> +
>> +    struct tmd_get_mitigation_device_list_req_msg_v01 req;
>> +    struct tmd_get_mitigation_device_list_resp_msg_v01 *resp;
>> +    int ret = 0, i;
>> +    struct qmi_txn txn;
>> +
>> +    memset(&req, 0, sizeof(req));
> = { 0 }
> 
> [...]
> 
>> +
>> +    pr_info("QMI TMD service reset %s\n", client->name);
> Is it useful to the user? pr_debug?

Oops, forgot to remove these!
> 
>> +
>> +    list_for_each_entry(tmd, &client->cdev_list, node) {
>> +        qmi_tmd_send_state_request(tmd);
>> +    }
>> +}
>> +
>> +static void thermal_qmi_del_server(struct qmi_handle *qmi, struct
>> qmi_service *service)
>> +{
>> +    struct qmi_tmd_client *client = container_of(qmi, struct
>> qmi_tmd_client, handle);
>> +
>> +    pr_info("QMI TMD service stop %s\n", client->name);
> Ditto
> 
>> +
>> +    client->connection_active = false;
>> +}
>> +
>> +static int thermal_qmi_new_server(struct qmi_handle *qmi, struct
>> qmi_service *service)
>> +{
>> +    struct qmi_tmd_client *client = container_of(qmi, struct
>> qmi_tmd_client, handle);
>> +    struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node,
>> service->port };
>> +
>> +    pr_info("QMI TMD service start %s\n", client->name);
> Ditto
> 
>> +        tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s",
>> +                        client->name, subnode->name);
>> +        if (!tmd->type)
>> +            return dev_err_probe(dev, -ENOMEM, "Cooling device name\n");
> Cooling device name-what? :D

"error -12: Cooling device name"

Maybe this shouldn't print an error at all? I usually just like to have
something to grep for so I don't have to binary search through error
paths when debugging.
> 
>> +
>> +        if (of_property_read_string(subnode, "label", &name)) {
>> +            return dev_err_probe(client->dev, -EINVAL,
>> +                         "Fail to parse dev name for %s\n",
> Failed

ack
> 
> [...]
> 
>> +static int qmi_tmd_client_probe(struct platform_device *pdev)
>> +{
>> +    struct device *dev;
>> +    struct qmi_tmd_client *client;
>> +    const struct qmi_instance_id *match;
>> +    int ret;
>> +
>> +    dev = &pdev->dev;
> Initialize at declaration
> 
>> +
>> +    client = devm_kzalloc(dev, sizeof(*client), GFP_KERNEL);
>> +    if (!client)
>> +        return -ENOMEM;
>> +
>> +    client->dev = dev;
>> +
>> +    match = of_device_get_match_data(dev);
>> +    if (!match)
>> +        return dev_err_probe(dev, -EINVAL, "No match data\n");
>> +
>> +    client->id = match->id;
>> +    client->name = match->name;
>> +
>> +    mutex_init(&client->mutex);
>> +    INIT_LIST_HEAD(&client->cdev_list);
>> +    INIT_WORK(&client->svc_arrive_work, qmi_tmd_svc_arrive);
>> +
>> +    ret = qmi_tmd_alloc_cdevs(client);
>> +    if (ret)
>> +        return ret;
>> +
>> +    platform_set_drvdata(pdev, client);
>> +
>> +    ret = qmi_handle_init(&client->handle,
>> +                 
>> TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN,
>> +                  &thermal_qmi_event_ops, NULL);
>> +    if (ret < 0)
>> +        return dev_err_probe(client->dev, ret, "QMI handle init
>> failed for client %#x\n",
>> +                  client->id);
>> +
>> +    ret = qmi_add_lookup(&client->handle, TMD_SERVICE_ID_V01,
>> TMD_SERVICE_VERS_V01,
>> +                 client->id);
>> +    if (ret < 0) {
>> +        qmi_handle_release(&client->handle);
>> +        return dev_err_probe(client->dev, ret, "QMI register failed
>> for client 0x%x\n",
>> +                  client->id);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int qmi_tmd_client_remove(struct platform_device *pdev)
> void + .remove_new

Ack
> 
> Konrad

-- 
// Caleb (they/them)

Re: [PATCH 3/4] thermal: qcom: add qmi-cooling driver

[Daniel Lezcano]

Hi Caleb,

On 29/09/2023 18:16, Caleb Connolly wrote:
> The Thermal Mitigation Device (TMD) service exposes various platform
> specific thermal mitigations available on remote subsystems (ie the
> modem, adsp, cdsp, and sdsp). The service is exposed via the QMI messaging
> interface, usually over the QRTR transport.
> 
> These controls affect things like the power limits of the modem power
> amplifier and cpu core, skin temperature mitigations, as well as rail
> voltage restrictions under cold conditions.
As it is a new driver, is it possible to elaborate a bit more the 
description of this driver. For instance, why there are notifications 
from the server, what is 'new server', how can happen 'del server' ? 
etc... That will help in the future to maintain the driver.


> Each remote subsystem has its own TMD instance, where each child node
> represents a single thermal control.

The QMI TMD driver is not a cooling device from a formal thermal 
framework semantic point of view. Like the cpufreq drivers are not.

However they fall under the semantic of thermal mitigation devices which 
is relatively new (eg. they can warm up).

 From a design point view, it sounds more adequate to have the same 
approach as the other passive cooling device, especially that this 
device may be used in the future by other subsystems (AFAIU).

I suggest to remove everything related to the cooling device and keep 
the driver specific bits. For instance this driver name should what it 
is, qmi-tmd and not qmi-cooling. And give simple interfaces to act on 
it. Moreover, this interface would have to sort out 'cooling' and 
'warming' effects vs the specified state.

As asked in a previous email about a performance / power QoS. IMO it 
would be more consistent to have a 'performance_cooling_device' based on 
a performance QoS as we have a cpufreq_cooling_device based on freq_qos 
and a devfreq_cooling_device based on a dev_freq_qos.

More comments below (but cooling related code skipped). Overall the 
variable names for the message are too long, even it is the sake of self 
explanatory names, that does not help actually. It would be more easy to 
read shorted variable but with a comment describing their purposes.

> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   drivers/thermal/qcom/Kconfig       |  13 +
>   drivers/thermal/qcom/Makefile      |   1 +
>   drivers/thermal/qcom/qmi-cooling.c | 520 +++++++++++++++++++++++++++++++++++++
>   drivers/thermal/qcom/qmi-cooling.h | 428 ++++++++++++++++++++++++++++++
>   4 files changed, 962 insertions(+)
> 
> diff --git a/drivers/thermal/qcom/Kconfig b/drivers/thermal/qcom/Kconfig
> index 2c7f3f9a26eb..a24c840b78b3 100644
> --- a/drivers/thermal/qcom/Kconfig
> +++ b/drivers/thermal/qcom/Kconfig
> @@ -41,3 +41,16 @@ config QCOM_LMH
>   	  input from temperature and current sensors.  On many newer Qualcomm SoCs
>   	  LMh is configured in the firmware and this feature need not be enabled.
>   	  However, on certain SoCs like sdm845 LMh has to be configured from kernel.
> +
> +config QCOM_QMI_COOLING
> +	tristate "Qualcomm QMI cooling drivers"
> +	depends on QCOM_RPROC_COMMON
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	select QCOM_QMI_HELPERS
> +	help
> +	   This enables the remote subsystem cooling devices. These cooling
> +	   devices will be used by Qualcomm chipset to place various remote
> +	   subsystem mitigations like remote processor passive mitigation,
> +	   remote subsystem voltage restriction at low temperatures etc.
> +	   The QMI cooling device will interface with remote subsystem
> +	   using Qualcomm remoteproc interface.
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index 0fa2512042e7..94dd98e89af9 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -6,3 +6,4 @@ qcom_tsens-y			+= tsens.o tsens-v2.o tsens-v1.o tsens-v0_1.o \
>   obj-$(CONFIG_QCOM_SPMI_ADC_TM5)	+= qcom-spmi-adc-tm5.o
>   obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>   obj-$(CONFIG_QCOM_LMH)		+= lmh.o
> +obj-$(CONFIG_QCOM_QMI_COOLING) += qmi-cooling.o
> diff --git a/drivers/thermal/qcom/qmi-cooling.c b/drivers/thermal/qcom/qmi-cooling.c
> new file mode 100644
> index 000000000000..c34fecd7eefa
> --- /dev/null
> +++ b/drivers/thermal/qcom/qmi-cooling.c
> @@ -0,0 +1,520 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017, The Linux Foundation
> + * Copyright (c) 2023, Linaro Limited
> + *
> + * QMI Thermal Mitigation Device (TMD) client driver.
> + * This driver provides an in-kernel client to handle hot and cold thermal
> + * mitigations for remote subsystems (modem and DSPs) running the TMD service.
> + * It doesn't implement any handling of reports from remote subsystems.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/net.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc/qcom_rproc.h>
> +#include <linux/slab.h>
> +#include <linux/soc/qcom/qmi.h>
> +#include <linux/thermal.h>
> +
> +#include "qmi-cooling.h"
> +
> +#define MODEM0_INSTANCE_ID	0x0
> +#define ADSP_INSTANCE_ID	0x1
> +#define CDSP_INSTANCE_ID	0x43
> +#define SLPI_INSTANCE_ID	0x53
> +
> +#define QMI_TMD_RESP_TIMEOUT msecs_to_jiffies(100)
> +
> +/**
> + * struct qmi_instance_id - QMI instance ID and name
> + * @id:		The QMI instance ID
> + * @name:	Friendly name for this instance
> + */
> +struct qmi_instance_id {
> +	u32 id;
> +	const char *name;
> +};
> +
> +/**
> + * struct qmi_tmd_client - TMD client state
> + * @dev:	Device associated with this client
> + * @name:	Friendly name for the remote TMD service
> + * @handle:	QMI connection handle
> + * @mutex:	Lock to synchronise QMI communication
> + * @id:		The QMI TMD service instance ID
> + * @cdev_list:	The list of cooling devices (controls) enabled for this instance
> + * @svc_arrive_work: Work item for initialising the client when the TMD service
> + *		     starts.
> + * @connection_active: Whether or not we're connected to the QMI TMD service
> + */
> +struct qmi_tmd_client {
> +	struct device *dev;
> +	const char *name;

dev, name and id are basically used to write information in the logs. 
Those logs are too numerous in this driver and most of them can go away.

The remaining places where these fields are needed can be replaced by 
local variables.

> +	struct qmi_handle handle;
> +	struct mutex mutex;
> +	u32 id;
> +	struct list_head cdev_list;
> +	struct work_struct svc_arrive_work;
> +	bool connection_active;
> +};
> +
> +/**
> + * struct qmi_tmd - A TMD cooling device
> + * @np:		OF node associated with this control
> + * @type:	The control type (exposed via sysfs)
> + * @qmi_name:	The common name of this control shared by the remote subsystem
> + * @cdev:	Thermal framework cooling device handle
> + * @cur_state:	The current cooling/warming/mitigation state
> + * @max_state:	The maximum state
> + * @client:	The TMD client instance this control is associated with
> + */
> +struct qmi_tmd {
> +	struct device_node *np;

This reference should not be needed.

> +	const char *type;

Also, that is duplicate information, it should not be needed.

> +	char qmi_name[QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1];

kstrdup on char *name;

(and this field is already in the qmi_tmd namespace so putting 
*qmi*_name is duplicate information)

> +	struct list_head node;
> +	struct thermal_cooling_device *cdev;
> +	unsigned int cur_state;
> +	unsigned int max_state;
> +	struct qmi_tmd_client *client;
> +};
> +
> +/* Notify the remote subsystem of the requested cooling state */
> +static int qmi_tmd_send_state_request(struct qmi_tmd *tmd)
> +{
> +	struct tmd_set_mitigation_level_resp_msg_v01 tmd_resp;
> +	struct tmd_set_mitigation_level_req_msg_v01 req;

The structure names are a bit long, we may want to shortened them somehow.

> +	struct qmi_tmd_client *client;
> +	struct qmi_txn txn;
> +	int ret = 0;
> +
> +	client = tmd->client;
> +
> +	if (!client->connection_active)
> +		return 0;

	return -ENOTCONN; ?

> +
> +	memset(&req, 0, sizeof(req));
> +	memset(&tmd_resp, 0, sizeof(tmd_resp));

As previously said:

	tmd_resp = { 0 };
	reg = { 0 };

at declaration time

> +	strscpy(req.mitigation_dev_id.mitigation_dev_id, tmd->qmi_name,
> +		QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);

Variables name too, nested names too long. It is difficult to read.

> +	req.mitigation_level = tmd->cur_state;

What would it be for 'warming' device ?

> +	mutex_lock(&client->mutex);
> +
> +	ret = qmi_txn_init(&client->handle, &txn,
> +			   tmd_set_mitigation_level_resp_msg_v01_ei, &tmd_resp);

Too loong :)

Having a nice description in the comment when declaring a shortened 
tmd_set_mitigation_level_resp_msg_v01_ei variable name should be more 
adequate.

> +	if (ret < 0) {
> +		dev_err(client->dev, "qmi set state %d txn init failed for %s ret %d\n",
> +			tmd->cur_state, tmd->type, ret);

No need to write an error message. Return is enough.

> +		goto qmi_send_exit;
> +	}
> +
> +	ret = qmi_send_request(&client->handle, NULL, &txn,
> +			       QMI_TMD_SET_MITIGATION_LEVEL_REQ_V01,
> +			       TMD_SET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN,
> +			       tmd_set_mitigation_level_req_msg_v01_ei, &req);
> +	if (ret < 0) {
> +		dev_err(client->dev, "qmi set state %d txn send failed for %s ret %d\n",
> +			tmd->cur_state, tmd->type, ret);

No need to write an error message

> +		qmi_txn_cancel(&txn);
> +		goto qmi_send_exit;
> +	}
> +
> +	ret = qmi_txn_wait(&txn, QMI_TMD_RESP_TIMEOUT);
> +	if (ret < 0) {
> +		dev_err(client->dev, "qmi set state %d txn wait failed for %s ret %d\n",
> +			tmd->cur_state, tmd->type, ret);

No need to write an error message

> +		goto qmi_send_exit;
> +	}
> +	ret = 0;

Usually, ret = 0 is just before the exit label

> +
> +	if (tmd_resp.resp.result != QMI_RESULT_SUCCESS_V01) {
> +		ret = tmd_resp.resp.result;

Is the result an errno ?

> +		dev_err(client->dev, "qmi set state %d NOT success for %s ret %d\n",
> +			tmd->cur_state, tmd->type, ret);

No need to write an error message

> +		goto qmi_send_exit;
> +	}
> +
> +	dev_dbg(client->dev, "Requested state %d/%d for %s\n", tmd->cur_state,
> +		tmd->max_state, tmd->type);

No need to write this debug message

> +
> +qmi_send_exit:
> +	mutex_unlock(&client->mutex);
> +	return ret;
> +}
> +

[ ... ]

> +/*
> + * Init a single TMD control by registering a cooling device for it, or
> + * synchronising state with the remote subsystem if recovering from a service
> + * restart. This is called when the TMD service starts up.
> + */
> +static int qmi_tmd_init_control(struct qmi_tmd_client *client, const char *label,
> +				u8 max_state)
> +{
> +	struct qmi_tmd *tmd = NULL;

Initialization not useful

> +	list_for_each_entry(tmd, &client->cdev_list, node)
> +		if (!strncasecmp(tmd->qmi_name, label,
> +				 QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1))
> +			goto found;

Invert the logic:

		if (strncasecmp(...))
			continue;

		if (tmd->cdev)
			return qmi_tmd_send_state_request(tmd);

		/* return 0 */
	}
> +
> +	dev_dbg(client->dev,
> +		"TMD '%s' available in firmware but not specified in DT\n",
> +		label);

No need to write this debug message

> +	return 0;
> +
> +found:
> +	tmd->max_state = max_state;
> +	/*
> +	 * If the cooling device already exists then the QMI service went away and
> +	 * came back. So just make sure the current cooling device state is
> +	 * reflected on the remote side and then return.
> +	 */
> +	if (tmd->cdev)
> +		return qmi_tmd_send_state_request(tmd);
> +
> +	return qmi_register_cooling_device(tmd);
> +}
> +
> +/*
> + * When the QMI service starts up on a remote subsystem this function will fetch
> + * the list of TMDs on the subsystem, match it to the TMDs specified in devicetree
> + * and call qmi_tmd_init_control() for each
> + */
> +static void qmi_tmd_svc_arrive(struct work_struct *work)
> +{
> +	struct qmi_tmd_client *client =
> +		container_of(work, struct qmi_tmd_client, svc_arrive_work);
> +
> +	struct tmd_get_mitigation_device_list_req_msg_v01 req;
> +	struct tmd_get_mitigation_device_list_resp_msg_v01 *resp;
> +	int ret = 0, i;
> +	struct qmi_txn txn;
> +
> +	memset(&req, 0, sizeof(req));
> +	/* resp struct is 1.1kB, allocate it on the heap. */
> +	resp = kzalloc(sizeof(*resp), GFP_KERNEL);
> +	if (!resp)
> +		return;
> +
> +	/* Get a list of TMDs supported by the remoteproc */
> +	mutex_lock(&client->mutex);
> +	ret = qmi_txn_init(&client->handle, &txn,
> +			   tmd_get_mitigation_device_list_resp_msg_v01_ei, resp);
> +	if (ret < 0) {
> +		dev_err(client->dev,
> +			"Transaction init error for instance_id: %#x ret %d\n",
> +			client->id, ret);

No need to write an error message, qmi_txn_init() writes one

> +		goto reg_exit;
> +	}
> +
> +	ret = qmi_send_request(&client->handle, NULL, &txn,
> +			       QMI_TMD_GET_MITIGATION_DEVICE_LIST_REQ_V01,
> +			       TMD_GET_MITIGATION_DEVICE_LIST_REQ_MSG_V01_MAX_MSG_LEN,
> +			       tmd_get_mitigation_device_list_req_msg_v01_ei, &req);
> +	if (ret < 0) {
> +		qmi_txn_cancel(&txn);
> +		goto reg_exit;
> +	}
> +
> +	ret = qmi_txn_wait(&txn, QMI_TMD_RESP_TIMEOUT);
> +	if (ret < 0) {
> +		dev_err(client->dev, "Transaction wait error for client %#x ret:%d\n",
> +			client->id, ret);

No need to write an error message

> +		goto reg_exit;
> +	}
> +	if (resp->resp.result != QMI_RESULT_SUCCESS_V01) {
> +		ret = resp->resp.result;
> +		dev_err(client->dev, "Failed to get device list for client %#x ret:%d\n",
> +			client->id, ret);

No need to write an error message

> +		goto reg_exit;
> +	}
> +	mutex_unlock(&client->mutex);
> +
> +	client->connection_active = true;
> +
> +	for (i = 0; i < resp->mitigation_device_list_len; i++) {
> +		struct tmd_mitigation_dev_list_type_v01 *device =
> +			&resp->mitigation_device_list[i];
> +
> +		ret = qmi_tmd_init_control(client,
> +					   device->mitigation_dev_id.mitigation_dev_id,
> +					   device->max_mitigation_level);
> +		if (ret)
> +			break;
> +	}
> +
> +	kfree(resp);
> +	return;
> +
> +reg_exit:
> +	mutex_unlock(&client->mutex);
> +	kfree(resp);
> +}
> +
> +static void thermal_qmi_net_reset(struct qmi_handle *qmi)
> +{
> +	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
> +	struct qmi_tmd *tmd = NULL;
> +
> +	pr_info("QMI TMD service reset %s\n", client->name);

This message is not needed

> +
> +	list_for_each_entry(tmd, &client->cdev_list, node) {
> +		qmi_tmd_send_state_request(tmd);
> +	}
> +}
> +
> +static void thermal_qmi_del_server(struct qmi_handle *qmi, struct qmi_service *service)
> +{
> +	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
> +
> +	pr_info("QMI TMD service stop %s\n", client->name);

This message is not needed

> +
> +	client->connection_active = false;
> +}
> +
> +static int thermal_qmi_new_server(struct qmi_handle *qmi, struct qmi_service *service)
> +{
> +	struct qmi_tmd_client *client = container_of(qmi, struct qmi_tmd_client, handle);
> +	struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port };
> +
> +	pr_info("QMI TMD service start %s\n", client->name);

This message is not needed

> +	mutex_lock(&client->mutex);
> +	kernel_connect(qmi->sock, (struct sockaddr *)&sq, sizeof(sq), 0);

error code ?

> +	mutex_unlock(&client->mutex);
> +	queue_work(system_highpri_wq, &client->svc_arrive_work);
> +
> +	return 0;
> +}
> +
> +static struct qmi_ops thermal_qmi_event_ops = {
> +	.new_server = thermal_qmi_new_server,
> +	.del_server = thermal_qmi_del_server,
> +	.net_reset = thermal_qmi_net_reset,
> +};
> +
> +static void qmi_tmd_cleanup(struct qmi_tmd_client *client)
> +{
> +	struct qmi_tmd *tmd, *c_next;
> +
> +	client->connection_active = false;
> +
> +	mutex_lock(&client->mutex);
> +	qmi_handle_release(&client->handle);
> +	cancel_work(&client->svc_arrive_work);
> +	list_for_each_entry_safe(tmd, c_next, &client->cdev_list, node) {
> +		if (tmd->cdev)
> +			thermal_cooling_device_unregister(tmd->cdev);
> +
> +		list_del(&tmd->node);
> +	}
> +	mutex_unlock(&client->mutex);
> +}
> +
> +/* Parse the controls and allocate a qmi_tmd for each of them */
> +static int qmi_tmd_alloc_cdevs(struct qmi_tmd_client *client)
> +{
> +	struct device *dev = client->dev;
> +	struct qmi_tmd *tmd;
> +	struct device_node *subnode, *node = dev->of_node;
> +	int ret;
> +
> +	for_each_available_child_of_node(node, subnode) {
> +		const char *name;
> +
> +		tmd = devm_kzalloc(dev, sizeof(*tmd), GFP_KERNEL);
> +		if (!tmd)
> +			return dev_err_probe(client->dev, -ENOMEM,
> +					     "Couldn't allocate tmd\n");

No error display with memory allocation

> +
> +		tmd->type = devm_kasprintf(client->dev, GFP_KERNEL, "%s:%s",
> +						client->name, subnode->name);
> +		if (!tmd->type)
> +			return dev_err_probe(dev, -ENOMEM, "Cooling device name\n");
> +
> +		if (of_property_read_string(subnode, "label", &name)) {
> +			return dev_err_probe(client->dev, -EINVAL,
> +					     "Fail to parse dev name for %s\n",
> +					     subnode->name);
> +		}
> +
> +		ret = strscpy(tmd->qmi_name, name,
> +			      QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1);
> +		if (ret == -E2BIG) {
> +			return dev_err_probe(dev, -EINVAL, "TMD label %s is too long\n",
> +					     name);
> +		}
> +
> +		tmd->client = client;
> +		tmd->np = subnode;
> +		tmd->cur_state = 0;
> +		list_add(&tmd->node, &client->cdev_list);
> +	}
> +
> +	if (list_empty(&client->cdev_list))
> +		return dev_err_probe(client->dev, -EINVAL,
> +				     "No cooling devices specified for client %s (%#x)\n",
> +				     client->name, client->id);

Stricto sensu, they are not cooling devices. They are mitigation devices.

> +
> +	return 0;
> +}
> +
> +static int qmi_tmd_client_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	struct qmi_tmd_client *client;
> +	const struct qmi_instance_id *match;
> +	int ret;
> +
> +	dev = &pdev->dev;
> +
> +	client = devm_kzalloc(dev, sizeof(*client), GFP_KERNEL);
> +	if (!client)
> +		return -ENOMEM;
> +
> +	client->dev = dev;
> +	match = of_device_get_match_data(dev);
> +	if (!match)
> +		return dev_err_probe(dev, -EINVAL, "No match data\n");
> +
> +	client->id = match->id;
> +	client->name = match->name;

See the comment at the beginning of the code about the client structure 
definition.
  +
> +	mutex_init(&client->mutex);
> +	INIT_LIST_HEAD(&client->cdev_list);
> +	INIT_WORK(&client->svc_arrive_work, qmi_tmd_svc_arrive);
> +
> +	ret = qmi_tmd_alloc_cdevs(client);
> +	if (ret)
> +		return ret;

It is cumbersome to assign client->dev and then reuse it as a devm 
resource in the qmi_tmd_alloc_cdevs. I suggest to pass 'dev' to the 
function and rename it to dev_qmi_tmd_alloc(dev, ...), so it is clear 
for the reader the function is supposed to rely on the devm_ cleanup 
mechanism for the client.

> +
> +	platform_set_drvdata(pdev, client);
> +
> +	ret = qmi_handle_init(&client->handle,
> +			      TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN,
> +			      &thermal_qmi_event_ops, NULL);
> +	if (ret < 0)
> +		return dev_err_probe(client->dev, ret, "QMI handle init failed for client %#x\n",
> +			      client->id);

What are those notifications?

> +	ret = qmi_add_lookup(&client->handle, TMD_SERVICE_ID_V01, TMD_SERVICE_VERS_V01,
> +			     client->id);
> +	if (ret < 0) {
> +		qmi_handle_release(&client->handle);
> +		return dev_err_probe(client->dev, ret, "QMI register failed for client 0x%x\n",
> +			      client->id);
> +	}
> +
> +	return 0;
> +}
> +
> +static int qmi_tmd_client_remove(struct platform_device *pdev)
> +{
> +	struct qmi_tmd_client *client = platform_get_drvdata(pdev);
> +
> +	qmi_tmd_cleanup(client);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id qmi_tmd_device_table[] = {
> +	{
> +		.compatible = "qcom,qmi-cooling-modem",
> +		.data = &((struct qmi_instance_id) { MODEM0_INSTANCE_ID, "modem" }),
> +	}, {
> +		.compatible = "qcom,qmi-cooling-adsp",
> +		.data = &((struct qmi_instance_id) { ADSP_INSTANCE_ID, "adsp" }),
> +	}, {
> +		.compatible = "qcom,qmi-cooling-cdsp",
> +		.data = &((struct qmi_instance_id) { CDSP_INSTANCE_ID, "cdsp" }),
> +	}, {
> +		.compatible = "qcom,qmi-cooling-slpi",
> +		.data = &((struct qmi_instance_id) { SLPI_INSTANCE_ID, "slpi" }),
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qmi_tmd_device_table);
> +
> +static struct platform_driver qmi_tmd_device_driver = {
> +	.probe          = qmi_tmd_client_probe,
> +	.remove         = qmi_tmd_client_remove,
> +	.driver         = {
> +		.name   = "qcom-qmi-cooling",
> +		.of_match_table = qmi_tmd_device_table,
> +	},
> +};
> +
> +module_platform_driver(qmi_tmd_device_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Qualcomm QMI Thermal Mitigation Device driver");
> diff --git a/drivers/thermal/qcom/qmi-cooling.h b/drivers/thermal/qcom/qmi-cooling.h
> new file mode 100644
> index 000000000000..f46b827b4ce6
> --- /dev/null
> +++ b/drivers/thermal/qcom/qmi-cooling.h
> @@ -0,0 +1,428 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2017, The Linux Foundation
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef __QCOM_COOLING_H__
> +#define __QCOM_COOLING_H__
> +
> +#include <linux/soc/qcom/qmi.h>
> +
> +#define TMD_SERVICE_ID_V01 0x18
> +#define TMD_SERVICE_VERS_V01 0x01
> +
> +#define QMI_TMD_GET_MITIGATION_DEVICE_LIST_RESP_V01 0x0020
> +#define QMI_TMD_GET_MITIGATION_LEVEL_REQ_V01 0x0022
> +#define QMI_TMD_GET_SUPPORTED_MSGS_REQ_V01 0x001E
> +#define QMI_TMD_SET_MITIGATION_LEVEL_REQ_V01 0x0021
> +#define QMI_TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_V01 0x0023
> +#define QMI_TMD_GET_SUPPORTED_MSGS_RESP_V01 0x001E
> +#define QMI_TMD_SET_MITIGATION_LEVEL_RESP_V01 0x0021
> +#define QMI_TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_V01 0x0024
> +#define QMI_TMD_MITIGATION_LEVEL_REPORT_IND_V01 0x0025
> +#define QMI_TMD_GET_MITIGATION_LEVEL_RESP_V01 0x0022
> +#define QMI_TMD_GET_SUPPORTED_FIELDS_REQ_V01 0x001F
> +#define QMI_TMD_GET_MITIGATION_DEVICE_LIST_REQ_V01 0x0020
> +#define QMI_TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_V01 0x0023
> +#define QMI_TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_V01 0x0024
> +#define QMI_TMD_GET_SUPPORTED_FIELDS_RESP_V01 0x001F

What is the purpose of this header ?

The content is not shared and apparently lot is unused in the .c code.

> +#define QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 32
> +#define QMI_TMD_MITIGATION_DEV_LIST_MAX_V01 32
> +
> +struct tmd_mitigation_dev_id_type_v01 {
> +	char mitigation_dev_id[QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1];
> +};
> +
> +static const struct qmi_elem_info tmd_mitigation_dev_id_type_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRING,
> +		.elem_len = QMI_TMD_MITIGATION_DEV_ID_LENGTH_MAX_V01 + 1,
> +		.elem_size = sizeof(char),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0,
> +		.offset = offsetof(struct tmd_mitigation_dev_id_type_v01,
> +				   mitigation_dev_id),
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_mitigation_dev_list_type_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_dev_id;
> +	uint8_t max_mitigation_level;
> +};
> +
> +static const struct qmi_elem_info tmd_mitigation_dev_list_type_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0,
> +		.offset = offsetof(struct tmd_mitigation_dev_list_type_v01,
> +				   mitigation_dev_id),
> +		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0,
> +		.offset = offsetof(struct tmd_mitigation_dev_list_type_v01,
> +				   max_mitigation_level),
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_get_mitigation_device_list_req_msg_v01 {
> +	char placeholder;
> +};
> +
> +#define TMD_GET_MITIGATION_DEVICE_LIST_REQ_MSG_V01_MAX_MSG_LEN 0
> +const struct qmi_elem_info tmd_get_mitigation_device_list_req_msg_v01_ei[] = {
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_get_mitigation_device_list_resp_msg_v01 {
> +	struct qmi_response_type_v01 resp;
> +	uint8_t mitigation_device_list_valid;
> +	uint32_t mitigation_device_list_len;
> +	struct tmd_mitigation_dev_list_type_v01
> +		mitigation_device_list[QMI_TMD_MITIGATION_DEV_LIST_MAX_V01];
> +};
> +
> +#define TMD_GET_MITIGATION_DEVICE_LIST_RESP_MSG_V01_MAX_MSG_LEN 1099
> +static const struct qmi_elem_info tmd_get_mitigation_device_list_resp_msg_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct qmi_response_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
> +				   resp),
> +		.ei_array = qmi_response_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_OPT_FLAG,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x10,
> +		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
> +				   mitigation_device_list_valid),
> +	},
> +	{
> +		.data_type = QMI_DATA_LEN,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x10,
> +		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
> +				   mitigation_device_list_len),
> +	},
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = QMI_TMD_MITIGATION_DEV_LIST_MAX_V01,
> +		.elem_size = sizeof(struct tmd_mitigation_dev_list_type_v01),
> +		.array_type = VAR_LEN_ARRAY,
> +		.tlv_type = 0x10,
> +		.offset = offsetof(struct tmd_get_mitigation_device_list_resp_msg_v01,
> +				   mitigation_device_list),
> +		.ei_array = tmd_mitigation_dev_list_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_set_mitigation_level_req_msg_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_dev_id;
> +	uint8_t mitigation_level;
> +};
> +
> +#define TMD_SET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 40
> +static const struct qmi_elem_info tmd_set_mitigation_level_req_msg_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x01,
> +		.offset = offsetof(struct tmd_set_mitigation_level_req_msg_v01,
> +				   mitigation_dev_id),
> +		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct tmd_set_mitigation_level_req_msg_v01,
> +				   mitigation_level),
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_set_mitigation_level_resp_msg_v01 {
> +	struct qmi_response_type_v01 resp;
> +};
> +
> +#define TMD_SET_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
> +static const struct qmi_elem_info tmd_set_mitigation_level_resp_msg_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct qmi_response_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct tmd_set_mitigation_level_resp_msg_v01, resp),
> +		.ei_array = qmi_response_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_get_mitigation_level_req_msg_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
> +};
> +#define TMD_GET_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
> +
> +static const struct qmi_elem_info tmd_get_mitigation_level_req_msg_v01_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x01,
> +		.offset = offsetof(struct tmd_get_mitigation_level_req_msg_v01,
> +				   mitigation_device),
> +		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_get_mitigation_level_resp_msg_v01 {
> +	struct qmi_response_type_v01 resp;
> +	uint8_t current_mitigation_level_valid;
> +	uint8_t current_mitigation_level;
> +	uint8_t requested_mitigation_level_valid;
> +	uint8_t requested_mitigation_level;
> +};
> +
> +#define TMD_GET_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 15
> +static const struct qmi_elem_info tmd_get_mitigation_level_resp_msg_ei[] = {
> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct qmi_response_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01, resp),
> +		.ei_array = qmi_response_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_OPT_FLAG,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x10,
> +		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
> +				   current_mitigation_level_valid),
> +	},
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x10,
> +		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
> +				   current_mitigation_level),
> +	},
> +	{
> +		.data_type = QMI_OPT_FLAG,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x11,
> +		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
> +				   requested_mitigation_level_valid),
> +	},
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x11,
> +		.offset = offsetof(struct tmd_get_mitigation_level_resp_msg_v01,
> +				   requested_mitigation_level),
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +struct tmd_register_notification_mitigation_level_req_msg_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
> +};
> +
> +#define TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
> +static const struct qmi_elem_info
> +	tmd_register_notification_mitigation_level_req_msg_v01_ei[] = {
> +		{
> +			.data_type = QMI_STRUCT,
> +			.elem_len = 1,
> +			.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +			.array_type = NO_ARRAY,
> +			.tlv_type = 0x01,
> +			.offset = offsetof(
> +				struct tmd_register_notification_mitigation_level_req_msg_v01,
> +				mitigation_device),
> +			.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +		},
> +		{
> +			.data_type = QMI_EOTI,
> +			.array_type = NO_ARRAY,
> +			.tlv_type = QMI_COMMON_TLV_TYPE,
> +		},
> +	};
> +
> +struct tmd_register_notification_mitigation_level_resp_msg_v01 {
> +	struct qmi_response_type_v01 resp;
> +};
> +
> +#define TMD_REGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
> +static const struct qmi_elem_info
> +	tmd_register_notification_mitigation_level_resp_msg_v01_ei[] = {
> +		{
> +			.data_type = QMI_STRUCT,
> +			.elem_len = 1,
> +			.elem_size = sizeof(struct qmi_response_type_v01),
> +			.array_type = NO_ARRAY,
> +			.tlv_type = 0x02,
> +			.offset = offsetof(
> +				struct tmd_register_notification_mitigation_level_resp_msg_v01,
> +				resp),
> +			.ei_array = qmi_response_type_v01_ei,
> +		},
> +		{
> +			.data_type = QMI_EOTI,
> +			.array_type = NO_ARRAY,
> +			.tlv_type = QMI_COMMON_TLV_TYPE,
> +		},
> +	};
> +
> +struct tmd_deregister_notification_mitigation_level_req_msg_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
> +};
> +
> +#define TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_REQ_MSG_V01_MAX_MSG_LEN 36
> +static const struct qmi_elem_info
> +	tmd_deregister_notification_mitigation_level_req_msg_v01_ei[] = {

Not used

> +		{
> +			.data_type = QMI_STRUCT,
> +			.elem_len = 1,
> +			.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +			.array_type = NO_ARRAY,
> +			.tlv_type = 0x01,
> +			.offset = offsetof(
> +				struct tmd_deregister_notification_mitigation_level_req_msg_v01,
> +				mitigation_device),
> +			.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +		},
> +		{
> +			.data_type = QMI_EOTI,
> +			.array_type = NO_ARRAY,
> +			.tlv_type = QMI_COMMON_TLV_TYPE,
> +		},
> +	};
> +
> +struct tmd_deregister_notification_mitigation_level_resp_msg_v01 {
> +	struct qmi_response_type_v01 resp;
> +};
> +
> +#define TMD_DEREGISTER_NOTIFICATION_MITIGATION_LEVEL_RESP_MSG_V01_MAX_MSG_LEN 7
> +static const struct qmi_elem_info
> +	tmd_deregister_notification_mitigation_level_resp_msg_v01_ei[] = {

Not used

> +		{
> +			.data_type = QMI_STRUCT,
> +			.elem_len = 1,
> +			.elem_size = sizeof(struct qmi_response_type_v01),
> +			.array_type = NO_ARRAY,
> +			.tlv_type = 0x02,
> +			.offset = offsetof(
> +				struct tmd_deregister_notification_mitigation_level_resp_msg_v01,
> +				resp),
> +			.ei_array = qmi_response_type_v01_ei,
> +		},
> +		{
> +			.data_type = QMI_EOTI,
> +			.array_type = NO_ARRAY,
> +			.tlv_type = QMI_COMMON_TLV_TYPE,
> +		},
> +	};
> +
> +struct tmd_mitigation_level_report_ind_msg_v01 {
> +	struct tmd_mitigation_dev_id_type_v01 mitigation_device;
> +	uint8_t current_mitigation_level;
> +};

Not used the structure below is not used too.

> +
> +#define TMD_MITIGATION_LEVEL_REPORT_IND_MSG_V01_MAX_MSG_LEN 40

Where is it used?

> +static const struct qmi_elem_info tmd_mitigation_level_report_ind_msg_v01_ei[] = {

I don't see this variable used.

> +	{
> +		.data_type = QMI_STRUCT,
> +		.elem_len = 1,
> +		.elem_size = sizeof(struct tmd_mitigation_dev_id_type_v01),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x01,
> +		.offset = offsetof(struct tmd_mitigation_level_report_ind_msg_v01,
> +				   mitigation_device),
> +		.ei_array = tmd_mitigation_dev_id_type_v01_ei,
> +	},
> +	{
> +		.data_type = QMI_UNSIGNED_1_BYTE,
> +		.elem_len = 1,
> +		.elem_size = sizeof(uint8_t),
> +		.array_type = NO_ARRAY,
> +		.tlv_type = 0x02,
> +		.offset = offsetof(struct tmd_mitigation_level_report_ind_msg_v01,
> +				   current_mitigation_level),
> +	},
> +	{
> +		.data_type = QMI_EOTI,
> +		.array_type = NO_ARRAY,
> +		.tlv_type = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +#endif /* __QMI_COOLING_INTERNAL_H__ */
> 

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

[Caleb Connolly]

Add myself as the maintainer for the Qualcomm Cooling
driver.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 01dde0acebd3..d92cfa1dd57f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17800,6 +17800,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
 F:	drivers/mtd/nand/raw/qcom_nandc.c
 
+QUALCOMM QMI (THERMAL MITIGATION DEVICE) COOLING DRIVER
+M:	Caleb Connolly <caleb.connolly@linaro.org>
+L:	linux-pm@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/thermal/qcom,qmi-cooling.yaml
+F:	drivers/thermal/qcom/qmi-cooling.*
+
 QUALCOMM QSEECOM DRIVER
 M:	Maximilian Luz <luzmaximilian@gmail.com>
 L:	linux-arm-msm@vger.kernel.org

-- 
2.42.0

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Konrad Dybcio]

On 29.09.2023 18:16, Caleb Connolly wrote:
> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> It exposes various mitigations including passive thermal controls and
> rail voltage restrictions.
> 
> This series introduces support for exposing TMDs as cooling devices
> in the kernel through the thermal framework, using the QMI interface.
> 
> Each TMD client is described as a child of the remoteproc node in
> devicetree. With subnodes for each control.
> 
> This series is based on previous work by Bhupesh Sharma which can be
> found at [1]. I'm sending this as a fresh series as it has been a
> year since the original version and I have rewritten most of the driver.
> 
Since you're adding support for funky hw, it would be appreciated
if you also linked to a tree that has the dt bits hooked up, the
schema example may not tell the whole story

Konrad

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Caleb Connolly]

On 29/09/2023 18:17, Konrad Dybcio wrote:
> On 29.09.2023 18:16, Caleb Connolly wrote:
>> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
>> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
>> It exposes various mitigations including passive thermal controls and
>> rail voltage restrictions.
>>
>> This series introduces support for exposing TMDs as cooling devices
>> in the kernel through the thermal framework, using the QMI interface.
>>
>> Each TMD client is described as a child of the remoteproc node in
>> devicetree. With subnodes for each control.
>>
>> This series is based on previous work by Bhupesh Sharma which can be
>> found at [1]. I'm sending this as a fresh series as it has been a
>> year since the original version and I have rewritten most of the driver.
>>
> Since you're adding support for funky hw, it would be appreciated
> if you also linked to a tree that has the dt bits hooked up, the
> schema example may not tell the whole story

For sure, you can find a patch with a reference DTS part here

https://git.codelinaro.org/caleb_connolly/kernel/-/commit/9067f80db58bbce81d5f0703aa2fd261e88bc812
> 
> Konrad

-- 
// Caleb (they/them)

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Manivannan Sadhasivam]

On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> It exposes various mitigations including passive thermal controls and
> rail voltage restrictions.
> 
> This series introduces support for exposing TMDs as cooling devices
> in the kernel through the thermal framework, using the QMI interface.
> 
> Each TMD client is described as a child of the remoteproc node in
> devicetree. With subnodes for each control.
> 

Daniel expressed concerns in the past aganist representing TMD driver as a
cooling device since it is not tied to thermal zones and the governors cannot
use it. Instead he suggested to represent it as a powercap device with thermal
constraints.

So please look into that approach.

- Mani

> This series is based on previous work by Bhupesh Sharma which can be
> found at [1]. I'm sending this as a fresh series as it has been a
> year since the original version and I have rewritten most of the driver.
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> 
> ---
> Caleb Connolly (4):
>       remoteproc: qcom: probe all child devices
>       dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
>       thermal: qcom: add qmi-cooling driver
>       MAINTAINERS: Add entry for Qualcomm Cooling Driver
> 
>  .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
>  .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
>  .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
>  MAINTAINERS                                        |   8 +
>  drivers/remoteproc/qcom_q6v5.c                     |   4 +
>  drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
>  drivers/thermal/qcom/Kconfig                       |  13 +
>  drivers/thermal/qcom/Makefile                      |   1 +
>  drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
>  drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
>  10 files changed, 1161 insertions(+), 8 deletions(-)
> ---
> base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> 
> // Caleb (they/them)
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Caleb Connolly]

On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
>> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
>> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
>> It exposes various mitigations including passive thermal controls and
>> rail voltage restrictions.
>>
>> This series introduces support for exposing TMDs as cooling devices
>> in the kernel through the thermal framework, using the QMI interface.
>>
>> Each TMD client is described as a child of the remoteproc node in
>> devicetree. With subnodes for each control.
>>
> 
> Daniel expressed concerns in the past aganist representing TMD driver as a
> cooling device since it is not tied to thermal zones and the governors cannot
> use it. Instead he suggested to represent it as a powercap device with thermal
> constraints.

Hi Mani,

Forgive me as I'm not yet super familiar with the thermal subsystem.

As I understand it, the DT layout here enables each control to be 
referenced under the thermal zones, at least this is the approach taken 
in CAF 4.9.

Maybe I don't quite understand what you mean, are you saying that using 
thermal zones is the wrong approach?
> 
> So please look into that approach.

Any recommended reading? Or drivers I can use as a reference?

Thanks
> 
> - Mani
> 
>> This series is based on previous work by Bhupesh Sharma which can be
>> found at [1]. I'm sending this as a fresh series as it has been a
>> year since the original version and I have rewritten most of the driver.
>>
>> [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
>>
>> ---
>> Caleb Connolly (4):
>>        remoteproc: qcom: probe all child devices
>>        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
>>        thermal: qcom: add qmi-cooling driver
>>        MAINTAINERS: Add entry for Qualcomm Cooling Driver
>>
>>   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
>>   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
>>   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
>>   MAINTAINERS                                        |   8 +
>>   drivers/remoteproc/qcom_q6v5.c                     |   4 +
>>   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
>>   drivers/thermal/qcom/Kconfig                       |  13 +
>>   drivers/thermal/qcom/Makefile                      |   1 +
>>   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
>>   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
>>   10 files changed, 1161 insertions(+), 8 deletions(-)
>> ---
>> base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
>>
>> // Caleb (they/them)
>>
> 

-- 
// Caleb (they/them)

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Manivannan Sadhasivam]

On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> 
> 
> On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > It exposes various mitigations including passive thermal controls and
> > > rail voltage restrictions.
> > > 
> > > This series introduces support for exposing TMDs as cooling devices
> > > in the kernel through the thermal framework, using the QMI interface.
> > > 
> > > Each TMD client is described as a child of the remoteproc node in
> > > devicetree. With subnodes for each control.
> > > 
> > 
> > Daniel expressed concerns in the past aganist representing TMD driver as a
> > cooling device since it is not tied to thermal zones and the governors cannot
> > use it. Instead he suggested to represent it as a powercap device with thermal
> > constraints.
> 
> Hi Mani,
> 
> Forgive me as I'm not yet super familiar with the thermal subsystem.
> 
> As I understand it, the DT layout here enables each control to be referenced
> under the thermal zones, at least this is the approach taken in CAF 4.9.
> 
> Maybe I don't quite understand what you mean, are you saying that using
> thermal zones is the wrong approach?

Thermal framework expects each thermal zone represented in DT to have atleast
one corresponding thermal sensor defined using "thermal-sensors" property. But
with TMD, there is no thermal sensor AFAIK.

> > 
> > So please look into that approach.
> 
> Any recommended reading? Or drivers I can use as a reference?
> 

drivers/powercap/arm_scmi_powercap.c seems to be a good reference.

- Mani

> Thanks
> > 
> > - Mani
> > 
> > > This series is based on previous work by Bhupesh Sharma which can be
> > > found at [1]. I'm sending this as a fresh series as it has been a
> > > year since the original version and I have rewritten most of the driver.
> > > 
> > > [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> > > 
> > > ---
> > > Caleb Connolly (4):
> > >        remoteproc: qcom: probe all child devices
> > >        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
> > >        thermal: qcom: add qmi-cooling driver
> > >        MAINTAINERS: Add entry for Qualcomm Cooling Driver
> > > 
> > >   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
> > >   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
> > >   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
> > >   MAINTAINERS                                        |   8 +
> > >   drivers/remoteproc/qcom_q6v5.c                     |   4 +
> > >   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
> > >   drivers/thermal/qcom/Kconfig                       |  13 +
> > >   drivers/thermal/qcom/Makefile                      |   1 +
> > >   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
> > >   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
> > >   10 files changed, 1161 insertions(+), 8 deletions(-)
> > > ---
> > > base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> > > 
> > > // Caleb (they/them)
> > > 
> > 
> 
> -- 
> // Caleb (they/them)

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Dmitry Baryshkov]

On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> >
> >
> > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > It exposes various mitigations including passive thermal controls and
> > > > rail voltage restrictions.
> > > >
> > > > This series introduces support for exposing TMDs as cooling devices
> > > > in the kernel through the thermal framework, using the QMI interface.
> > > >
> > > > Each TMD client is described as a child of the remoteproc node in
> > > > devicetree. With subnodes for each control.
> > > >
> > >
> > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > cooling device since it is not tied to thermal zones and the governors cannot
> > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > constraints.
> >
> > Hi Mani,
> >
> > Forgive me as I'm not yet super familiar with the thermal subsystem.
> >
> > As I understand it, the DT layout here enables each control to be referenced
> > under the thermal zones, at least this is the approach taken in CAF 4.9.
> >
> > Maybe I don't quite understand what you mean, are you saying that using
> > thermal zones is the wrong approach?
>
> Thermal framework expects each thermal zone represented in DT to have atleast
> one corresponding thermal sensor defined using "thermal-sensors" property. But
> with TMD, there is no thermal sensor AFAIK.

As far as I understand, no. It is perfectly fine to have 'cooling'
devices, which react to external thermal monitoring events. I might be
mistaken, but I think that is the case here, isn't it?

>
> > >
> > > So please look into that approach.
> >
> > Any recommended reading? Or drivers I can use as a reference?
> >
>
> drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
>
> - Mani
>
> > Thanks
> > >
> > > - Mani
> > >
> > > > This series is based on previous work by Bhupesh Sharma which can be
> > > > found at [1]. I'm sending this as a fresh series as it has been a
> > > > year since the original version and I have rewritten most of the driver.
> > > >
> > > > [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> > > >
> > > > ---
> > > > Caleb Connolly (4):
> > > >        remoteproc: qcom: probe all child devices
> > > >        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
> > > >        thermal: qcom: add qmi-cooling driver
> > > >        MAINTAINERS: Add entry for Qualcomm Cooling Driver
> > > >
> > > >   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
> > > >   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
> > > >   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
> > > >   MAINTAINERS                                        |   8 +
> > > >   drivers/remoteproc/qcom_q6v5.c                     |   4 +
> > > >   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
> > > >   drivers/thermal/qcom/Kconfig                       |  13 +
> > > >   drivers/thermal/qcom/Makefile                      |   1 +
> > > >   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
> > > >   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
> > > >   10 files changed, 1161 insertions(+), 8 deletions(-)
> > > > ---
> > > > base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> > > >
> > > > // Caleb (they/them)
> > > >
> > >
> >
> > --
> > // Caleb (they/them)
>
> --
> மணிவண்ணன் சதாசிவம்



-- 
With best wishes
Dmitry

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Caleb Connolly]

On 02/10/2023 16:00, Dmitry Baryshkov wrote:
> On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
>>
>> On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
>>>
>>>
>>> On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
>>>> On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
>>>>> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
>>>>> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
>>>>> It exposes various mitigations including passive thermal controls and
>>>>> rail voltage restrictions.
>>>>>
>>>>> This series introduces support for exposing TMDs as cooling devices
>>>>> in the kernel through the thermal framework, using the QMI interface.
>>>>>
>>>>> Each TMD client is described as a child of the remoteproc node in
>>>>> devicetree. With subnodes for each control.
>>>>>
>>>>
>>>> Daniel expressed concerns in the past aganist representing TMD driver as a
>>>> cooling device since it is not tied to thermal zones and the governors cannot
>>>> use it. Instead he suggested to represent it as a powercap device with thermal
>>>> constraints.

I think so, see for example in CAF 4.9 [1]

As far as I can tell, the QMI TMD service was written specifically *for*
the cooling_device API, so it's a bit of a chicken and egg problem
trying to coerce it to some other form...

That all being said, I can definitely see the potential value in using
the powercap framework. Especially if it would let us tie the TMD
controls more directly with the remoteproc device and more easily
integrate it with DTPM.

For example, would it also make sense to adjust these values to preserve
battery life? Which API makes the most sense for that?

[1]:
https://github.com/android-linux-stable/msm-4.9/blob/384eee701481c3dd08822819cf4d5f1da5729db1/arch/arm64/boot/dts/qcom/sdm670-thermal.dtsi#L661
>>>
>>> Hi Mani,
>>>
>>> Forgive me as I'm not yet super familiar with the thermal subsystem.
>>>
>>> As I understand it, the DT layout here enables each control to be referenced
>>> under the thermal zones, at least this is the approach taken in CAF 4.9.
>>>
>>> Maybe I don't quite understand what you mean, are you saying that using
>>> thermal zones is the wrong approach?
>>
>> Thermal framework expects each thermal zone represented in DT to have atleast
>> one corresponding thermal sensor defined using "thermal-sensors" property. But
>> with TMD, there is no thermal sensor AFAIK.
> 
> As far as I understand, no. It is perfectly fine to have 'cooling'
> devices, which react to external thermal monitoring events. I might be
> mistaken, but I think that is the case here, isn't it?
> 
>>
>>>>
>>>> So please look into that approach.
>>>
>>> Any recommended reading? Or drivers I can use as a reference?
>>>
>>
>> drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
>>
>> - Mani
>>
>>> Thanks
>>>>
>>>> - Mani
>>>>
>>>>> This series is based on previous work by Bhupesh Sharma which can be
>>>>> found at [1]. I'm sending this as a fresh series as it has been a
>>>>> year since the original version and I have rewritten most of the driver.
>>>>>
>>>>> [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
>>>>>
>>>>> ---
>>>>> Caleb Connolly (4):
>>>>>        remoteproc: qcom: probe all child devices
>>>>>        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
>>>>>        thermal: qcom: add qmi-cooling driver
>>>>>        MAINTAINERS: Add entry for Qualcomm Cooling Driver
>>>>>
>>>>>   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
>>>>>   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
>>>>>   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
>>>>>   MAINTAINERS                                        |   8 +
>>>>>   drivers/remoteproc/qcom_q6v5.c                     |   4 +
>>>>>   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
>>>>>   drivers/thermal/qcom/Kconfig                       |  13 +
>>>>>   drivers/thermal/qcom/Makefile                      |   1 +
>>>>>   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
>>>>>   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
>>>>>   10 files changed, 1161 insertions(+), 8 deletions(-)
>>>>> ---
>>>>> base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
>>>>>
>>>>> // Caleb (they/them)
>>>>>
>>>>
>>>
>>> --
>>> // Caleb (they/them)
>>
>> --
>> மணிவண்ணன் சதாசிவம்
> 
> 
> 

-- 
// Caleb (they/them)

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Manivannan Sadhasivam]

On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
> On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> > >
> > >
> > > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > > It exposes various mitigations including passive thermal controls and
> > > > > rail voltage restrictions.
> > > > >
> > > > > This series introduces support for exposing TMDs as cooling devices
> > > > > in the kernel through the thermal framework, using the QMI interface.
> > > > >
> > > > > Each TMD client is described as a child of the remoteproc node in
> > > > > devicetree. With subnodes for each control.
> > > > >
> > > >
> > > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > > cooling device since it is not tied to thermal zones and the governors cannot
> > > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > > constraints.
> > >
> > > Hi Mani,
> > >
> > > Forgive me as I'm not yet super familiar with the thermal subsystem.
> > >
> > > As I understand it, the DT layout here enables each control to be referenced
> > > under the thermal zones, at least this is the approach taken in CAF 4.9.
> > >
> > > Maybe I don't quite understand what you mean, are you saying that using
> > > thermal zones is the wrong approach?
> >
> > Thermal framework expects each thermal zone represented in DT to have atleast
> > one corresponding thermal sensor defined using "thermal-sensors" property. But
> > with TMD, there is no thermal sensor AFAIK.
> 
> As far as I understand, no. It is perfectly fine to have 'cooling'
> devices, which react to external thermal monitoring events. I might be
> mistaken, but I think that is the case here, isn't it?
> 

Yes it is represented as cooling device(s). But I do not see any cognizant way
to plug it with thermal zones i.e., unless TMD itself reports temperature of the
modem, using it as a cooling device for external temperature events doesn't
sound good to me.

- Mani

> >
> > > >
> > > > So please look into that approach.
> > >
> > > Any recommended reading? Or drivers I can use as a reference?
> > >
> >
> > drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
> >
> > - Mani
> >
> > > Thanks
> > > >
> > > > - Mani
> > > >
> > > > > This series is based on previous work by Bhupesh Sharma which can be
> > > > > found at [1]. I'm sending this as a fresh series as it has been a
> > > > > year since the original version and I have rewritten most of the driver.
> > > > >
> > > > > [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> > > > >
> > > > > ---
> > > > > Caleb Connolly (4):
> > > > >        remoteproc: qcom: probe all child devices
> > > > >        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
> > > > >        thermal: qcom: add qmi-cooling driver
> > > > >        MAINTAINERS: Add entry for Qualcomm Cooling Driver
> > > > >
> > > > >   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
> > > > >   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
> > > > >   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
> > > > >   MAINTAINERS                                        |   8 +
> > > > >   drivers/remoteproc/qcom_q6v5.c                     |   4 +
> > > > >   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
> > > > >   drivers/thermal/qcom/Kconfig                       |  13 +
> > > > >   drivers/thermal/qcom/Makefile                      |   1 +
> > > > >   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
> > > > >   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
> > > > >   10 files changed, 1161 insertions(+), 8 deletions(-)
> > > > > ---
> > > > > base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> > > > >
> > > > > // Caleb (they/them)
> > > > >
> > > >
> > >
> > > --
> > > // Caleb (they/them)
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> 
> 
> 
> -- 
> With best wishes
> Dmitry

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Dmitry Baryshkov]

On Mon, 2 Oct 2023 at 18:58, Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> > > >
> > > >
> > > > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > > > It exposes various mitigations including passive thermal controls and
> > > > > > rail voltage restrictions.
> > > > > >
> > > > > > This series introduces support for exposing TMDs as cooling devices
> > > > > > in the kernel through the thermal framework, using the QMI interface.
> > > > > >
> > > > > > Each TMD client is described as a child of the remoteproc node in
> > > > > > devicetree. With subnodes for each control.
> > > > > >
> > > > >
> > > > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > > > cooling device since it is not tied to thermal zones and the governors cannot
> > > > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > > > constraints.
> > > >
> > > > Hi Mani,
> > > >
> > > > Forgive me as I'm not yet super familiar with the thermal subsystem.
> > > >
> > > > As I understand it, the DT layout here enables each control to be referenced
> > > > under the thermal zones, at least this is the approach taken in CAF 4.9.
> > > >
> > > > Maybe I don't quite understand what you mean, are you saying that using
> > > > thermal zones is the wrong approach?
> > >
> > > Thermal framework expects each thermal zone represented in DT to have atleast
> > > one corresponding thermal sensor defined using "thermal-sensors" property. But
> > > with TMD, there is no thermal sensor AFAIK.
> >
> > As far as I understand, no. It is perfectly fine to have 'cooling'
> > devices, which react to external thermal monitoring events. I might be
> > mistaken, but I think that is the case here, isn't it?
> >
>
> Yes it is represented as cooling device(s). But I do not see any cognizant way
> to plug it with thermal zones i.e., unless TMD itself reports temperature of the
> modem, using it as a cooling device for external temperature events doesn't
> sound good to me.

Why? We have compute, q6, wlan tsens sensors. So it seems natural to
tell CDSP to slow down if compute sensor reports overheating.

>
> - Mani
>
> > >
> > > > >
> > > > > So please look into that approach.
> > > >
> > > > Any recommended reading? Or drivers I can use as a reference?
> > > >
> > >
> > > drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
> > >
> > > - Mani
> > >
> > > > Thanks
> > > > >
> > > > > - Mani
> > > > >
> > > > > > This series is based on previous work by Bhupesh Sharma which can be
> > > > > > found at [1]. I'm sending this as a fresh series as it has been a
> > > > > > year since the original version and I have rewritten most of the driver.
> > > > > >
> > > > > > [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> > > > > >
> > > > > > ---
> > > > > > Caleb Connolly (4):
> > > > > >        remoteproc: qcom: probe all child devices
> > > > > >        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
> > > > > >        thermal: qcom: add qmi-cooling driver
> > > > > >        MAINTAINERS: Add entry for Qualcomm Cooling Driver
> > > > > >
> > > > > >   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
> > > > > >   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
> > > > > >   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
> > > > > >   MAINTAINERS                                        |   8 +
> > > > > >   drivers/remoteproc/qcom_q6v5.c                     |   4 +
> > > > > >   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
> > > > > >   drivers/thermal/qcom/Kconfig                       |  13 +
> > > > > >   drivers/thermal/qcom/Makefile                      |   1 +
> > > > > >   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
> > > > > >   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
> > > > > >   10 files changed, 1161 insertions(+), 8 deletions(-)
> > > > > > ---
> > > > > > base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> > > > > >
> > > > > > // Caleb (they/them)
> > > > > >
> > > > >
> > > >
> > > > --
> > > > // Caleb (they/them)
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
> >
> >
> >
> > --
> > With best wishes
> > Dmitry
>
> --
> மணிவண்ணன் சதாசிவம்



-- 
With best wishes
Dmitry

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Manivannan Sadhasivam]

On Mon, Oct 02, 2023 at 07:00:27PM +0300, Dmitry Baryshkov wrote:
> On Mon, 2 Oct 2023 at 18:58, Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
> > > On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > >
> > > > On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> > > > >
> > > > >
> > > > > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > > > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > > > > It exposes various mitigations including passive thermal controls and
> > > > > > > rail voltage restrictions.
> > > > > > >
> > > > > > > This series introduces support for exposing TMDs as cooling devices
> > > > > > > in the kernel through the thermal framework, using the QMI interface.
> > > > > > >
> > > > > > > Each TMD client is described as a child of the remoteproc node in
> > > > > > > devicetree. With subnodes for each control.
> > > > > > >
> > > > > >
> > > > > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > > > > cooling device since it is not tied to thermal zones and the governors cannot
> > > > > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > > > > constraints.
> > > > >
> > > > > Hi Mani,
> > > > >
> > > > > Forgive me as I'm not yet super familiar with the thermal subsystem.
> > > > >
> > > > > As I understand it, the DT layout here enables each control to be referenced
> > > > > under the thermal zones, at least this is the approach taken in CAF 4.9.
> > > > >
> > > > > Maybe I don't quite understand what you mean, are you saying that using
> > > > > thermal zones is the wrong approach?
> > > >
> > > > Thermal framework expects each thermal zone represented in DT to have atleast
> > > > one corresponding thermal sensor defined using "thermal-sensors" property. But
> > > > with TMD, there is no thermal sensor AFAIK.
> > >
> > > As far as I understand, no. It is perfectly fine to have 'cooling'
> > > devices, which react to external thermal monitoring events. I might be
> > > mistaken, but I think that is the case here, isn't it?
> > >
> >
> > Yes it is represented as cooling device(s). But I do not see any cognizant way
> > to plug it with thermal zones i.e., unless TMD itself reports temperature of the
> > modem, using it as a cooling device for external temperature events doesn't
> > sound good to me.
> 
> Why? We have compute, q6, wlan tsens sensors. So it seems natural to
> tell CDSP to slow down if compute sensor reports overheating.
> 

TMD is for external devices such as PCIe modems as well. Is there a temperature
sensor for that?

- Mani

> >
> > - Mani
> >
> > > >
> > > > > >
> > > > > > So please look into that approach.
> > > > >
> > > > > Any recommended reading? Or drivers I can use as a reference?
> > > > >
> > > >
> > > > drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
> > > >
> > > > - Mani
> > > >
> > > > > Thanks
> > > > > >
> > > > > > - Mani
> > > > > >
> > > > > > > This series is based on previous work by Bhupesh Sharma which can be
> > > > > > > found at [1]. I'm sending this as a fresh series as it has been a
> > > > > > > year since the original version and I have rewritten most of the driver.
> > > > > > >
> > > > > > > [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
> > > > > > >
> > > > > > > ---
> > > > > > > Caleb Connolly (4):
> > > > > > >        remoteproc: qcom: probe all child devices
> > > > > > >        dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
> > > > > > >        thermal: qcom: add qmi-cooling driver
> > > > > > >        MAINTAINERS: Add entry for Qualcomm Cooling Driver
> > > > > > >
> > > > > > >   .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
> > > > > > >   .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
> > > > > > >   .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
> > > > > > >   MAINTAINERS                                        |   8 +
> > > > > > >   drivers/remoteproc/qcom_q6v5.c                     |   4 +
> > > > > > >   drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
> > > > > > >   drivers/thermal/qcom/Kconfig                       |  13 +
> > > > > > >   drivers/thermal/qcom/Makefile                      |   1 +
> > > > > > >   drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
> > > > > > >   drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
> > > > > > >   10 files changed, 1161 insertions(+), 8 deletions(-)
> > > > > > > ---
> > > > > > > base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
> > > > > > >
> > > > > > > // Caleb (they/them)
> > > > > > >
> > > > > >
> > > > >
> > > > > --
> > > > > // Caleb (they/them)
> > > >
> > > > --
> > > > மணிவண்ணன் சதாசிவம்
> > >
> > >
> > >
> > > --
> > > With best wishes
> > > Dmitry
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> 
> 
> 
> -- 
> With best wishes
> Dmitry

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Neil Armstrong]

On 02/10/2023 18:13, Manivannan Sadhasivam wrote:
> On Mon, Oct 02, 2023 at 07:00:27PM +0300, Dmitry Baryshkov wrote:
>> On Mon, 2 Oct 2023 at 18:58, Manivannan Sadhasivam <mani@kernel.org> wrote:
>>>
>>> On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
>>>> On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
>>>>>
>>>>> On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
>>>>>>
>>>>>>
>>>>>> On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
>>>>>>> On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
>>>>>>>> The Thermal Mitigation Device (TMD) Service is a QMI service that runs
>>>>>>>> on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
>>>>>>>> It exposes various mitigations including passive thermal controls and
>>>>>>>> rail voltage restrictions.
>>>>>>>>
>>>>>>>> This series introduces support for exposing TMDs as cooling devices
>>>>>>>> in the kernel through the thermal framework, using the QMI interface.
>>>>>>>>
>>>>>>>> Each TMD client is described as a child of the remoteproc node in
>>>>>>>> devicetree. With subnodes for each control.
>>>>>>>>
>>>>>>>
>>>>>>> Daniel expressed concerns in the past aganist representing TMD driver as a
>>>>>>> cooling device since it is not tied to thermal zones and the governors cannot
>>>>>>> use it. Instead he suggested to represent it as a powercap device with thermal
>>>>>>> constraints.
>>>>>>
>>>>>> Hi Mani,
>>>>>>
>>>>>> Forgive me as I'm not yet super familiar with the thermal subsystem.
>>>>>>
>>>>>> As I understand it, the DT layout here enables each control to be referenced
>>>>>> under the thermal zones, at least this is the approach taken in CAF 4.9.
>>>>>>
>>>>>> Maybe I don't quite understand what you mean, are you saying that using
>>>>>> thermal zones is the wrong approach?
>>>>>
>>>>> Thermal framework expects each thermal zone represented in DT to have atleast
>>>>> one corresponding thermal sensor defined using "thermal-sensors" property. But
>>>>> with TMD, there is no thermal sensor AFAIK.
>>>>
>>>> As far as I understand, no. It is perfectly fine to have 'cooling'
>>>> devices, which react to external thermal monitoring events. I might be
>>>> mistaken, but I think that is the case here, isn't it?
>>>>
>>>
>>> Yes it is represented as cooling device(s). But I do not see any cognizant way
>>> to plug it with thermal zones i.e., unless TMD itself reports temperature of the
>>> modem, using it as a cooling device for external temperature events doesn't
>>> sound good to me.
>>
>> Why? We have compute, q6, wlan tsens sensors. So it seems natural to
>> tell CDSP to slow down if compute sensor reports overheating.
>>
> 
> TMD is for external devices such as PCIe modems as well. Is there a temperature
> sensor for that?

Is could, for example NVMe does.

But here Caleb only exposes the internal DSP over the TMD as cooling devices, so
it matches the tsens input we have.

Neil

> 
> - Mani
> 
>>>
>>> - Mani
>>>
>>>>>
>>>>>>>
>>>>>>> So please look into that approach.
>>>>>>
>>>>>> Any recommended reading? Or drivers I can use as a reference?
>>>>>>
>>>>>
>>>>> drivers/powercap/arm_scmi_powercap.c seems to be a good reference.
>>>>>
>>>>> - Mani
>>>>>
>>>>>> Thanks
>>>>>>>
>>>>>>> - Mani
>>>>>>>
>>>>>>>> This series is based on previous work by Bhupesh Sharma which can be
>>>>>>>> found at [1]. I'm sending this as a fresh series as it has been a
>>>>>>>> year since the original version and I have rewritten most of the driver.
>>>>>>>>
>>>>>>>> [1]: https://lore.kernel.org/linux-arm-msm/20220912085049.3517140-1-bhupesh.sharma@linaro.org/
>>>>>>>>
>>>>>>>> ---
>>>>>>>> Caleb Connolly (4):
>>>>>>>>         remoteproc: qcom: probe all child devices
>>>>>>>>         dt-bindings: thermal: Add qcom,qmi-cooling yaml bindings
>>>>>>>>         thermal: qcom: add qmi-cooling driver
>>>>>>>>         MAINTAINERS: Add entry for Qualcomm Cooling Driver
>>>>>>>>
>>>>>>>>    .../bindings/remoteproc/qcom,msm8996-mss-pil.yaml  |  13 +
>>>>>>>>    .../bindings/remoteproc/qcom,pas-common.yaml       |   6 +
>>>>>>>>    .../bindings/thermal/qcom,qmi-cooling.yaml         | 168 +++++++
>>>>>>>>    MAINTAINERS                                        |   8 +
>>>>>>>>    drivers/remoteproc/qcom_q6v5.c                     |   4 +
>>>>>>>>    drivers/remoteproc/qcom_q6v5_mss.c                 |   8 -
>>>>>>>>    drivers/thermal/qcom/Kconfig                       |  13 +
>>>>>>>>    drivers/thermal/qcom/Makefile                      |   1 +
>>>>>>>>    drivers/thermal/qcom/qmi-cooling.c                 | 520 +++++++++++++++++++++
>>>>>>>>    drivers/thermal/qcom/qmi-cooling.h                 | 428 +++++++++++++++++
>>>>>>>>    10 files changed, 1161 insertions(+), 8 deletions(-)
>>>>>>>> ---
>>>>>>>> base-commit: 9067f80db58bbce81d5f0703aa2fd261e88bc812
>>>>>>>>
>>>>>>>> // Caleb (they/them)
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> // Caleb (they/them)
>>>>>
>>>>> --
>>>>> மணிவண்ணன் சதாசிவம்
>>>>
>>>>
>>>>
>>>> --
>>>> With best wishes
>>>> Dmitry
>>>
>>> --
>>> மணிவண்ணன் சதாசிவம்
>>
>>
>>
>> -- 
>> With best wishes
>> Dmitry
> 

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Bjorn Andersson]

On Mon, Oct 02, 2023 at 09:43:08PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 02, 2023 at 07:00:27PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 2 Oct 2023 at 18:58, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
> > > > On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > > >
> > > > > On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> > > > > >
> > > > > >
> > > > > > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > > > > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > > > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > > > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > > > > > It exposes various mitigations including passive thermal controls and
> > > > > > > > rail voltage restrictions.
> > > > > > > >
> > > > > > > > This series introduces support for exposing TMDs as cooling devices
> > > > > > > > in the kernel through the thermal framework, using the QMI interface.
> > > > > > > >
> > > > > > > > Each TMD client is described as a child of the remoteproc node in
> > > > > > > > devicetree. With subnodes for each control.
> > > > > > > >
> > > > > > >
> > > > > > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > > > > > cooling device since it is not tied to thermal zones and the governors cannot
> > > > > > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > > > > > constraints.
> > > > > >
> > > > > > Hi Mani,
> > > > > >
> > > > > > Forgive me as I'm not yet super familiar with the thermal subsystem.
> > > > > >
> > > > > > As I understand it, the DT layout here enables each control to be referenced
> > > > > > under the thermal zones, at least this is the approach taken in CAF 4.9.
> > > > > >
> > > > > > Maybe I don't quite understand what you mean, are you saying that using
> > > > > > thermal zones is the wrong approach?
> > > > >
> > > > > Thermal framework expects each thermal zone represented in DT to have atleast
> > > > > one corresponding thermal sensor defined using "thermal-sensors" property. But
> > > > > with TMD, there is no thermal sensor AFAIK.
> > > >
> > > > As far as I understand, no. It is perfectly fine to have 'cooling'
> > > > devices, which react to external thermal monitoring events. I might be
> > > > mistaken, but I think that is the case here, isn't it?
> > > >
> > >
> > > Yes it is represented as cooling device(s). But I do not see any cognizant way
> > > to plug it with thermal zones i.e., unless TMD itself reports temperature of the
> > > modem, using it as a cooling device for external temperature events doesn't
> > > sound good to me.
> > 
> > Why? We have compute, q6, wlan tsens sensors. So it seems natural to
> > tell CDSP to slow down if compute sensor reports overheating.
> > 
> 
> TMD is for external devices such as PCIe modems as well. Is there a temperature
> sensor for that?
> 

According to the schematics for the SC8280XP CRD sys_therm5 would be the
sensor you're looking for.

Regards,
Bjorn

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Manivannan Sadhasivam]

On Wed, Oct 04, 2023 at 07:36:58PM -0700, Bjorn Andersson wrote:
> On Mon, Oct 02, 2023 at 09:43:08PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Oct 02, 2023 at 07:00:27PM +0300, Dmitry Baryshkov wrote:
> > > On Mon, 2 Oct 2023 at 18:58, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > >
> > > > On Mon, Oct 02, 2023 at 06:00:37PM +0300, Dmitry Baryshkov wrote:
> > > > > On Mon, 2 Oct 2023 at 17:52, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > > > > >
> > > > > > On Sun, Oct 01, 2023 at 06:26:14PM +0100, Caleb Connolly wrote:
> > > > > > >
> > > > > > >
> > > > > > > On 01/10/2023 16:57, Manivannan Sadhasivam wrote:
> > > > > > > > On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > > > > > > > > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > > > > > > > > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > > > > > > > > It exposes various mitigations including passive thermal controls and
> > > > > > > > > rail voltage restrictions.
> > > > > > > > >
> > > > > > > > > This series introduces support for exposing TMDs as cooling devices
> > > > > > > > > in the kernel through the thermal framework, using the QMI interface.
> > > > > > > > >
> > > > > > > > > Each TMD client is described as a child of the remoteproc node in
> > > > > > > > > devicetree. With subnodes for each control.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Daniel expressed concerns in the past aganist representing TMD driver as a
> > > > > > > > cooling device since it is not tied to thermal zones and the governors cannot
> > > > > > > > use it. Instead he suggested to represent it as a powercap device with thermal
> > > > > > > > constraints.
> > > > > > >
> > > > > > > Hi Mani,
> > > > > > >
> > > > > > > Forgive me as I'm not yet super familiar with the thermal subsystem.
> > > > > > >
> > > > > > > As I understand it, the DT layout here enables each control to be referenced
> > > > > > > under the thermal zones, at least this is the approach taken in CAF 4.9.
> > > > > > >
> > > > > > > Maybe I don't quite understand what you mean, are you saying that using
> > > > > > > thermal zones is the wrong approach?
> > > > > >
> > > > > > Thermal framework expects each thermal zone represented in DT to have atleast
> > > > > > one corresponding thermal sensor defined using "thermal-sensors" property. But
> > > > > > with TMD, there is no thermal sensor AFAIK.
> > > > >
> > > > > As far as I understand, no. It is perfectly fine to have 'cooling'
> > > > > devices, which react to external thermal monitoring events. I might be
> > > > > mistaken, but I think that is the case here, isn't it?
> > > > >
> > > >
> > > > Yes it is represented as cooling device(s). But I do not see any cognizant way
> > > > to plug it with thermal zones i.e., unless TMD itself reports temperature of the
> > > > modem, using it as a cooling device for external temperature events doesn't
> > > > sound good to me.
> > > 
> > > Why? We have compute, q6, wlan tsens sensors. So it seems natural to
> > > tell CDSP to slow down if compute sensor reports overheating.
> > > 
> > 
> > TMD is for external devices such as PCIe modems as well. Is there a temperature
> > sensor for that?
> > 
> 
> According to the schematics for the SC8280XP CRD sys_therm5 would be the
> sensor you're looking for.
> 

Hmm, then it seems fine from my end as long we have the correct sensor data to
hook up these cooling devices.

- Mani

> Regards,
> Bjorn

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/4] thermal: Introduce Qualcomm Thermal Mitigation Device support

[Bjorn Andersson]

On Sun, Oct 01, 2023 at 09:27:01PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Sep 29, 2023 at 05:16:16PM +0100, Caleb Connolly wrote:
> > The Thermal Mitigation Device (TMD) Service is a QMI service that runs
> > on remote subsystems (the modem and DSPs) on Qualcomm SoCs.
> > It exposes various mitigations including passive thermal controls and
> > rail voltage restrictions.
> > 
> > This series introduces support for exposing TMDs as cooling devices
> > in the kernel through the thermal framework, using the QMI interface.
> > 
> > Each TMD client is described as a child of the remoteproc node in
> > devicetree. With subnodes for each control.
> > 
> 
> Daniel expressed concerns in the past aganist representing TMD driver as a
> cooling device since it is not tied to thermal zones and the governors cannot
> use it. Instead he suggested to represent it as a powercap device with thermal
> constraints.
> 
> So please look into that approach.
> 

The powercap framework revolves around the idea that we have some amount
of power (micro-watt) being available to the system, which can be split
across a range of devices.

Say that we implement this as a powercap thing, what current consumption
would you attribute these entires? How would you map a given uW value to
the mitigation levels provided by the qmi-cooling instances?


Beyond that, I'm still not sure how we would plug this in. We don't have
a picture of the power consumption/flow through the system at any point
in time - as the control of the power grid is distributed across the
various subsystems.

Regards,
Bjorn