Skip to contentSkip to conversation

Use qmp_send to update co-processor load state

PATCH Superseded Sibi Sankar 5 participants 25 messages Jul 20, 2021 – Jul 30, 2021

Subsystems (8)

v4 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v4 cover Use qmp_send to update co-processor load state

The power domains exposed by the AOSS QMP driver control the load state resources linked to modem, adsp, cdsp remoteprocs. These are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. AOSS uses this information to wait for the co-processors to suspend before starting its sleep sequence. These co-processors enter low-power modes independent to that of the application processor and the load state resources linked to them are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's stop modeling them as power-domains and replace them with generic qmp_send interface instead.

https://lore.kernel.org/lkml/20200913034603.GV3715@yoga/
Previous discussion on dropping power-domain support from AOSS QMP driver

Depends on:
aoss yaml: https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/
qmp_send: https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/

V4:
* Rebase patch 1 due to the aoss-qmp yaml conversion (Dropping Rb).
* Commit message change and sc8180x co-processor addition to patch 2. [Rob/Bjorn]
* Drop unused pdev and kfree the load state string in q6v5_deinit /probe path for patch 4. [Matthias]
* Replaced "binding" with "property" across the series. [Matthias]
* Commit message change and drop incorrect cleanup on cooling device probe failures. [Matthias]

V3:
* Misc. documentation fixes [patch 2]:
- Reduce power-domain maxItems due to load_state pd removal
- Combine compatibles where possible with the load_state pd removal
- Fixup the qcom,qmp ref to phandle type

V2:
* load_state is currently broken on mainline so be safely dropped without side-effects.
* Rebased on top of qmp_send v3 series.
* Dropped R-b from Stephen and Rob on patch 3 due to the yaml conversion.
* New patch [12] to drop unused aoss-qmp header.
* Commit message update [patch 1] [Rob]
* Reorder the series [Stephen]

Sibi Sankar (13):
dt-bindings: soc: qcom: aoss: Drop the load state power-domain
dt-bindings: remoteproc: qcom: pas: Add QMP property
dt-bindings: remoteproc: qcom: Add QMP property
remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state
arm64: dts: qcom: sc7180: Use QMP property to control load state
arm64: dts: qcom: sc7280: Use QMP property to control load state
arm64: dts: qcom: sdm845: Use QMP property to control load state
arm64: dts: qcom: sm8150: Use QMP property to control load state
arm64: dts: qcom: sm8250: Use QMP property to control load state
arm64: dts: qcom: sm8350: Use QMP property to control load state
soc: qcom: aoss: Drop power domain support
dt-bindings: msm/dp: Remove aoss-qmp header
dt-bindings: soc: qcom: aoss: Delete unused power-domain definitions

 .../bindings/display/msm/dp-controller.yaml        |   1 -
 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  |  65 +++++++------
 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |   7 +-
 .../bindings/soc/qcom/qcom,aoss-qmp.yaml           |  11 +--
 arch/arm64/boot/dts/qcom/sc7180.dtsi               |   9 +-
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |   2 -
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |   8 +-
 arch/arm64/boot/dts/qcom/sm8150.dtsi               |  28 +++---
 arch/arm64/boot/dts/qcom/sm8250.dtsi               |  22 ++---
 arch/arm64/boot/dts/qcom/sm8350.dtsi               |  30 +++---
 drivers/remoteproc/qcom_q6v5.c                     |  57 ++++++++++-
 drivers/remoteproc/qcom_q6v5.h                     |   7 +-
 drivers/remoteproc/qcom_q6v5_adsp.c                |   7 +-
 drivers/remoteproc/qcom_q6v5_mss.c                 |  44 ++-------
 drivers/remoteproc/qcom_q6v5_pas.c                 |  85 ++++------------
 drivers/remoteproc/qcom_q6v5_wcss.c                |   4 +-
 drivers/soc/qcom/qcom_aoss.c                       | 107 ---------------------
 include/dt-bindings/power/qcom-aoss-qmp.h          |  14 ---
 18 files changed, 185 insertions(+), 323 deletions(-)
 delete mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

PATCH v4 1/13 dt-bindings: soc: qcom: aoss: Drop the load state power-domain

The power-domains exposed by AOSS QMP node are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. These co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load power-domain and replace them with generic qmp_send interface instead.

---

v4:
* Rebase patch due to the recent aoss-qmp yaml conversion (Dropping Rb).

 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.yaml | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)
Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.yaml +1 -10
index 93e4b737ee1b..c55e98fc14fa 100644
@@ -19,8 +19,7 @@ description:
 
   The AOSS side channel exposes control over a set of resources, used to control
   a set of debug related clocks and to affect the low power state of resources
-  related to the secondary subsystems. These resources are exposed as a set of
-  power-domains.
+  related to the secondary subsystems.
 
 properties:
   compatible:
@@ -57,13 +56,6 @@ properties:
     description:
       The single clock represents the QDSS clock.
 
-  "#power-domain-cells":
-    const: 1
-    description: |
-        The provided power-domains are:
-        CDSP state (0), LPASS state (1), modem state (2), SLPI
-        state (3), SPSS state (4) and Venus state (5).
-
 required:
   - compatible
   - reg
@@ -101,7 +93,6 @@ examples:
       mboxes = <&apss_shared 0>;
 
       #clock-cells = <0>;
-      #power-domain-cells = <1>;
 
       cx_cdev: cx {
         #cooling-cells = <2>;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Reviewed-by on v4 1/13
#2 lore raw

Re: [PATCH v4 01/13] dt-bindings: soc: qcom: aoss: Drop the load state power-domain

Quoted 8 lines · Sibi Sankar

The power-domains exposed by AOSS QMP node are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. These co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load power-domain and replace them with generic qmp_send interface instead.

Reviewed-by on v4 1/13
#3 lore raw

Re: [PATCH v4 01/13] dt-bindings: soc: qcom: aoss: Drop the load state power-domain

In reply to Sibi Sankar

Quoting Sibi Sankar (2021-07-19 21:36:35)

Quoted 9 lines

The power-domains exposed by AOSS QMP node are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. These co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load power-domain and replace them with generic qmp_send interface instead.

---

maintainer Acked-by on v4 1/13
#4 lore raw

Re: [PATCH v4 01/13] dt-bindings: soc: qcom: aoss: Drop the load state power-domain

In reply to Sibi Sankar
Quoted 7 lines · Sibi Sankar

The power-domains exposed by AOSS QMP node are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. These co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load power-domain and replace them with generic qmp_send interface instead.

Quoted 9 lines

---

v4:
* Rebase patch due to the recent aoss-qmp yaml conversion (Dropping Rb).

 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.yaml | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)
#5 lore raw

PATCH v4 2/13 dt-bindings: remoteproc: qcom: pas: Add QMP property

In reply to Sibi Sankar

The load state power-domain, used by the co-processors to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down, suffers from the side-effect of changing states during suspend/resume. However the co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load state power-domain and replace them with the qmp property for all SoCs supporting low power mode signalling.

---

v4:
* Commit message change and sc8180x co-processor addition. [Rob/Bjorn]

 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 65 +++++++++++-----------
 1 file changed, 33 insertions(+), 32 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +33 -32
index c597ccced623..1182afb5f593 100644
@@ -78,11 +78,11 @@ properties:
 
   power-domains:
     minItems: 1
-    maxItems: 3
+    maxItems: 2
 
   power-domain-names:
     minItems: 1
-    maxItems: 3
+    maxItems: 2
 
   firmware-name:
     $ref: /schemas/types.yaml#/definitions/string
@@ -92,6 +92,10 @@ properties:
     maxItems: 1
     description: Reference to the reserved-memory for the Hexagon core
 
+  qcom,qmp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference to the AOSS side-channel message RAM.
+
   qcom,smem-states:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     description: States used by the AP to signal the Hexagon core
@@ -368,13 +372,11 @@ allOf:
       properties:
         power-domains:
           items:
-            - description: Load State power domain
             - description: CX power domain
             - description: MX power domain
             - description: MSS power domain
         power-domain-names:
           items:
-            - const: load_state
             - const: cx
             - const: mx
             - const: mss
@@ -390,43 +392,20 @@ allOf:
       properties:
         power-domains:
           items:
-            - description: Load State power domain
             - description: CX power domain
-        power-domain-names:
-          items:
-            - const: load_state
-            - const: cx
 
   - if:
       properties:
         compatible:
           contains:
             enum:
+              - qcom,sdx55-mpss-pas
               - qcom,sm8150-mpss-pas
               - qcom,sm8350-mpss-pas
     then:
       properties:
         power-domains:
           items:
-            - description: Load State power domain
-            - description: CX power domain
-            - description: MSS power domain
-        power-domain-names:
-          items:
-            - const: load_state
-            - const: cx
-            - const: mss
-
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sdx55-mpss-pas
-    then:
-      properties:
-        power-domains:
-          items:
             - description: CX power domain
             - description: MSS power domain
         power-domain-names:
@@ -450,12 +429,10 @@ allOf:
       properties:
         power-domains:
           items:
-            - description: Load State power domain
             - description: LCX power domain
             - description: LMX power domain
         power-domain-names:
           items:
-            - const: load_state
             - const: lcx
             - const: lmx
 
@@ -469,12 +446,10 @@ allOf:
       properties:
         power-domains:
           items:
-            - description: Load State power domain
             - description: CX power domain
             - description: MXC power domain
         power-domain-names:
           items:
-            - const: load_state
             - const: cx
             - const: mxc
 
@@ -510,6 +485,32 @@ allOf:
             - const: mss_restart
             - const: pdc_reset
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,sc7180-mpss-pas
+              - qcom,sc8180x-adsp-pas
+              - qcom,sc8180x-cdsp-pas
+              - qcom,sc8180x-mpss-pas
+              - qcom,sm8150-adsp-pas
+              - qcom,sm8150-cdsp-pas
+              - qcom,sm8150-mpss-pas
+              - qcom,sm8150-slpi-pas
+              - qcom,sm8250-adsp-pas
+              - qcom,sm8250-cdsp-pas
+              - qcom,sm8250-slpi-pas
+              - qcom,sm8350-adsp-pas
+              - qcom,sm8350-cdsp-pas
+              - qcom,sm8350-mpss-pas
+              - qcom,sm8350-slpi-pas
+    then:
+      properties:
+        qcom,qmp:
+          items:
+            - description: Reference to the AOSS side-channel message RAM.
+
 examples:
   - |
     #include <dt-bindings/clock/qcom,rpmcc.h>
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#6 lore raw

Re: [PATCH v4 02/13] dt-bindings: remoteproc: qcom: pas: Add QMP property

Quoted 23 lines · Sibi Sankar

The load state power-domain, used by the co-processors to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down, suffers from the side-effect of changing states during suspend/resume. However the co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load state power-domain and replace them with the qmp property for all SoCs supporting low power mode signalling.

---

v4:
* Commit message change and sc8180x co-processor addition. [Rob/Bjorn]

 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 65 +++++++++++-----------
 1 file changed, 33 insertions(+), 32 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +0 -0
index c597ccced623..1182afb5f593 100644

Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml properties: · line 85

Quoted 10 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +2 -2
@@ -78,11 +78,11 @@ properties:
 
   power-domains:
     minItems: 1
-    maxItems: 3
+    maxItems: 2
 
   power-domain-names:
     minItems: 1
-    maxItems: 3
+    maxItems: 2

It seems maxItems should have been 4 in the first place and should remain unchanged after removing the load state power domain. With this patch:

- if:
properties:
compatible:
contains:
enum:
- qcom,sc7180-mpss-pas
then:
properties:
power-domains:
items:
- description: CX power domain
- description: MX power domain
- description: MSS power domain
power-domain-names:
items:
- const: cx
- const: mx
- const: mss

#7 lore raw

Re: [PATCH v4 02/13] dt-bindings: remoteproc: qcom: pas: Add QMP property

Hey Matthias,

Thanks for taking time to review
the series.

Quoted 32 lines · Matthias Kaehlcke
Quoted 31 lines · Sibi Sankar

The load state power-domain, used by the co-processors to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down, suffers from the side-effect of changing states during suspend/resume. However the co-processors enter low-power modes independent to that of the application processor and their states are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior let's drop the load state power-domain and replace them with the qmp property for all SoCs supporting low power mode signalling.

---

v4:
* Commit message change and sc8180x co-processor addition.
[Rob/Bjorn]

 .../devicetree/bindings/remoteproc/qcom,adsp.yaml  | 65 
+++++++++++-----------
 1 file changed, 33 insertions(+), 32 deletions(-)
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +1 -1
index c597ccced623..1182afb5f593 100644
@@ -78,11 +78,11 @@ properties:
 
   power-domains:
     minItems: 1
-    maxItems: 3
+    maxItems: 2
 
Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml properties: · line 85

Quoted 4 lines
Documentation/devicetree/bindings/remoteproc/qcom,adsp.yaml +1 -1
   power-domain-names:
     minItems: 1
-    maxItems: 3
+    maxItems: 2

It seems maxItems should have been 4 in the first place and should remain unchanged after removing the load state power domain. With this patch:

sc7180-mpss-pas actually uses only
cx and mss. The mpss-pas compatible
is overridden by the mss-pil compatible
for all the platforms present upstream
for sc7180, that's the reason we probably
haven't run into any binding check failures.
I'll keep the max-items to 2 and fix-up
the sc7180 power-domain requirements
instead.

Quoted 19 lines

- if:
properties:
compatible:
contains:
enum:
- qcom,sc7180-mpss-pas
then:
properties:
power-domains:
items:
- description: CX power domain
- description: MX power domain
- description: MSS power domain
power-domain-names:
items:
- const: cx
- const: mx
- const: mss

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
#8 lore raw

PATCH v4 3/13 dt-bindings: remoteproc: qcom: Add QMP property

In reply to Sibi Sankar

Add Qualcomm Mailbox Protocol (QMP) property to replace the power domain exposed by the AOSS QMP node.

---
 Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +6 -1
index 69c49c7b2cff..494257010629 100644
@@ -174,7 +174,12 @@ For the compatible string below the following supplies are required:
 		    must be "cx", "mx"
 	qcom,sc7180-mss-pil:
 	qcom,sdm845-mss-pil:
-		    must be "cx", "mx", "mss", "load_state"
+		    must be "cx", "mx", "mss"
+
+- qcom,qmp:
+	Usage: optional
+	Value type: <phandle>
+	Definition: reference to the AOSS side-channel message RAM.
 
 - qcom,smem-states:
 	Usage: required
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#9 lore raw

PATCH v4 4/13 remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state

In reply to Sibi Sankar

The power domains exposed by the AOSS QMP driver control the load state resources linked to modem, adsp, cdsp remoteprocs. These are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. AOSS uses this information to wait for the co-processors to suspend before starting its sleep sequence.

These co-processors enter low-power modes independent to that of the application processor and the load state resources linked to them are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior lets stop using the power-domains exposed by the AOSS QMP node and replace them with generic qmp_send interface instead.

---

v4:
* Drop unused pdev and kfree the load state string in q6v5_deinit /probe path for patch 4. [Matthias]

 drivers/remoteproc/qcom_q6v5.c      | 57 ++++++++++++++++++++++++-
 drivers/remoteproc/qcom_q6v5.h      |  7 ++-
 drivers/remoteproc/qcom_q6v5_adsp.c |  7 ++-
 drivers/remoteproc/qcom_q6v5_mss.c  | 44 ++++---------------
 drivers/remoteproc/qcom_q6v5_pas.c  | 85 +++++++++----------------------------
 drivers/remoteproc/qcom_q6v5_wcss.c |  4 +-
 6 files changed, 96 insertions(+), 108 deletions(-)
drivers/remoteproc/qcom_q6v5.c +56 -1
index 7e9244c748da..997ff21271f7 100644
@@ -16,8 +16,28 @@
 #include "qcom_common.h"
 #include "qcom_q6v5.h"
 
+#define Q6V5_LOAD_STATE_MSG_LEN	64
 #define Q6V5_PANIC_DELAY_MS	200
 
+static int q6v5_load_state_toggle(struct qcom_q6v5 *q6v5, bool enable)
+{
+	char buf[Q6V5_LOAD_STATE_MSG_LEN] = {};
+	int ret;
+
+	if (IS_ERR(q6v5->qmp))
+		return 0;
+
+	snprintf(buf, sizeof(buf),
+		 "{class: image, res: load_state, name: %s, val: %s}",
+		 q6v5->load_state, enable ? "on" : "off");
+
+	ret = qmp_send(q6v5->qmp, buf, sizeof(buf));
+	if (ret)
+		dev_err(q6v5->dev, "failed to toggle load state\n");
+
+	return ret;
+}
+
 /**
  * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
  * @q6v5:	reference to qcom_q6v5 context to be reinitialized
@@ -26,6 +46,12 @@
  */
 int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
 {
+	int ret;
+
+	ret = q6v5_load_state_toggle(q6v5, true);
+	if (ret)
+		return ret;
+
 	reinit_completion(&q6v5->start_done);
 	reinit_completion(&q6v5->stop_done);
 
@@ -47,6 +73,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_prepare);
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
 {
 	disable_irq(q6v5->handover_irq);
+	q6v5_load_state_toggle(q6v5, false);
 
 	return !q6v5->handover_issued;
 }
@@ -196,12 +223,13 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
  * @pdev:	platform_device reference for acquiring resources
  * @rproc:	associated remoteproc instance
  * @crash_reason: SMEM id for crash reason string, or 0 if none
+ * @load_state: load state resource string
  * @handover:	function to be called when proxy resources should be released
  *
  * Return: 0 on success, negative errno on failure
  */
 int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
-		   struct rproc *rproc, int crash_reason,
+		   struct rproc *rproc, int crash_reason, const char *load_state,
 		   void (*handover)(struct qcom_q6v5 *q6v5))
 {
 	int ret;
@@ -286,9 +314,36 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
 		return PTR_ERR(q6v5->state);
 	}
 
+	q6v5->load_state = kstrdup_const(load_state, GFP_KERNEL);
+	q6v5->qmp = qmp_get(&pdev->dev);
+	if (IS_ERR(q6v5->qmp)) {
+		if (PTR_ERR(q6v5->qmp) != -ENODEV) {
+			if (PTR_ERR(q6v5->qmp) != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "failed to acquire load state\n");
+			kfree_const(q6v5->load_state);
+			return PTR_ERR(q6v5->qmp);
+		}
+	} else {
+		if (!q6v5->load_state) {
+			dev_err(&pdev->dev, "load state resource string empty\n");
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
 
+/**
+ * qcom_q6v5_deinit() - deinitialize the q6v5 common struct
+ * @q6v5:	reference to qcom_q6v5 context to be deinitialized
+ */
+void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
+{
+	kfree_const(q6v5->load_state);
+	qmp_put(q6v5->qmp);
+}
+EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Qualcomm Peripheral Image Loader for Q6V5");
drivers/remoteproc/qcom_q6v5.h +6 -1
index 1c212f670cbc..f35e04471ed7 100644
@@ -5,6 +5,7 @@
 
 #include <linux/kernel.h>
 #include <linux/completion.h>
+#include <linux/soc/qcom/qcom_aoss.h>
 
 struct rproc;
 struct qcom_smem_state;
@@ -15,6 +16,8 @@ struct qcom_q6v5 {
 	struct rproc *rproc;
 
 	struct qcom_smem_state *state;
+	struct qmp *qmp;
+
 	unsigned stop_bit;
 
 	int wdog_irq;
@@ -32,12 +35,14 @@ struct qcom_q6v5 {
 
 	bool running;
 
+	const char *load_state;
 	void (*handover)(struct qcom_q6v5 *q6v5);
 };
 
 int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
-		   struct rproc *rproc, int crash_reason,
+		   struct rproc *rproc, int crash_reason, const char *load_state,
 		   void (*handover)(struct qcom_q6v5 *q6v5));
+void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5);
 
 int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
drivers/remoteproc/qcom_q6v5_adsp.c +5 -2
index 8b0d8bbacd2e..098362e6e233 100644
@@ -185,7 +185,9 @@ static int adsp_start(struct rproc *rproc)
 	int ret;
 	unsigned int val;
 
-	qcom_q6v5_prepare(&adsp->q6v5);
+	ret = qcom_q6v5_prepare(&adsp->q6v5);
+	if (ret)
+		return ret;
 
 	ret = clk_prepare_enable(adsp->xo);
 	if (ret)
@@ -465,7 +467,7 @@ static int adsp_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_pm;
 
-	ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem,
+	ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem, NULL,
 			     qcom_adsp_pil_handover);
 	if (ret)
 		goto disable_pm;
@@ -500,6 +502,7 @@ static int adsp_remove(struct platform_device *pdev)
 
 	rproc_del(adsp->rproc);
 
+	qcom_q6v5_deinit(&adsp->q6v5);
 	qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev);
 	qcom_remove_sysmon_subdev(adsp->sysmon);
 	qcom_remove_ssr_subdev(adsp->rproc, &adsp->ssr_subdev);
drivers/remoteproc/qcom_q6v5_mss.c +8 -36

134 lines not on this page · raw message

drivers/remoteproc/qcom_q6v5_pas.c +19 -66

261 lines not on this page · raw message

drivers/remoteproc/qcom_q6v5_wcss.c +2 -2
index 20d50ec7eff1..0689288a2425 100644
@@ -1044,8 +1044,7 @@ static int q6v5_wcss_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
-	ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, desc->crash_reason_smem,
-			     NULL);
+	ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, desc->crash_reason_smem, NULL, NULL);
 	if (ret)
 		goto free_rproc;
 
@@ -1075,6 +1074,7 @@ static int q6v5_wcss_remove(struct platform_device *pdev)
 {
 	struct rproc *rproc = platform_get_drvdata(pdev);
 
+	qcom_q6v5_deinit(&qproc->q6v5);
 	rproc_del(rproc);
 	rproc_free(rproc);
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#10 lore raw

Re: [PATCH v4 04/13] remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state

Quoting Sibi Sankar (2021-07-19 21:36:38)

Quoted 5 lines
drivers/remoteproc/qcom_q6v5.c +0 -0
index 7e9244c748da..997ff21271f7 100644

drivers/remoteproc/qcom_q6v5.c line 24

Quoted 9 lines
drivers/remoteproc/qcom_q6v5.c +4 -0
@@ -16,8 +16,28 @@
 #include "qcom_common.h"
 #include "qcom_q6v5.h"
 
+#define Q6V5_LOAD_STATE_MSG_LEN        64
 #define Q6V5_PANIC_DELAY_MS    200
 
+static int q6v5_load_state_toggle(struct qcom_q6v5 *q6v5, bool enable)
+{
+       char buf[Q6V5_LOAD_STATE_MSG_LEN] = {};

Just to confirm, we want to set the whole buffer to zero before writing it? Sounds good to not send stack junk over to to the other side but maybe we could skip initializing to zero for the first part of the buffer that isn't used?

drivers/remoteproc/qcom_q6v5.c line 32

Quoted 8 lines
drivers/remoteproc/qcom_q6v5.c +8 -0
+       int ret;
+
+       if (IS_ERR(q6v5->qmp))
+               return 0;
+
+       snprintf(buf, sizeof(buf),
+                "{class: image, res: load_state, name: %s, val: %s}",
+                q6v5->load_state, enable ? "on" : "off");

Should we WARN_ON() if the message doesn't fit into the 64-bytes?

Quoted 27 lines
drivers/remoteproc/qcom_q6v5.c +10 -1
+
+       ret = qmp_send(q6v5->qmp, buf, sizeof(buf));
+       if (ret)
+               dev_err(q6v5->dev, "failed to toggle load state\n");
+
+       return ret;
+}
+
 /**
  * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
  * @q6v5:      reference to qcom_q6v5 context to be reinitialized
@@ -196,12 +223,13 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
  * @pdev:      platform_device reference for acquiring resources
  * @rproc:     associated remoteproc instance
  * @crash_reason: SMEM id for crash reason string, or 0 if none
+ * @load_state: load state resource string
  * @handover:  function to be called when proxy resources should be released
  *
  * Return: 0 on success, negative errno on failure
  */
 int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
-                  struct rproc *rproc, int crash_reason,
+                  struct rproc *rproc, int crash_reason, const char *load_state,
                   void (*handover)(struct qcom_q6v5 *q6v5))
 {
        int ret;

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 322

Quoted 9 lines
drivers/remoteproc/qcom_q6v5.c +6 -0
@@ -286,9 +314,36 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
                return PTR_ERR(q6v5->state);
        }
 
+       q6v5->load_state = kstrdup_const(load_state, GFP_KERNEL);
+       q6v5->qmp = qmp_get(&pdev->dev);
+       if (IS_ERR(q6v5->qmp)) {
+               if (PTR_ERR(q6v5->qmp) != -ENODEV) {
+                       if (PTR_ERR(q6v5->qmp) != -EPROBE_DEFER)
+                               dev_err(&pdev->dev, "failed to acquire load state\n");

Use dev_err_probe()?

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 327

Quoted 5 lines
drivers/remoteproc/qcom_q6v5.c +5 -0
+                       kfree_const(q6v5->load_state);
+                       return PTR_ERR(q6v5->qmp);
+               }
+       } else {
+               if (!q6v5->load_state) {

Use else if and deindent?

Quoted 9 lines
drivers/remoteproc/qcom_q6v5.c +5 -0
+                       dev_err(&pdev->dev, "load state resource string empty\n");
+                       return -EINVAL;
+               }
+       }
+
        return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
 
#11 lore raw

Re: [PATCH v4 04/13] remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state

Quoted 12 lines · Stephen Boyd

Quoting Sibi Sankar (2021-07-19 21:36:38)

Quoted 11 lines
drivers/remoteproc/qcom_q6v5.c +1 -0
index 7e9244c748da..997ff21271f7 100644
@@ -16,8 +16,28 @@
 #include "qcom_common.h"
 #include "qcom_q6v5.h"
 
+#define Q6V5_LOAD_STATE_MSG_LEN        64
 #define Q6V5_PANIC_DELAY_MS    200
 
Quoted 9 lines

drivers/remoteproc/qcom_q6v5.c line 24

Quoted 4 lines
drivers/remoteproc/qcom_q6v5.c +3 -0
+static int q6v5_load_state_toggle(struct qcom_q6v5 *q6v5, bool 
 enable)
+{
+       char buf[Q6V5_LOAD_STATE_MSG_LEN] = {};

Just to confirm, we want to set the whole buffer to zero before writing it? Sounds good to not send stack junk over to to the other side but maybe we could skip initializing to zero for the first part of the buffer that isn't used?

Sure, it makes sense to incorporate
a warn_on and memset the remainder
of the buffer after populating it.

Quoted 47 lines

drivers/remoteproc/qcom_q6v5.c line 32

Quoted 9 lines
drivers/remoteproc/qcom_q6v5.c +8 -0
+       int ret;
+
+       if (IS_ERR(q6v5->qmp))
+               return 0;
+
+       snprintf(buf, sizeof(buf),
+                "{class: image, res: load_state, name: %s, val: %s}",
+                q6v5->load_state, enable ? "on" : "off");

Should we WARN_ON() if the message doesn't fit into the 64-bytes?

Quoted 35 lines
drivers/remoteproc/qcom_q6v5.c +8 -0
+
+       ret = qmp_send(q6v5->qmp, buf, sizeof(buf));
+       if (ret)
+               dev_err(q6v5->dev, "failed to toggle load state\n");
+
+       return ret;
+}
+
 /**
  * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before 

start
* @q6v5: reference to qcom_q6v5 context to be reinitialized

drivers/remoteproc/qcom_q6v5.c +1 -0
@@ -196,12 +223,13 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
  * @pdev:      platform_device reference for acquiring resources
  * @rproc:     associated remoteproc instance
  * @crash_reason: SMEM id for crash reason string, or 0 if none
+ * @load_state: load state resource string
  * @handover:  function to be called when proxy resources should be 

released
*
* Return: 0 on success, negative errno on failure
*/
int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device
*pdev,

drivers/remoteproc/qcom_q6v5.c +1 -1
-                  struct rproc *rproc, int crash_reason,
+                  struct rproc *rproc, int crash_reason, const char 

*load_state,

                   void (*handover)(struct qcom_q6v5 *q6v5))
 {
        int ret;
drivers/remoteproc/qcom_q6v5.c +0 -0
@@ -286,9 +314,36 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct 
 platform_device *pdev,
                return PTR_ERR(q6v5->state);
        }
 
Quoted 9 lines

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,

Quoted 7 lines
drivers/remoteproc/qcom_q6v5.c +6 -0
+       q6v5->load_state = kstrdup_const(load_state, GFP_KERNEL);
+       q6v5->qmp = qmp_get(&pdev->dev);
+       if (IS_ERR(q6v5->qmp)) {
+               if (PTR_ERR(q6v5->qmp) != -ENODEV) {
+                       if (PTR_ERR(q6v5->qmp) != -EPROBE_DEFER)
+                               dev_err(&pdev->dev, "failed to acquire 
 load state\n");

Use dev_err_probe()?

sure I'll use it.

Quoted 8 lines

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 327

Quoted 6 lines
drivers/remoteproc/qcom_q6v5.c +5 -0
+                       kfree_const(q6v5->load_state);
+                       return PTR_ERR(q6v5->qmp);
+               }
+       } else {
+               if (!q6v5->load_state) {

Use else if and deindent?

lol, my bad.

Quoted 11 lines
Quoted 11 lines
drivers/remoteproc/qcom_q6v5.c +5 -0
+                       dev_err(&pdev->dev, "load state resource 
 string empty\n");
+                       return -EINVAL;
+               }
+       }
+
        return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
#12 lore raw

Re: [PATCH v4 04/13] remoteproc: qcom: q6v5: Use qmp_send to update co-processor load state

In reply to Sibi Sankar
Quoted 34 lines · Sibi Sankar

The power domains exposed by the AOSS QMP driver control the load state resources linked to modem, adsp, cdsp remoteprocs. These are used to notify the Always on Subsystem (AOSS) that a particular co-processor is up/down. AOSS uses this information to wait for the co-processors to suspend before starting its sleep sequence.

These co-processors enter low-power modes independent to that of the application processor and the load state resources linked to them are expected to remain unaltered across system suspend/resume cycles. To achieve this behavior lets stop using the power-domains exposed by the AOSS QMP node and replace them with generic qmp_send interface instead.

---

v4:
* Drop unused pdev and kfree the load state string in q6v5_deinit /probe path for patch 4. [Matthias]

 drivers/remoteproc/qcom_q6v5.c      | 57 ++++++++++++++++++++++++-
 drivers/remoteproc/qcom_q6v5.h      |  7 ++-
 drivers/remoteproc/qcom_q6v5_adsp.c |  7 ++-
 drivers/remoteproc/qcom_q6v5_mss.c  | 44 ++++---------------
 drivers/remoteproc/qcom_q6v5_pas.c  | 85 +++++++++----------------------------
 drivers/remoteproc/qcom_q6v5_wcss.c |  4 +-
 6 files changed, 96 insertions(+), 108 deletions(-)
drivers/remoteproc/qcom_q6v5.c +0 -0
index 7e9244c748da..997ff21271f7 100644
@@ -16,8 +16,28 @@
 #include "qcom_common.h"
 #include "qcom_q6v5.h"

drivers/remoteproc/qcom_q6v5.c line 27

Quoted 10 lines
drivers/remoteproc/qcom_q6v5.c +7 -0
 
+#define Q6V5_LOAD_STATE_MSG_LEN	64
 #define Q6V5_PANIC_DELAY_MS	200
 
+static int q6v5_load_state_toggle(struct qcom_q6v5 *q6v5, bool enable)
+{
+	char buf[Q6V5_LOAD_STATE_MSG_LEN] = {};
+	int ret;
+
+	if (IS_ERR(q6v5->qmp))

I would prefer that you keep it NULL when there's no qmp.

Quoted 53 lines
drivers/remoteproc/qcom_q6v5.c +22 -1
+		return 0;
+
+	snprintf(buf, sizeof(buf),
+		 "{class: image, res: load_state, name: %s, val: %s}",
+		 q6v5->load_state, enable ? "on" : "off");
+
+	ret = qmp_send(q6v5->qmp, buf, sizeof(buf));
+	if (ret)
+		dev_err(q6v5->dev, "failed to toggle load state\n");
+
+	return ret;
+}
+
 /**
  * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
  * @q6v5:	reference to qcom_q6v5 context to be reinitialized
@@ -26,6 +46,12 @@
  */
 int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
 {
+	int ret;
+
+	ret = q6v5_load_state_toggle(q6v5, true);
+	if (ret)
+		return ret;
+
 	reinit_completion(&q6v5->start_done);
 	reinit_completion(&q6v5->stop_done);
 
@@ -47,6 +73,7 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_prepare);
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
 {
 	disable_irq(q6v5->handover_irq);
+	q6v5_load_state_toggle(q6v5, false);
 
 	return !q6v5->handover_issued;
 }
@@ -196,12 +223,13 @@ EXPORT_SYMBOL_GPL(qcom_q6v5_panic);
  * @pdev:	platform_device reference for acquiring resources
  * @rproc:	associated remoteproc instance
  * @crash_reason: SMEM id for crash reason string, or 0 if none
+ * @load_state: load state resource string
  * @handover:	function to be called when proxy resources should be released
  *
  * Return: 0 on success, negative errno on failure
  */
 int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
-		   struct rproc *rproc, int crash_reason,
+		   struct rproc *rproc, int crash_reason, const char *load_state,
 		   void (*handover)(struct qcom_q6v5 *q6v5))
 {
 	int ret;

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 317

Quoted 4 lines
drivers/remoteproc/qcom_q6v5.c +1 -0
@@ -286,9 +314,36 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
 		return PTR_ERR(q6v5->state);
 	}
 
+	q6v5->load_state = kstrdup_const(load_state, GFP_KERNEL);

Can't you use devm_kstrdup_const(&pdev->dev, ...) for this?

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 323

Quoted 6 lines
drivers/remoteproc/qcom_q6v5.c +6 -0
+	q6v5->qmp = qmp_get(&pdev->dev);
+	if (IS_ERR(q6v5->qmp)) {
+		if (PTR_ERR(q6v5->qmp) != -ENODEV) {
+			if (PTR_ERR(q6v5->qmp) != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "failed to acquire load state\n");
+			kfree_const(q6v5->load_state);

Then you don't need to free it here.

drivers/remoteproc/qcom_q6v5.c int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, · line 329

Quoted 6 lines
drivers/remoteproc/qcom_q6v5.c +6 -0
+			return PTR_ERR(q6v5->qmp);
+		}
+	} else {
+		if (!q6v5->load_state) {
+			dev_err(&pdev->dev, "load state resource string empty\n");
+			return -EINVAL;

I see two cases here:

1) kstrdup_const() failed to allocate memory, the error print is unnecessary and misleading and it would be more appropriate to return -ENOMEM.

2) kstrdup_const() failed because you passed load_state == NULL, in which case the error message could be more helpful by saying "unexpected qcom,qmp property found" or something like that.

Quoted 62 lines
drivers/remoteproc/qcom_q6v5.c +14 -0
+		}
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_q6v5_init);
 
+/**
+ * qcom_q6v5_deinit() - deinitialize the q6v5 common struct
+ * @q6v5:	reference to qcom_q6v5 context to be deinitialized
+ */
+void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5)
+{
+	kfree_const(q6v5->load_state);
+	qmp_put(q6v5->qmp);
+}
+EXPORT_SYMBOL_GPL(qcom_q6v5_deinit);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Qualcomm Peripheral Image Loader for Q6V5");
drivers/remoteproc/qcom_q6v5.h +6 -1
index 1c212f670cbc..f35e04471ed7 100644
@@ -5,6 +5,7 @@
 
 #include <linux/kernel.h>
 #include <linux/completion.h>
+#include <linux/soc/qcom/qcom_aoss.h>
 
 struct rproc;
 struct qcom_smem_state;
@@ -15,6 +16,8 @@ struct qcom_q6v5 {
 	struct rproc *rproc;
 
 	struct qcom_smem_state *state;
+	struct qmp *qmp;
+
 	unsigned stop_bit;
 
 	int wdog_irq;
@@ -32,12 +35,14 @@ struct qcom_q6v5 {
 
 	bool running;
 
+	const char *load_state;
 	void (*handover)(struct qcom_q6v5 *q6v5);
 };
 
 int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
-		   struct rproc *rproc, int crash_reason,
+		   struct rproc *rproc, int crash_reason, const char *load_state,
 		   void (*handover)(struct qcom_q6v5 *q6v5));
+void qcom_q6v5_deinit(struct qcom_q6v5 *q6v5);
 
 int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
 int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
drivers/remoteproc/qcom_q6v5_adsp.c +0 -0
index 8b0d8bbacd2e..098362e6e233 100644

drivers/remoteproc/qcom_q6v5_adsp.c static int adsp_start(struct rproc *rproc) · line 190

Quoted 7 lines
drivers/remoteproc/qcom_q6v5_adsp.c +3 -1
@@ -185,7 +185,9 @@ static int adsp_start(struct rproc *rproc)
 	int ret;
 	unsigned int val;
 
-	qcom_q6v5_prepare(&adsp->q6v5);
+	ret = qcom_q6v5_prepare(&adsp->q6v5);
+	if (ret)
+		return ret;

In the (hopefully unlikely) case that we have instances of qcom_q6v5_prepare() failing today, the switch to the new qmp interface would also introduce a regression in the same commit.

Could you please add the error handling in a separate commit? Just so that we can pinpoint which of the two changes caused any issues if we need to bisect this?

Regards,
Bjorn

#13 lore raw

PATCH v4 5/13 arm64: dts: qcom: sc7180: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SC7180 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
arch/arm64/boot/dts/qcom/sc7180.dtsi +4 -5
index a5d58eb92896..d3e5b67afcb4 100644
@@ -15,7 +15,6 @@
 #include <dt-bindings/interconnect/qcom,sc7180.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy-qcom-qusb2.h>
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/reset/qcom,sdm845-pdc.h>
@@ -1893,14 +1892,15 @@
 			clock-names = "iface", "bus", "nav", "snoc_axi",
 				      "mnoc_axi", "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
-					<&rpmhpd SC7180_CX>,
+			power-domains = <&rpmhpd SC7180_CX>,
 					<&rpmhpd SC7180_MX>,
 					<&rpmhpd SC7180_MSS>;
-			power-domain-names = "load_state", "cx", "mx", "mss";
+			power-domain-names = "cx", "mx", "mss";
 
 			memory-region = <&mpss_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&modem_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -3116,7 +3116,6 @@
 			mboxes = <&apss_shared 0>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 		};
 
 		spmi_bus: spmi@c440000 {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#14 lore raw

PATCH v4 6/13 arm64: dts: qcom: sc7280: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SC7280 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 --
 1 file changed, 2 deletions(-)
arch/arm64/boot/dts/qcom/sc7280.dtsi +0 -2
index a8c274ad74c4..d1adf64e21e9 100644
@@ -9,7 +9,6 @@
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/reset/qcom,sdm845-pdc.h>
@@ -1149,7 +1148,6 @@
 					IPCC_MPROC_SIGNAL_GLINK_QMP>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 		};
 
 		spmi_bus: spmi@c440000 {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#15 lore raw

PATCH v4 7/13 arm64: dts: qcom: sdm845: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SDM845 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/qcom/sdm845.dtsi +4 -4
index 1796ae8372be..8c3601810fb2 100644
@@ -2984,6 +2984,8 @@
 			clock-names = "iface", "bus", "mem", "gpll0_mss",
 				      "snoc_axi", "mnoc_axi", "prng", "xo";
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&modem_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -2993,11 +2995,10 @@
 
 			qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
 
-			power-domains = <&aoss_qmp 2>,
-					<&rpmhpd SDM845_CX>,
+			power-domains = <&rpmhpd SDM845_CX>,
 					<&rpmhpd SDM845_MX>,
 					<&rpmhpd SDM845_MSS>;
-			power-domain-names = "load_state", "cx", "mx", "mss";
+			power-domain-names = "cx", "mx", "mss";
 
 			mba {
 				memory-region = <&mba_region>;
@@ -4582,7 +4583,6 @@
 			mboxes = <&apss_shared 0>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 
 			cx_cdev: cx {
 				#cooling-cells = <2>;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#16 lore raw

PATCH v4 8/13 arm64: dts: qcom: sm8150: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SM8150 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sm8150.dtsi | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
arch/arm64/boot/dts/qcom/sm8150.dtsi +14 -14
index 612dda0fef43..7a040b74ef5a 100644
@@ -6,7 +6,6 @@
 
 #include <dt-bindings/dma/qcom-gpi.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
@@ -1110,13 +1109,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_SLPI>,
-					<&rpmhpd 3>,
+			power-domains = <&rpmhpd 3>,
 					<&rpmhpd 2>;
-			power-domain-names = "load_state", "lcx", "lmx";
+			power-domain-names = "lcx", "lmx";
 
 			memory-region = <&slpi_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&slpi_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -1560,13 +1560,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
-					<&rpmhpd 7>,
+			power-domains = <&rpmhpd 7>,
 					<&rpmhpd 0>;
-			power-domain-names = "load_state", "cx", "mss";
+			power-domain-names = "cx", "mss";
 
 			memory-region = <&mpss_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&modem_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -2186,12 +2187,12 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_CDSP>,
-					<&rpmhpd 7>;
-			power-domain-names = "load_state", "cx";
+			power-domains = <&rpmhpd 7>;
 
 			memory-region = <&cdsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&cdsp_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -2415,7 +2416,6 @@
 			mboxes = <&apss_shared 0>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 		};
 
 		tsens0: thermal-sensor@c263000 {
@@ -2562,12 +2562,12 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_LPASS>,
-					<&rpmhpd 7>;
-			power-domain-names = "load_state", "cx";
+			power-domains = <&rpmhpd 7>;
 
 			memory-region = <&adsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&adsp_smp2p_out 0>;
 			qcom,smem-state-names = "stop";
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#17 lore raw

PATCH v4 9/13 arm64: dts: qcom: sm8250: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SM8250 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
arch/arm64/boot/dts/qcom/sm8250.dtsi +11 -11
index 4798368b02ef..535b488575f1 100644
@@ -13,7 +13,6 @@
 #include <dt-bindings/interconnect/qcom,osm-l3.h>
 #include <dt-bindings/interconnect/qcom,sm8250.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,apr.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
@@ -1968,13 +1967,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_SLPI>,
-					<&rpmhpd SM8250_LCX>,
+			power-domains = <&rpmhpd SM8250_LCX>,
 					<&rpmhpd SM8250_LMX>;
-			power-domain-names = "load_state", "lcx", "lmx";
+			power-domain-names = "lcx", "lmx";
 
 			memory-region = <&slpi_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_slpi_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -2034,12 +2034,12 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_CDSP>,
-					<&rpmhpd SM8250_CX>;
-			power-domain-names = "load_state", "cx";
+			power-domains = <&rpmhpd SM8250_CX>;
 
 			memory-region = <&cdsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_cdsp_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -2782,7 +2782,6 @@
 					IPCC_MPROC_SIGNAL_GLINK_QMP>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 		};
 
 		spmi_bus: spmi@c440000 {
@@ -3699,13 +3698,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_LPASS>,
-					<&rpmhpd SM8250_LCX>,
+			power-domains = <&rpmhpd SM8250_LCX>,
 					<&rpmhpd SM8250_LMX>;
-			power-domain-names = "load_state", "lcx", "lmx";
+			power-domain-names = "lcx", "lmx";
 
 			memory-region = <&adsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_adsp_out 0>;
 			qcom,smem-state-names = "stop";
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#18 lore raw

PATCH v4 10/13 arm64: dts: qcom: sm8350: Use QMP property to control load state

In reply to Sibi Sankar

Use the Qualcomm Mailbox Protocol (QMP) property to control the load state resources on SM8350 SoCs and drop deprecated power-domains exposed by AOSS QMP node.

---
 arch/arm64/boot/dts/qcom/sm8350.dtsi | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)
arch/arm64/boot/dts/qcom/sm8350.dtsi +16 -14
index 0d16392bb976..2d78a55f33c2 100644
@@ -8,7 +8,6 @@
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/interconnect/qcom,sm8350.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
 #include <dt-bindings/thermal/thermal.h>
@@ -703,15 +702,16 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
-					<&rpmhpd 0>,
+			power-domains = <&rpmhpd 0>,
 					<&rpmhpd 12>;
-			power-domain-names = "load_state", "cx", "mss";
+			power-domain-names = "cx", "mss";
 
 			interconnects = <&mc_virt MASTER_LLCC &mc_virt SLAVE_EBI1>;
 
 			memory-region = <&pil_modem_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_modem_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -771,7 +771,6 @@
 			mboxes = <&ipcc IPCC_CLIENT_AOP IPCC_MPROC_SIGNAL_GLINK_QMP>;
 
 			#clock-cells = <0>;
-			#power-domain-cells = <1>;
 		};
 
 		spmi_bus: spmi@c440000 {
@@ -1070,13 +1069,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_SLPI>,
-					<&rpmhpd 4>,
+			power-domains = <&rpmhpd 4>,
 					<&rpmhpd 5>;
-			power-domain-names = "load_state", "lcx", "lmx";
+			power-domain-names = "lcx", "lmx";
 
 			memory-region = <&pil_slpi_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_slpi_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -1110,15 +1110,16 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_CDSP>,
-					<&rpmhpd 0>,
+			power-domains = <&rpmhpd 0>,
 					<&rpmhpd 10>;
-			power-domain-names = "load_state", "cx", "mxc";
+			power-domain-names = "cx", "mxc";
 
 			interconnects = <&compute_noc MASTER_CDSP_PROC &mc_virt SLAVE_EBI1>;
 
 			memory-region = <&pil_cdsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_cdsp_out 0>;
 			qcom,smem-state-names = "stop";
 
@@ -1344,13 +1345,14 @@
 			clocks = <&rpmhcc RPMH_CXO_CLK>;
 			clock-names = "xo";
 
-			power-domains = <&aoss_qmp AOSS_QMP_LS_LPASS>,
-					<&rpmhpd 4>,
+			power-domains = <&rpmhpd 4>,
 					<&rpmhpd 5>;
-			power-domain-names = "load_state", "lcx", "lmx";
+			power-domain-names = "lcx", "lmx";
 
 			memory-region = <&pil_adsp_mem>;
 
+			qcom,qmp = <&aoss_qmp>;
+
 			qcom,smem-states = <&smp2p_adsp_out 0>;
 			qcom,smem-state-names = "stop";
 
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#19 lore raw

PATCH v4 11/13 soc: qcom: aoss: Drop power domain support

In reply to Sibi Sankar

Strip out the load state power-domain support from the driver since the low power mode signalling for the co-processors is now accessible through the direct qmp message send interface.

---

v4:
* Commit message change and drop incorrect cleanup on cooling device probe failures. [Matthias]

 drivers/soc/qcom/qcom_aoss.c | 107 -------------------------------------------
 1 file changed, 107 deletions(-)
drivers/soc/qcom/qcom_aoss.c +0 -107
index 3f588921a7b1..dccc5d96fe0a 100644
@@ -2,7 +2,6 @@
 /*
  * Copyright (c) 2019, Linaro Ltd
  */
-#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <linux/clk-provider.h>
 #include <linux/debugfs.h>
 #include <linux/interrupt.h>
@@ -11,7 +10,6 @@
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
-#include <linux/pm_domain.h>
 #include <linux/thermal.h>
 #include <linux/slab.h>
 #include <linux/soc/qcom/qcom_aoss.h>
@@ -68,7 +66,6 @@ struct qmp_cooling_device {
  * @event: wait_queue for synchronization with the IRQ
  * @tx_lock: provides synchronization between multiple callers of qmp_send()
  * @qdss_clk: QDSS clock hw struct
- * @pd_data: genpd data
  * @cooling_devs: thermal cooling devices
  */
 struct qmp {
@@ -88,20 +85,12 @@ struct qmp {
 	struct mutex tx_lock;
 
 	struct clk_hw qdss_clk;
-	struct genpd_onecell_data pd_data;
 	struct qmp_cooling_device *cooling_devs;
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *debugfs_file;
 #endif /* CONFIG_DEBUG_FS */
 };
 
-struct qmp_pd {
-	struct qmp *qmp;
-	struct generic_pm_domain pd;
-};
-
-#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd)
-
 static void qmp_kick(struct qmp *qmp)
 {
 	mbox_send_message(qmp->mbox_chan, NULL);
@@ -330,95 +319,6 @@ static void qmp_qdss_clk_remove(struct qmp *qmp)
 	clk_hw_unregister(&qmp->qdss_clk);
 }
 
-static int qmp_pd_power_toggle(struct qmp_pd *res, bool enable)
-{
-	char buf[QMP_MSG_LEN] = {};
-
-	snprintf(buf, sizeof(buf),
-		 "{class: image, res: load_state, name: %s, val: %s}",
-		 res->pd.name, enable ? "on" : "off");
-	return qmp_send(res->qmp, buf, sizeof(buf));
-}
-
-static int qmp_pd_power_on(struct generic_pm_domain *domain)
-{
-	return qmp_pd_power_toggle(to_qmp_pd_resource(domain), true);
-}
-
-static int qmp_pd_power_off(struct generic_pm_domain *domain)
-{
-	return qmp_pd_power_toggle(to_qmp_pd_resource(domain), false);
-}
-
-static const char * const sdm845_resources[] = {
-	[AOSS_QMP_LS_CDSP] = "cdsp",
-	[AOSS_QMP_LS_LPASS] = "adsp",
-	[AOSS_QMP_LS_MODEM] = "modem",
-	[AOSS_QMP_LS_SLPI] = "slpi",
-	[AOSS_QMP_LS_SPSS] = "spss",
-	[AOSS_QMP_LS_VENUS] = "venus",
-};
-
-static int qmp_pd_add(struct qmp *qmp)
-{
-	struct genpd_onecell_data *data = &qmp->pd_data;
-	struct device *dev = qmp->dev;
-	struct qmp_pd *res;
-	size_t num = ARRAY_SIZE(sdm845_resources);
-	int ret;
-	int i;
-
-	res = devm_kcalloc(dev, num, sizeof(*res), GFP_KERNEL);
-	if (!res)
-		return -ENOMEM;
-
-	data->domains = devm_kcalloc(dev, num, sizeof(*data->domains),
-				     GFP_KERNEL);
-	if (!data->domains)
-		return -ENOMEM;
-
-	for (i = 0; i < num; i++) {
-		res[i].qmp = qmp;
-		res[i].pd.name = sdm845_resources[i];
-		res[i].pd.power_on = qmp_pd_power_on;
-		res[i].pd.power_off = qmp_pd_power_off;
-
-		ret = pm_genpd_init(&res[i].pd, NULL, true);
-		if (ret < 0) {
-			dev_err(dev, "failed to init genpd\n");
-			goto unroll_genpds;
-		}
-
-		data->domains[i] = &res[i].pd;
-	}
-
-	data->num_domains = i;
-
-	ret = of_genpd_add_provider_onecell(dev->of_node, data);
-	if (ret < 0)
-		goto unroll_genpds;
-
-	return 0;
-
-unroll_genpds:
-	for (i--; i >= 0; i--)
-		pm_genpd_remove(data->domains[i]);
-
-	return ret;
-}
-
-static void qmp_pd_remove(struct qmp *qmp)
-{
-	struct genpd_onecell_data *data = &qmp->pd_data;
-	struct device *dev = qmp->dev;
-	int i;
-
-	of_genpd_del_provider(dev->of_node);
-
-	for (i = 0; i < data->num_domains; i++)
-		pm_genpd_remove(data->domains[i]);
-}
-
 static int qmp_cdev_get_max_state(struct thermal_cooling_device *cdev,
 				  unsigned long *state)
 {
@@ -650,10 +550,6 @@ static int qmp_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_close_qmp;
 
-	ret = qmp_pd_add(qmp);
-	if (ret)
-		goto err_remove_qdss_clk;
-
 	ret = qmp_cooling_devices_register(qmp);
 	if (ret)
 		dev_err(&pdev->dev, "failed to register aoss cooling devices\n");
@@ -669,8 +565,6 @@ static int qmp_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_remove_qdss_clk:
-	qmp_qdss_clk_remove(qmp);
 err_close_qmp:
 	qmp_close(qmp);
 err_free_mbox:
@@ -689,7 +583,6 @@ static int qmp_remove(struct platform_device *pdev)
 #endif /* CONFIG_DEBUG_FS */
 
 	qmp_qdss_clk_remove(qmp);
-	qmp_pd_remove(qmp);
 	qmp_cooling_devices_remove(qmp);
 
 	qmp_close(qmp);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#22 lore raw

PATCH v4 12/13 dt-bindings: msm/dp: Remove aoss-qmp header

In reply to Sibi Sankar

Remove the unused aoss-qmp header from the list of includes.

---
 Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 1 -
 1 file changed, 1 deletion(-)
Documentation/devicetree/bindings/display/msm/dp-controller.yaml +0 -1
index 64d8d9e5e47a..d89b3c510c27 100644
@@ -95,7 +95,6 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/clock/qcom,dispcc-sc7180.h>
-    #include <dt-bindings/power/qcom-aoss-qmp.h>
     #include <dt-bindings/power/qcom-rpmpd.h>
 
     displayport-controller@ae90000 {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#24 lore raw

PATCH v4 13/13 dt-bindings: soc: qcom: aoss: Delete unused power-domain definitions

In reply to Sibi Sankar

Delete unused power-domain definitions exposed by AOSS QMP.

---
 include/dt-bindings/power/qcom-aoss-qmp.h | 14 --------------
 1 file changed, 14 deletions(-)
 delete mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
include/dt-bindings/power/qcom-aoss-qmp.h deleted +0 -14
deleted file mode 100644
index ec336d31dee4..000000000000
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (c) 2018, Linaro Ltd. */
-
-#ifndef __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
-#define __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
-
-#define AOSS_QMP_LS_CDSP		0
-#define AOSS_QMP_LS_LPASS	1
-#define AOSS_QMP_LS_MODEM	2
-#define AOSS_QMP_LS_SLPI		3
-#define AOSS_QMP_LS_SPSS		4
-#define AOSS_QMP_LS_VENUS	5
-
-#endif
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project