devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] usb: dwc3: qcom: Add support for firmware-managed resources
@ 2025-11-27 10:31 Sriram Dash
  2025-11-27 10:31 ` [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: " Sriram Dash
  2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
  0 siblings, 2 replies; 12+ messages in thread
From: Sriram Dash @ 2025-11-27 10:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Sriram Dash,
	Konrad Dybcio, Shazad Hussain

On Qualcomm automotive SoC sa8255p, platform resources like clocks,
interconnect, resets, regulators and GDSC are configured remotely by
firmware.

PM OPP is used to abstract these resources in firmware
and SCMI power protocol is used to request resource operations by using
runtime PM framework APIs such as pm_runtime_get/put_sync to invoke
power_on/_off calls from kernel respectively.

"qcom,snps-dwc3-fw-managed" compatible allows the driver to
determine if device resources are managed by firmware.
Additionally, it makes the power-domains property mandatory
and excludes the clocks property for the IPs where resources
are managed by firmware.

Macros are introduced to represent key lifecycle events:
initialization, system and runtime suspend/resume, and exit.
The driver sets the appropriate resource state during probe,
remove, suspend, and resume operations, enabling bulk ON/OFF
transitions of grouped resources according to the
controller's operational state.

Below architecture diagram explains the firmware managed resource
abstraction:

                                         +--------------------+
                                         |   Shared Memory    |
                                         |                    |
                                         | +----------------+ |       +--------------------+
  +----------------------------+       +-+->  usb-shmem     <-+---+   |    Linux VM        |
  |        Firmware VM         |       | | +----------------+ |   |   |   +----------+     |
  |                            |       | |                    |   |   |   |   USB    |     |
  | +---------+ f +----------+ |       | |                    |   |   |   |  Driver  |     |
  | |Drivers  <---+  SCMI    | |   e   | |         |          |   |   |   +--+----^--+     |
  | | (clks,  | g | Server   +-+-------+ |                    |   |   |      |    |        |
  | |  GDSC,  +--->          | |   h     |         |          |  b|k  |     a|   l|        |
  | |  gpio,  |   +--^-----+-+ |         |                    |   |   |      |    |        |
  | |  resets,|      |     |   |         |         |          |   |   |  +---v----+----+   |
  | |  etc.)  |      |     |   |         |                    |   +---+--+  USB SCMI   |   |
  | +---------+      |     |   |         |                    |       |  |  INSTANCE   |   |
  |                  |     |   |         |  +---------------+ |       |  +-^-----+-----+   |
  |                  |     |   |         |  |  pcie-shmem   | |       |    |     |         |
  +------------------+-----+---+         |  +---------------+ |       +----+-----+---------+
                     |     |             |                    |            |     |
                     |     |             +--------------------+            |     |
                    d|IRQ i|HVC                                           j|IRQ c|HVC
                     |     |                                               |     |
                     |     |                                               |     |
+--------------------+-----v-----------------------------------------------+-----v---------+
|                                              HYPERVISOR                                  |
+------------------------------------------------------------------------------------------+

     +--------+   +--------+                                           +----------+
     | CLOCK  |   |  Reset |                                           |   USB    |
     +--------+   +--------+                                           +----------+

Below architecture diagram explains the PM suspend and resume
sequences (the runtime suspend or resume also will follow similar
flow, using different levels to signal the firmware for managing
respective resources for the operational states):

PM Suspend Sequence

+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +----------+
|Kernel_PM  |    |DWC3_QCOM  |    |DWC3_CORE  |    |  PHY  |    |   PMOPP   |    | SCMI_Transport |    | Firmware |
+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +----------+
     |                |                |              |              |                  |                  |
     | Call suspend   |                |              |              |                  |                  |
     |--------------->|                |              |              |                  |                  |
     |                | Notify core    |              |              |                  |                  |
     |                | for suspend    |              |              |                  |                  |
     |                |--------------->|              |              |                  |                  |
     |                |                | Trigger PHY  |              |                  |                  |
     |                |                | exit         |              |                  |                  |
     |                |                |------------->|              |                  |                  |
     |                |                | Ack PHY exit |              |                  |                  |
     |                |                |<-------------|              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |  core Suspend  |              |              |                  |                  |
     |                |    complete    |              |              |                  |                  |
     |                |<---------------|              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |                | dev_pm_set_level(SYSTEM_SUSPEND)               |                  |
     |                |--------------------------------------------->|                  |                  |
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |                |              |              | Forward set_level|                  |
     |                |                |              |              | (channel A,      |                  |
     |                |                |              |              | domain B,        |                  |
     |                |                |              |              | SYSTEM_SUSPEND)  |                  |
     |                |                |              |              |----------------->|                  |
     |                |                |              |              |                  | Pass suspend info|
     |                |                |              |              |                  | (channel A,      |
     |                |                |              |              |                  | domain B,        |
     |                |                |              |              |                  | SYSTEM_SUSPEND)  |
     |                |                |              |              |                  |----------------->|
     |                |                |              |              |                  |                  | Switch off clocks
     |                |                |              |              |                  |                  |  interconnects,
     |                |                |              |              |                  |                  |  disable GDSC,
     |                |                |              |              |                  |                  |  disable VBUS
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  | Ack              |
     |                |                |              |              |                  |<-----------------|
     |                |                |              |              | Ack              |                  |
     |                |                |              |              |<-----------------|                  |
     |                |                |              | Ack          |                  |                  |
     |                |<---------------------------------------------|                  |                  |
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |<---------------|                |              |              |                  |                  |
     | Suspend        |                |              |              |                  |                  |
     | complete       |                |              |              |                  |                  |
+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +--------+

PM Resume Sequence

+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +----------+
|Kernel_PM  |    |DWC3_QCOM  |    |DWC3_CORE  |    |  PHY  |    |   PMOPP   |    | SCMI_Transport |    | Firmware |
+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +----------+
     |                |                |              |              |                  |                  |
     | Call resume    |                |              |              |                  |                  |
     |--------------->|                |              |              |                  |                  |
     |                | Notify core    |              |              |                  |                  |
     |                | for resume     |              |              |                  |                  |
     |                |--------------->|              |              |                  |                  |
     |                |                | Trigger PHY  |              |                  |                  |
     |                |                | init         |              |                  |                  |
     |                |                |------------->|              |                  |                  |
     |                |                | Ack PHY init |              |                  |                  |
     |                |                |<-------------|              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |  core Resume   |              |              |                  |                  |
     |                |    complete    |              |              |                  |                  |
     |                |<---------------|              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |                |                | dev_pm_set_level(SYSTEM_RESUME)                |                  |
     |                |--------------------------------------------->|                  |                  |
     |                |                |              |              | Forward set_level|                  |
     |                |                |              |              | (channel A,      |                  |
     |                |                |              |              | domain B,        |                  |
     |                |                |              |              | SYSTEM_RESUME)   |                  |
     |                |                |              |              |----------------->|                  |
     |                |                |              |              |                  | Pass resume info |
     |                |                |              |              |                  | (channel A,      |
     |                |                |              |              |                  | domain B,        |
     |                |                |              |              |                  | SYSTEM_RESUME)   |
     |                |                |              |              |                  |----------------->|
     |                |                |              |              |                  |                  | Enable VBUS,
     |                |                |              |              |                  |                  | enable GDSC,
     |                |                |              |              |                  |                  | switch ON clocks,
     |                |                |              |              |                  |                  | interconnects
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  | Ack              |
     |                |                |              |              |                  |<-----------------|
     |                |                |              |              | Ack              |                  |
     |                |                |              |              |<-----------------|                  |
     |                |                |              | Ack          |                  |                  |
     |                |<---------------------------------------------|                  |                  |
     |                |                |              |              |                  |                  |
     |                |                |              |              |                  |                  |
     |<---------------|                |              |              |                  |                  |
     | Resume         |                |              |              |                  |                  |
     | complete       |                |              |              |                  |                  |
+-----------+    +-----------+    +-----------+    +-------+    +-----------+    +----------------+    +--------+

Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
---
Depends-on: https://lore.kernel.org/all/20250422231249.871995-1-quic_djaggi@quicinc.com/

---
Sriram Dash (2):
      dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
      usb: dwc3: qcom: Support firmware-managed resource states for power management

 .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
 drivers/usb/dwc3/dwc3-qcom.c                       |  97 ++++++++++--
 2 files changed, 199 insertions(+), 71 deletions(-)
---
base-commit: c77a6544d8a2364e4bee1b52890f577be27b7296
change-id: 20251127-controller_scmi_upstream-a344bad23bcc

Best regards,
-- 
Sriram Dash <sriram.dash@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
  2025-11-27 10:31 [PATCH 0/2] usb: dwc3: qcom: Add support for firmware-managed resources Sriram Dash
@ 2025-11-27 10:31 ` Sriram Dash
  2025-11-27 11:29   ` Rob Herring (Arm)
  2025-11-27 12:13   ` Krzysztof Kozlowski
  2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
  1 sibling, 2 replies; 12+ messages in thread
From: Sriram Dash @ 2025-11-27 10:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Sriram Dash,
	Konrad Dybcio

On Qualcomm automotive SoC sa8255p, platform resources like clocks,
interconnect, resets, regulators and GDSC are configured remotely by
firmware.

PM OPP is used to abstract these resources in firmware and SCMI perf
protocol is used to request resource operations by using runtime PM
framework APIs such as pm_runtime_get/put_sync to signal firmware
for managing resources accordingly for respective perf levels.

"qcom,snps-dwc3-fw-managed" compatible helps determine if
the device's resources are managed by firmware.
Additionally, it makes the power-domains property mandatory
and excludes the clocks property for the controller.

Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
---
 .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
 1 file changed, 111 insertions(+), 62 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
index 8cee7c5582f2..d2d1b42fbb07 100644
--- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
@@ -12,68 +12,65 @@ maintainers:
 description:
   Describes the Qualcomm USB block, based on Synopsys DWC3.
 
-select:
-  properties:
-    compatible:
-      contains:
-        const: qcom,snps-dwc3
-  required:
-    - compatible
-
 properties:
   compatible:
-    items:
-      - enum:
-          - qcom,glymur-dwc3
-          - qcom,glymur-dwc3-mp
-          - qcom,ipq4019-dwc3
-          - qcom,ipq5018-dwc3
-          - qcom,ipq5332-dwc3
-          - qcom,ipq5424-dwc3
-          - qcom,ipq6018-dwc3
-          - qcom,ipq8064-dwc3
-          - qcom,ipq8074-dwc3
-          - qcom,ipq9574-dwc3
-          - qcom,kaanapali-dwc3
-          - qcom,milos-dwc3
-          - qcom,msm8953-dwc3
-          - qcom,msm8994-dwc3
-          - qcom,msm8996-dwc3
-          - qcom,msm8998-dwc3
-          - qcom,qcm2290-dwc3
-          - qcom,qcs404-dwc3
-          - qcom,qcs615-dwc3
-          - qcom,qcs8300-dwc3
-          - qcom,qdu1000-dwc3
-          - qcom,sa8775p-dwc3
-          - qcom,sar2130p-dwc3
-          - qcom,sc7180-dwc3
-          - qcom,sc7280-dwc3
-          - qcom,sc8180x-dwc3
-          - qcom,sc8180x-dwc3-mp
-          - qcom,sc8280xp-dwc3
-          - qcom,sc8280xp-dwc3-mp
-          - qcom,sdm660-dwc3
-          - qcom,sdm670-dwc3
-          - qcom,sdm845-dwc3
-          - qcom,sdx55-dwc3
-          - qcom,sdx65-dwc3
-          - qcom,sdx75-dwc3
-          - qcom,sm4250-dwc3
-          - qcom,sm6115-dwc3
-          - qcom,sm6125-dwc3
-          - qcom,sm6350-dwc3
-          - qcom,sm6375-dwc3
-          - qcom,sm8150-dwc3
-          - qcom,sm8250-dwc3
-          - qcom,sm8350-dwc3
-          - qcom,sm8450-dwc3
-          - qcom,sm8550-dwc3
-          - qcom,sm8650-dwc3
-          - qcom,sm8750-dwc3
-          - qcom,x1e80100-dwc3
-          - qcom,x1e80100-dwc3-mp
-      - const: qcom,snps-dwc3
+    oneOf:
+      - items:
+          - enum:
+              - qcom,glymur-dwc3
+              - qcom,glymur-dwc3-mp
+              - qcom,ipq4019-dwc3
+              - qcom,ipq5018-dwc3
+              - qcom,ipq5332-dwc3
+              - qcom,ipq5424-dwc3
+              - qcom,ipq6018-dwc3
+              - qcom,ipq8064-dwc3
+              - qcom,ipq8074-dwc3
+              - qcom,ipq9574-dwc3
+              - qcom,kaanapali-dwc3
+              - qcom,milos-dwc3
+              - qcom,msm8953-dwc3
+              - qcom,msm8994-dwc3
+              - qcom,msm8996-dwc3
+              - qcom,msm8998-dwc3
+              - qcom,qcm2290-dwc3
+              - qcom,qcs404-dwc3
+              - qcom,qcs615-dwc3
+              - qcom,qcs8300-dwc3
+              - qcom,qdu1000-dwc3
+              - qcom,sa8775p-dwc3
+              - qcom,sar2130p-dwc3
+              - qcom,sc7180-dwc3
+              - qcom,sc7280-dwc3
+              - qcom,sc8180x-dwc3
+              - qcom,sc8180x-dwc3-mp
+              - qcom,sc8280xp-dwc3
+              - qcom,sc8280xp-dwc3-mp
+              - qcom,sdm660-dwc3
+              - qcom,sdm670-dwc3
+              - qcom,sdm845-dwc3
+              - qcom,sdx55-dwc3
+              - qcom,sdx65-dwc3
+              - qcom,sdx75-dwc3
+              - qcom,sm4250-dwc3
+              - qcom,sm6115-dwc3
+              - qcom,sm6125-dwc3
+              - qcom,sm6350-dwc3
+              - qcom,sm6375-dwc3
+              - qcom,sm8150-dwc3
+              - qcom,sm8250-dwc3
+              - qcom,sm8350-dwc3
+              - qcom,sm8450-dwc3
+              - qcom,sm8550-dwc3
+              - qcom,sm8650-dwc3
+              - qcom,sm8750-dwc3
+              - qcom,x1e80100-dwc3
+              - qcom,x1e80100-dwc3-mp
+          - const: qcom,snps-dwc3
+      - items:
+          - enum:
+              - qcom,sa8255p-dwc3
+          - const: qcom,snps-dwc3-fw-managed
 
   reg:
     maxItems: 1
@@ -158,13 +155,31 @@ properties:
 required:
   - compatible
   - reg
-  - clocks
-  - clock-names
   - interrupts
   - interrupt-names
 
 allOf:
   - $ref: snps,dwc3-common.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,snps-dwc3
+    then:
+      required:
+        - clocks
+        - clock-names
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,snps-dwc3-fw-managed
+    then:
+      required:
+        - power-domains
+
   - if:
       properties:
         compatible:
@@ -513,6 +528,7 @@ allOf:
               - qcom,qcs615-dwc3
               - qcom,qcs8300-dwc3
               - qcom,qdu1000-dwc3
+              - qcom,sa8255p-dwc3
               - qcom,sa8775p-dwc3
               - qcom,sc7180-dwc3
               - qcom,sc7280-dwc3
@@ -657,4 +673,37 @@ examples:
             phy-names = "usb2-phy", "usb3-phy";
         };
     };
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb@a600000 {
+            compatible = "qcom,sa8255p-dwc3", "qcom,snps-dwc3-fw-managed";
+            reg = <0x0 0x0a800000 0x0 0x10000>;
+
+            interrupts-extended = <&intc GIC_SPI 349 IRQ_TYPE_LEVEL_HIGH>,
+                                  <&intc GIC_SPI 352 IRQ_TYPE_LEVEL_HIGH>,
+                                  <&intc GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH>,
+                                  <&pdc 8 IRQ_TYPE_EDGE_BOTH>,
+                                  <&pdc 7 IRQ_TYPE_EDGE_BOTH>,
+                                  <&pdc 13 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dwc_usb3",
+                              "pwr_event",
+                              "hs_phy_irq",
+                              "dp_hs_phy_irq",
+                              "dm_hs_phy_irq",
+                              "ss_phy_irq";
+
+            power-domains = <&scmi1_dvfs 0>;
+
+            iommus = <&apps_smmu 0x0a0 0x0>;
+            snps,dis_u2_susphy_quirk;
+            snps,dis_enblslpm_quirk;
+            phys = <&usb_1_hsphy>, <&usb_1_qmpphy>;
+            phy-names = "usb2-phy", "usb3-phy";
+        };
+    };
 ...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-11-27 10:31 [PATCH 0/2] usb: dwc3: qcom: Add support for firmware-managed resources Sriram Dash
  2025-11-27 10:31 ` [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: " Sriram Dash
@ 2025-11-27 10:31 ` Sriram Dash
  2025-11-27 12:14   ` Krzysztof Kozlowski
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Sriram Dash @ 2025-11-27 10:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Sriram Dash,
	Konrad Dybcio, Shazad Hussain

Add support for firmware-managed resource states in the
Qualcomm DWC3 USB controller driver. On platforms
like sa8255p, where controller resources are abstracted
and managed collectively by firmware, the driver communicates
power management transitions using dedicated resource state
levels via dev_pm_opp_set_level().

Macros are introduced to represent key lifecycle events:
initialization, system and runtime suspend/resume, and exit.
The driver sets the appropriate resource state during probe,
remove, suspend, and resume operations, enabling bulk ON/OFF
transitions of grouped resources according to the
controller's operational state.

Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
Co-developed-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
Signed-off-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 97 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 9ac75547820d..9615ca6cfcae 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -13,6 +13,8 @@
 #include <linux/kernel.h>
 #include <linux/interconnect.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
 #include <linux/phy/phy.h>
 #include <linux/usb/of.h>
 #include <linux/reset.h>
@@ -85,10 +87,48 @@ struct dwc3_qcom {
 	struct icc_path		*icc_path_apps;
 
 	enum usb_role		current_role;
+	bool			fw_managed;
 };
 
 #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
 
+/*
+ * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
+ *
+ * On select Qualcomm platforms, the USB controller’s power-related
+ * resources including GDSC, reset lines, clocks, and interconnects
+ * are managed collectively by system firmware via SCMI. The driver
+ * signals the controller’s operational state to firmware using these
+ * levels, each mapped to a specific power management transition or
+ * lifecycle event:
+ *
+ * DWC3_QCOM_FW_MANAGED_INIT
+ *	Enable GDSC, Assert and Deassert Resets, and turn ON all clocks
+ *	and interconnects.
+ *
+ * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
+ *	Enable GDSC and turn ON all clocks and interconnects.
+ *
+ * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
+ *	Turn ON all clocks and interconnects.
+ *
+ * DWC3_QCOM_FW_MANAGED_EXIT
+ *	Turn OFF all clocks and interconnects, Assert reset and disable GDSC.
+ *
+ * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
+ *	Turn OFF all clocks and interconnects and disable GDSC.
+ *
+ * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
+ *	Turn OFF clocks and interconnects.
+ */
+
+#define DWC3_QCOM_FW_MANAGED_INIT			1
+#define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME		2
+#define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME		3
+#define DWC3_QCOM_FW_MANAGED_EXIT			8
+#define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND		9
+#define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND		10
+
 static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
 {
 	u32 reg;
@@ -335,7 +375,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
 		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
 }
 
-static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
+static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
 {
 	u32 val;
 	int i, ret;
@@ -348,6 +388,13 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
 		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
 			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
 	}
+	if (qcom->fw_managed) {
+		if (PMSG_IS_AUTO(msg))
+			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND);
+		else
+			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND);
+	}
+
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 
 	ret = dwc3_qcom_interconnect_disable(qcom);
@@ -369,7 +416,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
 	return 0;
 }
 
-static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
+static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
 {
 	int ret;
 	int i;
@@ -380,6 +427,18 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
 	if (dwc3_qcom_is_host(qcom) && wakeup)
 		dwc3_qcom_disable_interrupts(qcom);
 
+	if (qcom->fw_managed) {
+		if (PMSG_IS_AUTO(msg))
+			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME);
+		else
+			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME);
+
+		if (ret < 0) {
+			dev_err(qcom->dev, "Failed to Resume fw managed device\n");
+			return ret;
+		}
+	}
+
 	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
 	if (ret < 0)
 		return ret;
@@ -624,10 +683,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 
 	qcom->dev = &pdev->dev;
 
+	qcom->fw_managed = device_get_match_data(dev);
+	if (qcom->fw_managed) {
+		ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_INIT);
+		if (ret < 0)
+			return ret;
+	}
+
 	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
 	if (IS_ERR(qcom->resets)) {
-		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
-				     "failed to get resets\n");
+		dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
+			      "failed to get resets\n");
+		goto resources_off;
 	}
 
 	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
@@ -638,7 +705,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	ret = reset_control_assert(qcom->resets);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
-		return ret;
+		goto resources_off;
 	}
 
 	usleep_range(10, 1000);
@@ -727,6 +794,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 clk_disable:
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 
+resources_off:
+	if (qcom->fw_managed)
+		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
+
 	return ret;
 }
 
@@ -739,6 +810,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
 		return;
 
 	dwc3_core_remove(&qcom->dwc);
+
+	if (qcom->fw_managed)
+		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
+
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 	dwc3_qcom_interconnect_exit(qcom);
 
@@ -756,7 +831,7 @@ static int dwc3_qcom_pm_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
-	ret = dwc3_qcom_suspend(qcom, wakeup);
+	ret = dwc3_qcom_suspend(qcom, wakeup, PMSG_SUSPEND);
 	if (ret)
 		return ret;
 
@@ -772,7 +847,7 @@ static int dwc3_qcom_pm_resume(struct device *dev)
 	bool wakeup = device_may_wakeup(dev);
 	int ret;
 
-	ret = dwc3_qcom_resume(qcom, wakeup);
+	ret = dwc3_qcom_resume(qcom, wakeup, PMSG_RESUME);
 	if (ret)
 		return ret;
 
@@ -809,7 +884,7 @@ static int dwc3_qcom_runtime_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
-	return dwc3_qcom_suspend(qcom, true);
+	return dwc3_qcom_suspend(qcom, true, PMSG_AUTO_SUSPEND);
 }
 
 static int dwc3_qcom_runtime_resume(struct device *dev)
@@ -818,7 +893,7 @@ static int dwc3_qcom_runtime_resume(struct device *dev)
 	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
 	int ret;
 
-	ret = dwc3_qcom_resume(qcom, true);
+	ret = dwc3_qcom_resume(qcom, true, PMSG_AUTO_RESUME);
 	if (ret)
 		return ret;
 
@@ -839,6 +914,10 @@ static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
 };
 
 static const struct of_device_id dwc3_qcom_of_match[] = {
+	{
+		.compatible	= "qcom,snps-dwc3-fw-managed",
+		.data		= (void *)true,
+	},
 	{ .compatible = "qcom,snps-dwc3" },
 	{ }
 };

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
  2025-11-27 10:31 ` [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: " Sriram Dash
@ 2025-11-27 11:29   ` Rob Herring (Arm)
  2025-11-27 12:13   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-11-27 11:29 UTC (permalink / raw)
  To: Sriram Dash
  Cc: krishna.kurapati, faisal.hassan, Greg Kroah-Hartman, devicetree,
	jack.pham, Konrad Dybcio, Wesley Cheng, linux-usb, andersson,
	Conor Dooley, Krzysztof Kozlowski, linux-kernel, linux-arm-msm,
	Thinh Nguyen


On Thu, 27 Nov 2025 16:01:44 +0530, Sriram Dash wrote:
> On Qualcomm automotive SoC sa8255p, platform resources like clocks,
> interconnect, resets, regulators and GDSC are configured remotely by
> firmware.
> 
> PM OPP is used to abstract these resources in firmware and SCMI perf
> protocol is used to request resource operations by using runtime PM
> framework APIs such as pm_runtime_get/put_sync to signal firmware
> for managing resources accordingly for respective perf levels.
> 
> "qcom,snps-dwc3-fw-managed" compatible helps determine if
> the device's resources are managed by firmware.
> Additionally, it makes the power-domains property mandatory
> and excludes the clocks property for the controller.
> 
> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
>  1 file changed, 111 insertions(+), 62 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): #address-cells: 1 was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): #size-cells: 0 was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): interrupt-names:0: 'dwc_usb3' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): interrupt-names:1: 'pwr_event' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): interrupt-names:2: 'hs_phy_irq' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): interrupt-names:3: 'dp_hs_phy_irq' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): interrupt-names:4: 'dm_hs_phy_irq' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): compatible: 'oneOf' conditional failed, one must be fixed:
	'qcom,sdm845-dwc3' is not one of ['qcom,sa8255p-dwc3']
	'qcom,snps-dwc3' was expected
	'qcom,snps-dwc3-fw-managed' was expected
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dtb: usb@a6f8800 (qcom,sdm845-dwc3): Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'ranges', 'usb@a600000' were unexpected)
	from schema $id: http://devicetree.org/schemas/usb/qcom,snps-dwc3.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251127-controller_scmi_upstream-v1-1-38bcca513c28@oss.qualcomm.com

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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
  2025-11-27 10:31 ` [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: " Sriram Dash
  2025-11-27 11:29   ` Rob Herring (Arm)
@ 2025-11-27 12:13   ` Krzysztof Kozlowski
  2025-11-27 13:56     ` Krzysztof Kozlowski
  2025-12-01 11:35     ` Sriram Dash
  1 sibling, 2 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 12:13 UTC (permalink / raw)
  To: Sriram Dash, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Konrad Dybcio

On 27/11/2025 11:31, Sriram Dash wrote:
> On Qualcomm automotive SoC sa8255p, platform resources like clocks,
> interconnect, resets, regulators and GDSC are configured remotely by
> firmware.
> 
> PM OPP is used to abstract these resources in firmware and SCMI perf
> protocol is used to request resource operations by using runtime PM
> framework APIs such as pm_runtime_get/put_sync to signal firmware
> for managing resources accordingly for respective perf levels.
> 
> "qcom,snps-dwc3-fw-managed" compatible helps determine if
> the device's resources are managed by firmware.
> Additionally, it makes the power-domains property mandatory
> and excludes the clocks property for the controller.
> 
> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
>  1 file changed, 111 insertions(+), 62 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> index 8cee7c5582f2..d2d1b42fbb07 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> @@ -12,68 +12,65 @@ maintainers:
>  description:
>    Describes the Qualcomm USB block, based on Synopsys DWC3.
>  
> -select:
> -  properties:
> -    compatible:
> -      contains:
> -        const: qcom,snps-dwc3
> -  required:
> -    - compatible

I wonder why do you think dropping some code is fine...


> +      - items:
> +          - enum:
> +              - qcom,sa8255p-dwc3
> +          - const: qcom,snps-dwc3-fw-managed

No, you cannot keep coming with more generic compatibles.

If you want generic a compatible, you already have - qcom,snps-dwc3 -
and that "generic" part already said that everything is compatible with it.

Now you claim that existing generic compatible qcom,snps-dwc3 is not
generic enough and you need one more generic compatible.

Next year you will say that two generic compatibles are not generic
enough and you need third generic compatible.

In two years we will learn that three generic compatibles are not enough...

I think I was complaining on the lists a lot on this, so I am surprised
it is still coming back.

So no, you cannot claim that you need more generic compatibles because
one generic is not generic. NAK.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
@ 2025-11-27 12:14   ` Krzysztof Kozlowski
  2025-12-02  5:21   ` Bjorn Andersson
  2025-12-09  8:48   ` Dan Carpenter
  2 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 12:14 UTC (permalink / raw)
  To: Sriram Dash, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Konrad Dybcio,
	Shazad Hussain

On 27/11/2025 11:31, Sriram Dash wrote:
> Add support for firmware-managed resource states in the
> Qualcomm DWC3 USB controller driver. On platforms
> like sa8255p, where controller resources are abstracted
> and managed collectively by firmware, the driver communicates
> power management transitions using dedicated resource state
> levels via dev_pm_opp_set_level().
> 
> Macros are introduced to represent key lifecycle events:
> initialization, system and runtime suspend/resume, and exit.
> The driver sets the appropriate resource state during probe,
> remove, suspend, and resume operations, enabling bulk ON/OFF
> transitions of grouped resources according to the
> controller's operational state.
> 
> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
> Co-developed-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
> Signed-off-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>


Messed order of tags. Please read carefully submitting patches, so you
understand what you certify before you actually certify.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
  2025-11-27 12:13   ` Krzysztof Kozlowski
@ 2025-11-27 13:56     ` Krzysztof Kozlowski
  2025-12-01 11:35     ` Sriram Dash
  1 sibling, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 13:56 UTC (permalink / raw)
  To: Sriram Dash, Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Konrad Dybcio

On 27/11/2025 13:13, Krzysztof Kozlowski wrote:
> On 27/11/2025 11:31, Sriram Dash wrote:
>> On Qualcomm automotive SoC sa8255p, platform resources like clocks,
>> interconnect, resets, regulators and GDSC are configured remotely by
>> firmware.
>>
>> PM OPP is used to abstract these resources in firmware and SCMI perf
>> protocol is used to request resource operations by using runtime PM
>> framework APIs such as pm_runtime_get/put_sync to signal firmware
>> for managing resources accordingly for respective perf levels.
>>
>> "qcom,snps-dwc3-fw-managed" compatible helps determine if
>> the device's resources are managed by firmware.
>> Additionally, it makes the power-domains property mandatory
>> and excludes the clocks property for the controller.
>>
>> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
>> ---
>>  .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
>>  1 file changed, 111 insertions(+), 62 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> index 8cee7c5582f2..d2d1b42fbb07 100644
>> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> @@ -12,68 +12,65 @@ maintainers:
>>  description:
>>    Describes the Qualcomm USB block, based on Synopsys DWC3.
>>  
>> -select:
>> -  properties:
>> -    compatible:
>> -      contains:
>> -        const: qcom,snps-dwc3
>> -  required:
>> -    - compatible
> 
> I wonder why do you think dropping some code is fine...
> 
> 
>> +      - items:
>> +          - enum:
>> +              - qcom,sa8255p-dwc3
>> +          - const: qcom,snps-dwc3-fw-managed
> 
> No, you cannot keep coming with more generic compatibles.
> 
> If you want generic a compatible, you already have - qcom,snps-dwc3 -
> and that "generic" part already said that everything is compatible with it.
> 
> Now you claim that existing generic compatible qcom,snps-dwc3 is not
> generic enough and you need one more generic compatible.
> 
> Next year you will say that two generic compatibles are not generic
> enough and you need third generic compatible.
> 
> In two years we will learn that three generic compatibles are not enough...
> 
> I think I was complaining on the lists a lot on this, so I am surprised
> it is still coming back.
> 
> So no, you cannot claim that you need more generic compatibles because
> one generic is not generic. NAK.


What is even weirder is that other patchset for the same SA8255 FW
managed IP block correctly ignored generic compatible:
https://lore.kernel.org/all/20251114145646.2291324-3-ram.dwivedi@oss.qualcomm.com/

so somehow you should meet and align on common understanding of things.
Please do not send completely different patches for the same problem.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: Add support for firmware-managed resources
  2025-11-27 12:13   ` Krzysztof Kozlowski
  2025-11-27 13:56     ` Krzysztof Kozlowski
@ 2025-12-01 11:35     ` Sriram Dash
  1 sibling, 0 replies; 12+ messages in thread
From: Sriram Dash @ 2025-12-01 11:35 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: jack.pham, faisal.hassan, krishna.kurapati, andersson,
	linux-arm-msm, linux-usb, devicetree, linux-kernel, Konrad Dybcio

On 11/27/2025 5:43 PM, Krzysztof Kozlowski wrote:
> On 27/11/2025 11:31, Sriram Dash wrote:
>> On Qualcomm automotive SoC sa8255p, platform resources like clocks,
>> interconnect, resets, regulators and GDSC are configured remotely by
>> firmware.
>>
>> PM OPP is used to abstract these resources in firmware and SCMI perf
>> protocol is used to request resource operations by using runtime PM
>> framework APIs such as pm_runtime_get/put_sync to signal firmware
>> for managing resources accordingly for respective perf levels.
>>
>> "qcom,snps-dwc3-fw-managed" compatible helps determine if
>> the device's resources are managed by firmware.
>> Additionally, it makes the power-domains property mandatory
>> and excludes the clocks property for the controller.
>>
>> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
>> ---
>>  .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    | 173 +++++++++++++--------
>>  1 file changed, 111 insertions(+), 62 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> index 8cee7c5582f2..d2d1b42fbb07 100644
>> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>> @@ -12,68 +12,65 @@ maintainers:
>>  description:
>>    Describes the Qualcomm USB block, based on Synopsys DWC3.
>>  
>> -select:
>> -  properties:
>> -    compatible:
>> -      contains:
>> -        const: qcom,snps-dwc3
>> -  required:
>> -    - compatible
> I wonder why do you think dropping some code is fine...
>
>
>> +      - items:
>> +          - enum:
>> +              - qcom,sa8255p-dwc3
>> +          - const: qcom,snps-dwc3-fw-managed
> No, you cannot keep coming with more generic compatibles.
>
> If you want generic a compatible, you already have - qcom,snps-dwc3 -
> and that "generic" part already said that everything is compatible with it.
>
> Now you claim that existing generic compatible qcom,snps-dwc3 is not
> generic enough and you need one more generic compatible.
>
> Next year you will say that two generic compatibles are not generic
> enough and you need third generic compatible.
>
> In two years we will learn that three generic compatibles are not enough...
>
> I think I was complaining on the lists a lot on this, so I am surprised
> it is still coming back.
>
> So no, you cannot claim that you need more generic compatibles because
> one generic is not generic. NAK.


Hi Krzysztof,

understood. Shall i make it platform specific then ? For example,

Say, For x1e80100, where platform resources are not managed by firmware,
use compatible = "qcom,x1e80100-dwc3", "qcom,snps-dwc3";

For Soc 8255p, where platform resources are managed by firmware, still
will use the generic compatible say,  compatible = "qcom,sa8255p-dwc3",
"qcom,snps-dwc3";


and in the driver, we will handle with the platform specific compatible.

static const struct of_device_id dwc3_qcom_of_match[] = {
        {
                .compatible     = "qcom,sa8255p-dwc3",
                .data           = (void *)true,
        },
        { .compatible = "qcom,snps-dwc3" },
        { }
};


For any other Soc where the resources are managed by firmware, we can
still reuse the compatible qcom,sa8255p-dwc3

say compatible = "qcom-foo-dwc3", "qcom,sa8255p-dwc3", "qcom,snps-dwc3";

compatible = "qcom-bar-dwc3", "qcom,sa8255p-dwc3", "qcom,snps-dwc3";

...


>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
  2025-11-27 12:14   ` Krzysztof Kozlowski
@ 2025-12-02  5:21   ` Bjorn Andersson
  2025-12-02  6:37     ` Sriram Dash
  2025-12-09  8:48   ` Dan Carpenter
  2 siblings, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2025-12-02  5:21 UTC (permalink / raw)
  To: Sriram Dash
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen, jack.pham,
	faisal.hassan, krishna.kurapati, linux-arm-msm, linux-usb,
	devicetree, linux-kernel, Konrad Dybcio, Shazad Hussain

On Thu, Nov 27, 2025 at 04:01:45PM +0530, Sriram Dash wrote:
> Add support for firmware-managed resource states in the
> Qualcomm DWC3 USB controller driver. On platforms
> like sa8255p, where controller resources are abstracted
> and managed collectively by firmware, the driver communicates
> power management transitions using dedicated resource state
> levels via dev_pm_opp_set_level().
> 
> Macros are introduced to represent key lifecycle events:
> initialization, system and runtime suspend/resume, and exit.
> The driver sets the appropriate resource state during probe,
> remove, suspend, and resume operations, enabling bulk ON/OFF
> transitions of grouped resources according to the
> controller's operational state.
> 
> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
> Co-developed-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
> Signed-off-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 97 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 9ac75547820d..9615ca6cfcae 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -13,6 +13,8 @@
>  #include <linux/kernel.h>
>  #include <linux/interconnect.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_opp.h>
>  #include <linux/phy/phy.h>
>  #include <linux/usb/of.h>
>  #include <linux/reset.h>
> @@ -85,10 +87,48 @@ struct dwc3_qcom {
>  	struct icc_path		*icc_path_apps;
>  
>  	enum usb_role		current_role;
> +	bool			fw_managed;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>  
> +/*
> + * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
> + *
> + * On select Qualcomm platforms, the USB controller’s power-related
> + * resources including GDSC, reset lines, clocks, and interconnects
> + * are managed collectively by system firmware via SCMI. The driver
> + * signals the controller’s operational state to firmware using these
> + * levels, each mapped to a specific power management transition or
> + * lifecycle event:
> + *
> + * DWC3_QCOM_FW_MANAGED_INIT

Both power and performance states are typically...states...
But these are actions/transitions between states.


The purpose of doing firmware assisted resource management (like done in
ACPI) is that it abstracts away the power management aspects from the OS
implementation, here we instead seems to complicate the OS
implementation.

> + *	Enable GDSC, Assert and Deassert Resets, and turn ON all clocks
> + *	and interconnects.
> + *
> + * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
> + *	Enable GDSC and turn ON all clocks and interconnects.
> + *
> + * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
> + *	Turn ON all clocks and interconnects.
> + *
> + * DWC3_QCOM_FW_MANAGED_EXIT
> + *	Turn OFF all clocks and interconnects, Assert reset and disable GDSC.
> + *
> + * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
> + *	Turn OFF all clocks and interconnects and disable GDSC.
> + *
> + * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
> + *	Turn OFF clocks and interconnects.
> + */
> +
> +#define DWC3_QCOM_FW_MANAGED_INIT			1
> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME		2
> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME		3

Given that dwc3_core_probe() calls pm_runtime_forbid(), do we actually
hit these states, or are you in practice only hitting some "D0" and "D3"
states?

Could this be simplified to match what we would need here for an ACPI
system?

Regards,
Bjorn

> +#define DWC3_QCOM_FW_MANAGED_EXIT			8
> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND		9
> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND		10
> +
>  static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
>  {
>  	u32 reg;
> @@ -335,7 +375,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
>  		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
>  }
>  
> -static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
>  {
>  	u32 val;
>  	int i, ret;
> @@ -348,6 +388,13 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>  		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
>  			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
>  	}
> +	if (qcom->fw_managed) {
> +		if (PMSG_IS_AUTO(msg))
> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND);
> +		else
> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND);
> +	}
> +
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
>  	ret = dwc3_qcom_interconnect_disable(qcom);
> @@ -369,7 +416,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>  	return 0;
>  }
>  
> -static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
> +static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
>  {
>  	int ret;
>  	int i;
> @@ -380,6 +427,18 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
>  	if (dwc3_qcom_is_host(qcom) && wakeup)
>  		dwc3_qcom_disable_interrupts(qcom);
>  
> +	if (qcom->fw_managed) {
> +		if (PMSG_IS_AUTO(msg))
> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME);
> +		else
> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME);
> +
> +		if (ret < 0) {
> +			dev_err(qcom->dev, "Failed to Resume fw managed device\n");
> +			return ret;
> +		}
> +	}
> +
>  	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
>  	if (ret < 0)
>  		return ret;
> @@ -624,10 +683,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  
>  	qcom->dev = &pdev->dev;
>  
> +	qcom->fw_managed = device_get_match_data(dev);
> +	if (qcom->fw_managed) {
> +		ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_INIT);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
>  	if (IS_ERR(qcom->resets)) {
> -		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> -				     "failed to get resets\n");
> +		dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> +			      "failed to get resets\n");
> +		goto resources_off;
>  	}
>  
>  	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
> @@ -638,7 +705,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	ret = reset_control_assert(qcom->resets);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
> -		return ret;
> +		goto resources_off;
>  	}
>  
>  	usleep_range(10, 1000);
> @@ -727,6 +794,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  clk_disable:
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
> +resources_off:
> +	if (qcom->fw_managed)
> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
> +
>  	return ret;
>  }
>  
> @@ -739,6 +810,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
>  		return;
>  
>  	dwc3_core_remove(&qcom->dwc);
> +
> +	if (qcom->fw_managed)
> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
> +
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  	dwc3_qcom_interconnect_exit(qcom);
>  
> @@ -756,7 +831,7 @@ static int dwc3_qcom_pm_suspend(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	ret = dwc3_qcom_suspend(qcom, wakeup);
> +	ret = dwc3_qcom_suspend(qcom, wakeup, PMSG_SUSPEND);
>  	if (ret)
>  		return ret;
>  
> @@ -772,7 +847,7 @@ static int dwc3_qcom_pm_resume(struct device *dev)
>  	bool wakeup = device_may_wakeup(dev);
>  	int ret;
>  
> -	ret = dwc3_qcom_resume(qcom, wakeup);
> +	ret = dwc3_qcom_resume(qcom, wakeup, PMSG_RESUME);
>  	if (ret)
>  		return ret;
>  
> @@ -809,7 +884,7 @@ static int dwc3_qcom_runtime_suspend(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	return dwc3_qcom_suspend(qcom, true);
> +	return dwc3_qcom_suspend(qcom, true, PMSG_AUTO_SUSPEND);
>  }
>  
>  static int dwc3_qcom_runtime_resume(struct device *dev)
> @@ -818,7 +893,7 @@ static int dwc3_qcom_runtime_resume(struct device *dev)
>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>  	int ret;
>  
> -	ret = dwc3_qcom_resume(qcom, true);
> +	ret = dwc3_qcom_resume(qcom, true, PMSG_AUTO_RESUME);
>  	if (ret)
>  		return ret;
>  
> @@ -839,6 +914,10 @@ static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>  };
>  
>  static const struct of_device_id dwc3_qcom_of_match[] = {
> +	{
> +		.compatible	= "qcom,snps-dwc3-fw-managed",
> +		.data		= (void *)true,
> +	},
>  	{ .compatible = "qcom,snps-dwc3" },
>  	{ }
>  };
> 
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-12-02  5:21   ` Bjorn Andersson
@ 2025-12-02  6:37     ` Sriram Dash
  2025-12-02 17:17       ` Bjorn Andersson
  0 siblings, 1 reply; 12+ messages in thread
From: Sriram Dash @ 2025-12-02  6:37 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen, jack.pham,
	faisal.hassan, krishna.kurapati, linux-arm-msm, linux-usb,
	devicetree, linux-kernel, Konrad Dybcio, Shazad Hussain

On 12/2/2025 10:51 AM, Bjorn Andersson wrote:
> On Thu, Nov 27, 2025 at 04:01:45PM +0530, Sriram Dash wrote:
>> Add support for firmware-managed resource states in the
>> Qualcomm DWC3 USB controller driver. On platforms
>> like sa8255p, where controller resources are abstracted
>> and managed collectively by firmware, the driver communicates
>> power management transitions using dedicated resource state
>> levels via dev_pm_opp_set_level().
>>
>> Macros are introduced to represent key lifecycle events:
>> initialization, system and runtime suspend/resume, and exit.
>> The driver sets the appropriate resource state during probe,
>> remove, suspend, and resume operations, enabling bulk ON/OFF
>> transitions of grouped resources according to the
>> controller's operational state.
>>
>> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
>> Co-developed-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
>> Signed-off-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
>> ---
>>  drivers/usb/dwc3/dwc3-qcom.c | 97 ++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 88 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index 9ac75547820d..9615ca6cfcae 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/kernel.h>
>>  #include <linux/interconnect.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_opp.h>
>>  #include <linux/phy/phy.h>
>>  #include <linux/usb/of.h>
>>  #include <linux/reset.h>
>> @@ -85,10 +87,48 @@ struct dwc3_qcom {
>>  	struct icc_path		*icc_path_apps;
>>  
>>  	enum usb_role		current_role;
>> +	bool			fw_managed;
>>  };
>>  
>>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>>  
>> +/*
>> + * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
>> + *
>> + * On select Qualcomm platforms, the USB controller’s power-related
>> + * resources including GDSC, reset lines, clocks, and interconnects
>> + * are managed collectively by system firmware via SCMI. The driver
>> + * signals the controller’s operational state to firmware using these
>> + * levels, each mapped to a specific power management transition or
>> + * lifecycle event:
>> + *
>> + * DWC3_QCOM_FW_MANAGED_INIT
> Both power and performance states are typically...states...
> But these are actions/transitions between states.
>
>
> The purpose of doing firmware assisted resource management (like done in
> ACPI) is that it abstracts away the power management aspects from the OS
> implementation, here we instead seems to complicate the OS
> implementation.
>
>> + *	Enable GDSC, Assert and Deassert Resets, and turn ON all clocks
>> + *	and interconnects.
>> + *
>> + * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
>> + *	Enable GDSC and turn ON all clocks and interconnects.
>> + *
>> + * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
>> + *	Turn ON all clocks and interconnects.
>> + *
>> + * DWC3_QCOM_FW_MANAGED_EXIT
>> + *	Turn OFF all clocks and interconnects, Assert reset and disable GDSC.
>> + *
>> + * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
>> + *	Turn OFF all clocks and interconnects and disable GDSC.
>> + *
>> + * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
>> + *	Turn OFF clocks and interconnects.
>> + */
>> +
>> +#define DWC3_QCOM_FW_MANAGED_INIT			1
>> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME		2
>> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME		3
> Given that dwc3_core_probe() calls pm_runtime_forbid(), do we actually
> hit these states, or are you in practice only hitting some "D0" and "D3"
> states?
>
> Could this be simplified to match what we would need here for an ACPI
> system?


Hi Bjorn,

thanks for the comments.

You’re right that the wording in the comment makes these look like
explicit “do X/Y/Z now” transitions rather than passive states. The
intention is not to expose an imperative sequence to firmware, but to
advertise a small set of abstract “resource configurations” that
correspond to specific OS power‑management contexts in the driver.

On sa8255p, the USB controller and its associated resources (GDSC,
clocks, interconnects, resets) are grouped behind a single
firmware‑managed perf domain. From the driver’s perspective we only have
a few meaningful configurations:

initial bring‑up during probe,
system suspend / system resume,
runtime suspend / runtime resume (planned once runtime PM is enabled), and
final shutdown on remove.

The levels are meant to encode which phase of the lifecycle we are in,
so that firmware can choose an internal representation that matches its
own notion of “D0‑like”, “temporarily suspended” or “off / removed”,
including any differences in how aggressively it can drop power, assert
resets, or preserve context.

You are correct that INIT and the various RESUME levels are all “on” in
the sense that the controller ends up operational, and similarly EXIT /
SUSPEND variants are “off / not actively used”. Today the driver does
not try to model these as strict D0/D3/D3hot/D3cold equivalents, because:

INIT may require a more complete bring‑up after boot, where firmware
might need to perform extra initialization compared to a resume from a
prior suspended state.
SYSTEM_* vs RUNTIME_* are tied to the OS‑level PM entry points
(dwc3_qcom_suspend() is used for both system and runtime suspend;
runtime is currently forbidden but it is planned later). The distinction
gives firmware the option to use different policies for system sleep vs
runtime idle, including wake‑capability and context‑retention.
That said, I agree that the current comment over‑specifies the concrete
actions (“Enable GDSC, Assert and Deassert Resets…”) and makes the
interface look more complicated than it actually is.

We can reword it to describe the effective resource state, without
prescribing exactly how the firmware should sequence GDSC, resets and
clocks. However, I’d still like to keep the separation between system
and runtime paths so that we don’t have to extend the protocol again
when runtime PM is enabled.

/*
 * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
 *
 * On select Qualcomm platforms, the USB controller’s power-related
 * resources (such as GDSC, reset lines, clocks, and interconnects)
 * are managed collectively by system firmware. The driver reports
 * the controller’s lifecycle and power-management context using the
 * following abstract resource state levels. The exact sequencing and
 * choice of underlying resources for each level is left to firmware.
 *
 * DWC3_QCOM_FW_MANAGED_INIT
 *    Controller is initialized after probe and brought into a fully
 *    operational state suitable for normal use.
 *
 * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
 *    Controller returns from system suspend to a fully operational
 *    state suitable for normal use.
 *
 * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
 *    Controller returns from runtime suspend to an operational state
 *    sufficient for runtime activity.
 *
 * DWC3_QCOM_FW_MANAGED_EXIT
 *    Controller is shut down as part of driver removal and may be put
 *    into a fully powered-off state with no requirement for retention.
 *
 * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
 *    Controller is quiesced for system suspend; resources may be
 *    reduced or powered down according to platform policy.
 *
 * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
 *    Controller is quiesced for runtime suspend; a lower-power state
 *    is entered while allowing a later runtime resume.
 */
#define DWC3_QCOM_FW_MANAGED_INIT            1
#define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME        2
#define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME        3
#define DWC3_QCOM_FW_MANAGED_EXIT            8
#define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND        9
#define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND        10


Let me know if this is OK.



> Regards,
> Bjorn
>
>> +#define DWC3_QCOM_FW_MANAGED_EXIT			8
>> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND		9
>> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND		10
>> +
>>  static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
>>  {
>>  	u32 reg;
>> @@ -335,7 +375,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
>>  		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
>>  }
>>  
>> -static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
>>  {
>>  	u32 val;
>>  	int i, ret;
>> @@ -348,6 +388,13 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>>  		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
>>  			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
>>  	}
>> +	if (qcom->fw_managed) {
>> +		if (PMSG_IS_AUTO(msg))
>> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND);
>> +		else
>> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND);
>> +	}
>> +
>>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>>  
>>  	ret = dwc3_qcom_interconnect_disable(qcom);
>> @@ -369,7 +416,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>>  	return 0;
>>  }
>>  
>> -static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
>> +static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
>>  {
>>  	int ret;
>>  	int i;
>> @@ -380,6 +427,18 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
>>  	if (dwc3_qcom_is_host(qcom) && wakeup)
>>  		dwc3_qcom_disable_interrupts(qcom);
>>  
>> +	if (qcom->fw_managed) {
>> +		if (PMSG_IS_AUTO(msg))
>> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME);
>> +		else
>> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME);
>> +
>> +		if (ret < 0) {
>> +			dev_err(qcom->dev, "Failed to Resume fw managed device\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>>  	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
>>  	if (ret < 0)
>>  		return ret;
>> @@ -624,10 +683,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>  
>>  	qcom->dev = &pdev->dev;
>>  
>> +	qcom->fw_managed = device_get_match_data(dev);
>> +	if (qcom->fw_managed) {
>> +		ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_INIT);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>>  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
>>  	if (IS_ERR(qcom->resets)) {
>> -		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
>> -				     "failed to get resets\n");
>> +		dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
>> +			      "failed to get resets\n");
>> +		goto resources_off;
>>  	}
>>  
>>  	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
>> @@ -638,7 +705,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>  	ret = reset_control_assert(qcom->resets);
>>  	if (ret) {
>>  		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
>> -		return ret;
>> +		goto resources_off;
>>  	}
>>  
>>  	usleep_range(10, 1000);
>> @@ -727,6 +794,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>  clk_disable:
>>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>>  
>> +resources_off:
>> +	if (qcom->fw_managed)
>> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
>> +
>>  	return ret;
>>  }
>>  
>> @@ -739,6 +810,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
>>  		return;
>>  
>>  	dwc3_core_remove(&qcom->dwc);
>> +
>> +	if (qcom->fw_managed)
>> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
>> +
>>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>>  	dwc3_qcom_interconnect_exit(qcom);
>>  
>> @@ -756,7 +831,7 @@ static int dwc3_qcom_pm_suspend(struct device *dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	ret = dwc3_qcom_suspend(qcom, wakeup);
>> +	ret = dwc3_qcom_suspend(qcom, wakeup, PMSG_SUSPEND);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -772,7 +847,7 @@ static int dwc3_qcom_pm_resume(struct device *dev)
>>  	bool wakeup = device_may_wakeup(dev);
>>  	int ret;
>>  
>> -	ret = dwc3_qcom_resume(qcom, wakeup);
>> +	ret = dwc3_qcom_resume(qcom, wakeup, PMSG_RESUME);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -809,7 +884,7 @@ static int dwc3_qcom_runtime_suspend(struct device *dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	return dwc3_qcom_suspend(qcom, true);
>> +	return dwc3_qcom_suspend(qcom, true, PMSG_AUTO_SUSPEND);
>>  }
>>  
>>  static int dwc3_qcom_runtime_resume(struct device *dev)
>> @@ -818,7 +893,7 @@ static int dwc3_qcom_runtime_resume(struct device *dev)
>>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>>  	int ret;
>>  
>> -	ret = dwc3_qcom_resume(qcom, true);
>> +	ret = dwc3_qcom_resume(qcom, true, PMSG_AUTO_RESUME);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -839,6 +914,10 @@ static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>>  };
>>  
>>  static const struct of_device_id dwc3_qcom_of_match[] = {
>> +	{
>> +		.compatible	= "qcom,snps-dwc3-fw-managed",
>> +		.data		= (void *)true,
>> +	},
>>  	{ .compatible = "qcom,snps-dwc3" },
>>  	{ }
>>  };
>>
>> -- 
>> 2.34.1
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-12-02  6:37     ` Sriram Dash
@ 2025-12-02 17:17       ` Bjorn Andersson
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Andersson @ 2025-12-02 17:17 UTC (permalink / raw)
  To: Sriram Dash
  Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Wesley Cheng, Thinh Nguyen, jack.pham,
	faisal.hassan, krishna.kurapati, linux-arm-msm, linux-usb,
	devicetree, linux-kernel, Konrad Dybcio, Shazad Hussain

On Tue, Dec 02, 2025 at 12:07:01PM +0530, Sriram Dash wrote:
> On 12/2/2025 10:51 AM, Bjorn Andersson wrote:
> > On Thu, Nov 27, 2025 at 04:01:45PM +0530, Sriram Dash wrote:
> >> Add support for firmware-managed resource states in the
> >> Qualcomm DWC3 USB controller driver. On platforms
> >> like sa8255p, where controller resources are abstracted
> >> and managed collectively by firmware, the driver communicates
> >> power management transitions using dedicated resource state
> >> levels via dev_pm_opp_set_level().
> >>
> >> Macros are introduced to represent key lifecycle events:
> >> initialization, system and runtime suspend/resume, and exit.
> >> The driver sets the appropriate resource state during probe,
> >> remove, suspend, and resume operations, enabling bulk ON/OFF
> >> transitions of grouped resources according to the
> >> controller's operational state.
> >>
> >> Signed-off-by: Sriram Dash <sriram.dash@oss.qualcomm.com>
> >> Co-developed-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
> >> Signed-off-by: Shazad Hussain <shazad.hussain@oss.qualcomm.com>
> >> ---
> >>  drivers/usb/dwc3/dwc3-qcom.c | 97 ++++++++++++++++++++++++++++++++++++++++----
> >>  1 file changed, 88 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> >> index 9ac75547820d..9615ca6cfcae 100644
> >> --- a/drivers/usb/dwc3/dwc3-qcom.c
> >> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> >> @@ -13,6 +13,8 @@
> >>  #include <linux/kernel.h>
> >>  #include <linux/interconnect.h>
> >>  #include <linux/platform_device.h>
> >> +#include <linux/pm_domain.h>
> >> +#include <linux/pm_opp.h>
> >>  #include <linux/phy/phy.h>
> >>  #include <linux/usb/of.h>
> >>  #include <linux/reset.h>
> >> @@ -85,10 +87,48 @@ struct dwc3_qcom {
> >>  	struct icc_path		*icc_path_apps;
> >>  
> >>  	enum usb_role		current_role;
> >> +	bool			fw_managed;
> >>  };
> >>  
> >>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> >>  
> >> +/*
> >> + * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
> >> + *
> >> + * On select Qualcomm platforms, the USB controller’s power-related
> >> + * resources including GDSC, reset lines, clocks, and interconnects
> >> + * are managed collectively by system firmware via SCMI. The driver
> >> + * signals the controller’s operational state to firmware using these
> >> + * levels, each mapped to a specific power management transition or
> >> + * lifecycle event:
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_INIT
> > Both power and performance states are typically...states...
> > But these are actions/transitions between states.
> >
> >
> > The purpose of doing firmware assisted resource management (like done in
> > ACPI) is that it abstracts away the power management aspects from the OS
> > implementation, here we instead seems to complicate the OS
> > implementation.
> >
> >> + *	Enable GDSC, Assert and Deassert Resets, and turn ON all clocks
> >> + *	and interconnects.
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
> >> + *	Enable GDSC and turn ON all clocks and interconnects.
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
> >> + *	Turn ON all clocks and interconnects.
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_EXIT
> >> + *	Turn OFF all clocks and interconnects, Assert reset and disable GDSC.
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
> >> + *	Turn OFF all clocks and interconnects and disable GDSC.
> >> + *
> >> + * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
> >> + *	Turn OFF clocks and interconnects.
> >> + */
> >> +
> >> +#define DWC3_QCOM_FW_MANAGED_INIT			1
> >> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME		2
> >> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME		3
> > Given that dwc3_core_probe() calls pm_runtime_forbid(), do we actually
> > hit these states, or are you in practice only hitting some "D0" and "D3"
> > states?
> >
> > Could this be simplified to match what we would need here for an ACPI
> > system?
> 
> 
> Hi Bjorn,
> 
> thanks for the comments.
> 
> You’re right that the wording in the comment makes these look like
> explicit “do X/Y/Z now” transitions rather than passive states. The
> intention is not to expose an imperative sequence to firmware, but to
> advertise a small set of abstract “resource configurations” that
> correspond to specific OS power‑management contexts in the driver.
> 

The problem I have is that when you talk about "states" here, you have
the Linux device's runtime and/or system state (active vs idle vs
suspended), or you're talking about the state of a clock, a regulator,
etc (on vs off, perhaps "on at X Hz"), or a performance state (such as 11).

But the states you're talking about is "the state of changing from
active to idle" (which in the other model is the edges between states).

> On sa8255p, the USB controller and its associated resources (GDSC,
> clocks, interconnects, resets) are grouped behind a single
> firmware‑managed perf domain. From the driver’s perspective we only have
> a few meaningful configurations:
> 
> initial bring‑up during probe,
> system suspend / system resume,
> runtime suspend / runtime resume (planned once runtime PM is enabled), and
> final shutdown on remove.
> 
> The levels are meant to encode which phase of the lifecycle we are in,
> so that firmware can choose an internal representation that matches its
> own notion of “D0‑like”, “temporarily suspended” or “off / removed”,
> including any differences in how aggressively it can drop power, assert
> resets, or preserve context.
> 
> You are correct that INIT and the various RESUME levels are all “on” in
> the sense that the controller ends up operational, and similarly EXIT /
> SUSPEND variants are “off / not actively used”. Today the driver does
> not try to model these as strict D0/D3/D3hot/D3cold equivalents, because:
> 
> INIT may require a more complete bring‑up after boot, where firmware
> might need to perform extra initialization compared to a resume from a
> prior suspended state.

But technically, the driver doesn't know what resource state we're in at
probe time. UEFI might have performed initialization already, it might
have turned the controller off, it might have left it in the shallow
state for some reason.

So it seems to me that exposing this as ON/OFF/"shallow-off" to the OS,
and then have the firmware track the state and perform the adequate
transition would be better.

> SYSTEM_* vs RUNTIME_* are tied to the OS‑level PM entry points
> (dwc3_qcom_suspend() is used for both system and runtime suspend;
> runtime is currently forbidden but it is planned later). The distinction
> gives firmware the option to use different policies for system sleep vs
> runtime idle, including wake‑capability and context‑retention.

The wake capability is an interesting topic, because this would
generally be considered a policy decision presented to the OS (to user
space inf act), not a decision encoded in the firmware. I'm not sure how
we would expose that decision through this interface.

> That said, I agree that the current comment over‑specifies the concrete
> actions (“Enable GDSC, Assert and Deassert Resets…”) and makes the
> interface look more complicated than it actually is.
> 

I'm not concerned about the complexity of the operations abstracted away
by these signals, Linux has no expectations of the complexity of a
typical D3->D0 transition.

> We can reword it to describe the effective resource state, without
> prescribing exactly how the firmware should sequence GDSC, resets and
> clocks. However, I’d still like to keep the separation between system
> and runtime paths so that we don’t have to extend the protocol again
> when runtime PM is enabled.
> 
> /*
>  * QCOM DWC3 USB Controller: Firmware-Managed Resource State Levels
>  *
>  * On select Qualcomm platforms, the USB controller’s power-related
>  * resources (such as GDSC, reset lines, clocks, and interconnects)
>  * are managed collectively by system firmware. The driver reports
>  * the controller’s lifecycle and power-management context using the
>  * following abstract resource state levels. The exact sequencing and
>  * choice of underlying resources for each level is left to firmware.
>  *
>  * DWC3_QCOM_FW_MANAGED_INIT
>  *    Controller is initialized after probe and brought into a fully
>  *    operational state suitable for normal use.
>  *
>  * DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME
>  *    Controller returns from system suspend to a fully operational
>  *    state suitable for normal use.
>  *
>  * DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME
>  *    Controller returns from runtime suspend to an operational state
>  *    sufficient for runtime activity.
>  *
>  * DWC3_QCOM_FW_MANAGED_EXIT
>  *    Controller is shut down as part of driver removal and may be put
>  *    into a fully powered-off state with no requirement for retention.
>  *
>  * DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND
>  *    Controller is quiesced for system suspend; resources may be
>  *    reduced or powered down according to platform policy.
>  *
>  * DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND
>  *    Controller is quiesced for runtime suspend; a lower-power state
>  *    is entered while allowing a later runtime resume.
>  */
> #define DWC3_QCOM_FW_MANAGED_INIT            1
> #define DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME        2
> #define DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME        3
> #define DWC3_QCOM_FW_MANAGED_EXIT            8
> #define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND        9
> #define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND        10
> 
> 
> Let me know if this is OK.

My concern remains, that these are explained as "states", but once you
enter any of the states "init", "system_resume", or "runtime_resume" I
expect the hardware to be in some particular configuration (a state).

It is true that we're trying to convey the "state change" (an action) in
the Linux device's power model to the firmware, so I understand why
you're communicating an "action" in each step, but you're doing that by
taking a performance domain to a particular "state".

I.e. you're using the performance state selection as a messaging
mechanism.

Regards,
Bjorn

> 
> 
> 
> > Regards,
> > Bjorn
> >
> >> +#define DWC3_QCOM_FW_MANAGED_EXIT			8
> >> +#define DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND		9
> >> +#define DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND		10
> >> +
> >>  static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
> >>  {
> >>  	u32 reg;
> >> @@ -335,7 +375,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> >>  		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
> >>  }
> >>  
> >> -static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> >> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
> >>  {
> >>  	u32 val;
> >>  	int i, ret;
> >> @@ -348,6 +388,13 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> >>  		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
> >>  			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
> >>  	}
> >> +	if (qcom->fw_managed) {
> >> +		if (PMSG_IS_AUTO(msg))
> >> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_SUSPEND);
> >> +		else
> >> +			dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_SUSPEND);
> >> +	}
> >> +
> >>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
> >>  
> >>  	ret = dwc3_qcom_interconnect_disable(qcom);
> >> @@ -369,7 +416,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> >>  	return 0;
> >>  }
> >>  
> >> -static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
> >> +static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup, pm_message_t msg)
> >>  {
> >>  	int ret;
> >>  	int i;
> >> @@ -380,6 +427,18 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
> >>  	if (dwc3_qcom_is_host(qcom) && wakeup)
> >>  		dwc3_qcom_disable_interrupts(qcom);
> >>  
> >> +	if (qcom->fw_managed) {
> >> +		if (PMSG_IS_AUTO(msg))
> >> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_RUNTIME_RESUME);
> >> +		else
> >> +			ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_SYSTEM_RESUME);
> >> +
> >> +		if (ret < 0) {
> >> +			dev_err(qcom->dev, "Failed to Resume fw managed device\n");
> >> +			return ret;
> >> +		}
> >> +	}
> >> +
> >>  	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
> >>  	if (ret < 0)
> >>  		return ret;
> >> @@ -624,10 +683,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
> >>  
> >>  	qcom->dev = &pdev->dev;
> >>  
> >> +	qcom->fw_managed = device_get_match_data(dev);
> >> +	if (qcom->fw_managed) {
> >> +		ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_INIT);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +	}
> >> +
> >>  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
> >>  	if (IS_ERR(qcom->resets)) {
> >> -		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> >> -				     "failed to get resets\n");
> >> +		dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> >> +			      "failed to get resets\n");
> >> +		goto resources_off;
> >>  	}
> >>  
> >>  	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
> >> @@ -638,7 +705,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
> >>  	ret = reset_control_assert(qcom->resets);
> >>  	if (ret) {
> >>  		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
> >> -		return ret;
> >> +		goto resources_off;
> >>  	}
> >>  
> >>  	usleep_range(10, 1000);
> >> @@ -727,6 +794,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
> >>  clk_disable:
> >>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
> >>  
> >> +resources_off:
> >> +	if (qcom->fw_managed)
> >> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
> >> +
> >>  	return ret;
> >>  }
> >>  
> >> @@ -739,6 +810,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
> >>  		return;
> >>  
> >>  	dwc3_core_remove(&qcom->dwc);
> >> +
> >> +	if (qcom->fw_managed)
> >> +		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
> >> +
> >>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
> >>  	dwc3_qcom_interconnect_exit(qcom);
> >>  
> >> @@ -756,7 +831,7 @@ static int dwc3_qcom_pm_suspend(struct device *dev)
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> -	ret = dwc3_qcom_suspend(qcom, wakeup);
> >> +	ret = dwc3_qcom_suspend(qcom, wakeup, PMSG_SUSPEND);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> @@ -772,7 +847,7 @@ static int dwc3_qcom_pm_resume(struct device *dev)
> >>  	bool wakeup = device_may_wakeup(dev);
> >>  	int ret;
> >>  
> >> -	ret = dwc3_qcom_resume(qcom, wakeup);
> >> +	ret = dwc3_qcom_resume(qcom, wakeup, PMSG_RESUME);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> @@ -809,7 +884,7 @@ static int dwc3_qcom_runtime_suspend(struct device *dev)
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> -	return dwc3_qcom_suspend(qcom, true);
> >> +	return dwc3_qcom_suspend(qcom, true, PMSG_AUTO_SUSPEND);
> >>  }
> >>  
> >>  static int dwc3_qcom_runtime_resume(struct device *dev)
> >> @@ -818,7 +893,7 @@ static int dwc3_qcom_runtime_resume(struct device *dev)
> >>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> >>  	int ret;
> >>  
> >> -	ret = dwc3_qcom_resume(qcom, true);
> >> +	ret = dwc3_qcom_resume(qcom, true, PMSG_AUTO_RESUME);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> @@ -839,6 +914,10 @@ static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
> >>  };
> >>  
> >>  static const struct of_device_id dwc3_qcom_of_match[] = {
> >> +	{
> >> +		.compatible	= "qcom,snps-dwc3-fw-managed",
> >> +		.data		= (void *)true,
> >> +	},
> >>  	{ .compatible = "qcom,snps-dwc3" },
> >>  	{ }
> >>  };
> >>
> >> -- 
> >> 2.34.1
> >>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
  2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
  2025-11-27 12:14   ` Krzysztof Kozlowski
  2025-12-02  5:21   ` Bjorn Andersson
@ 2025-12-09  8:48   ` Dan Carpenter
  2 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2025-12-09  8:48 UTC (permalink / raw)
  To: oe-kbuild, Sriram Dash, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Wesley Cheng, Thinh Nguyen
  Cc: lkp, oe-kbuild-all, jack.pham, faisal.hassan, krishna.kurapati,
	andersson, linux-arm-msm, linux-usb, devicetree, linux-kernel,
	Sriram Dash, Konrad Dybcio, Shazad Hussain

Hi Sriram,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Sriram-Dash/dt-bindings-usb-qcom-snps-dwc3-Add-support-for-firmware-managed-resources/20251127-183548
base:   c77a6544d8a2364e4bee1b52890f577be27b7296
patch link:    https://lore.kernel.org/r/20251127-controller_scmi_upstream-v1-2-38bcca513c28%40oss.qualcomm.com
patch subject: [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management
config: nios2-randconfig-r071-20251204 (https://download.01.org/0day-ci/archive/20251206/202512060556.pgIgFNxx-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202512060556.pgIgFNxx-lkp@intel.com/

smatch warnings:
drivers/usb/dwc3/dwc3-qcom.c:801 dwc3_qcom_probe() error: uninitialized symbol 'ret'.

vim +/ret +801 drivers/usb/dwc3/dwc3-qcom.c

21188e8d6d7590 Krishna Kurapati   2025-09-07  668  
2bc02355f8ba2c Lee Jones          2019-06-17  669  static int dwc3_qcom_probe(struct platform_device *pdev)
2bc02355f8ba2c Lee Jones          2019-06-17  670  {
1881a32fe14df8 Bjorn Andersson    2025-04-14  671  	struct dwc3_probe_data	probe_data = {};
2bc02355f8ba2c Lee Jones          2019-06-17  672  	struct device		*dev = &pdev->dev;
a4333c3a6ba9ca Manu Gautam        2018-05-09  673  	struct dwc3_qcom	*qcom;
1881a32fe14df8 Bjorn Andersson    2025-04-14  674  	struct resource		res;
1881a32fe14df8 Bjorn Andersson    2025-04-14  675  	struct resource		*r;
e33ebb133a245a Bjorn Andersson    2025-05-08  676  	int			ret;
a4333c3a6ba9ca Manu Gautam        2018-05-09  677  	bool			ignore_pipe_clk;
e3fafbd8e36530 Johan Hovold       2022-08-04  678  	bool			wakeup_source;
a4333c3a6ba9ca Manu Gautam        2018-05-09  679  
a4333c3a6ba9ca Manu Gautam        2018-05-09  680  	qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
a4333c3a6ba9ca Manu Gautam        2018-05-09  681  	if (!qcom)
a4333c3a6ba9ca Manu Gautam        2018-05-09  682  		return -ENOMEM;
a4333c3a6ba9ca Manu Gautam        2018-05-09  683  
a4333c3a6ba9ca Manu Gautam        2018-05-09  684  	qcom->dev = &pdev->dev;
a4333c3a6ba9ca Manu Gautam        2018-05-09  685  
79456073227880 Sriram Dash        2025-11-27  686  	qcom->fw_managed = device_get_match_data(dev);
79456073227880 Sriram Dash        2025-11-27  687  	if (qcom->fw_managed) {
79456073227880 Sriram Dash        2025-11-27  688  		ret = dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_INIT);
79456073227880 Sriram Dash        2025-11-27  689  		if (ret < 0)
79456073227880 Sriram Dash        2025-11-27  690  			return ret;
79456073227880 Sriram Dash        2025-11-27  691  	}
79456073227880 Sriram Dash        2025-11-27  692  
a4333c3a6ba9ca Manu Gautam        2018-05-09  693  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
a4333c3a6ba9ca Manu Gautam        2018-05-09  694  	if (IS_ERR(qcom->resets)) {
79456073227880 Sriram Dash        2025-11-27  695  		dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
60d5b71933c4f1 Andrew Halaney     2023-06-05  696  			      "failed to get resets\n");

ret = dev_err_probe()

79456073227880 Sriram Dash        2025-11-27  697  		goto resources_off;
a4333c3a6ba9ca Manu Gautam        2018-05-09  698  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  699  
e33ebb133a245a Bjorn Andersson    2025-05-08  700  	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
e33ebb133a245a Bjorn Andersson    2025-05-08  701  	if (ret < 0)
e33ebb133a245a Bjorn Andersson    2025-05-08  702  		return dev_err_probe(dev, ret, "failed to get clocks\n");
e33ebb133a245a Bjorn Andersson    2025-05-08  703  	qcom->num_clocks = ret;
e33ebb133a245a Bjorn Andersson    2025-05-08  704  
a4333c3a6ba9ca Manu Gautam        2018-05-09  705  	ret = reset_control_assert(qcom->resets);
a4333c3a6ba9ca Manu Gautam        2018-05-09  706  	if (ret) {
a4333c3a6ba9ca Manu Gautam        2018-05-09  707  		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
79456073227880 Sriram Dash        2025-11-27  708  		goto resources_off;
a4333c3a6ba9ca Manu Gautam        2018-05-09  709  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  710  
a4333c3a6ba9ca Manu Gautam        2018-05-09  711  	usleep_range(10, 1000);
a4333c3a6ba9ca Manu Gautam        2018-05-09  712  
a4333c3a6ba9ca Manu Gautam        2018-05-09  713  	ret = reset_control_deassert(qcom->resets);
a4333c3a6ba9ca Manu Gautam        2018-05-09  714  	if (ret) {
a4333c3a6ba9ca Manu Gautam        2018-05-09  715  		dev_err(&pdev->dev, "failed to deassert resets, err=%d\n", ret);
ef8abc0ba49ce7 Krishna Kurapati   2025-07-09  716  		return ret;
a4333c3a6ba9ca Manu Gautam        2018-05-09  717  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  718  
e33ebb133a245a Bjorn Andersson    2025-05-08  719  	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
e33ebb133a245a Bjorn Andersson    2025-05-08  720  	if (ret < 0)
ef8abc0ba49ce7 Krishna Kurapati   2025-07-09  721  		return ret;
a4333c3a6ba9ca Manu Gautam        2018-05-09  722  
1881a32fe14df8 Bjorn Andersson    2025-04-14  723  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
4c0fca65d10548 Dan Carpenter      2025-04-23  724  	if (!r) {
4c0fca65d10548 Dan Carpenter      2025-04-23  725  		ret = -EINVAL;
1881a32fe14df8 Bjorn Andersson    2025-04-14  726  		goto clk_disable;
4c0fca65d10548 Dan Carpenter      2025-04-23  727  	}
1881a32fe14df8 Bjorn Andersson    2025-04-14  728  	res = *r;
1881a32fe14df8 Bjorn Andersson    2025-04-14  729  	res.end = res.start + SDM845_QSCRATCH_BASE_OFFSET;
1881a32fe14df8 Bjorn Andersson    2025-04-14  730  
1881a32fe14df8 Bjorn Andersson    2025-04-14  731  	qcom->qscratch_base = devm_ioremap(dev, res.end, SDM845_QSCRATCH_SIZE);
4c0fca65d10548 Dan Carpenter      2025-04-23  732  	if (!qcom->qscratch_base) {
4c0fca65d10548 Dan Carpenter      2025-04-23  733  		dev_err(dev, "failed to map qscratch region\n");
4c0fca65d10548 Dan Carpenter      2025-04-23  734  		ret = -ENOMEM;
41717b88abf1ca Krishna Kurapati   2024-03-05  735  		goto clk_disable;
a4333c3a6ba9ca Manu Gautam        2018-05-09  736  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  737  
2dc9f137e19426 Bjorn Andersson    2025-04-14  738  	ret = dwc3_qcom_setup_irq(qcom, pdev);
2bc02355f8ba2c Lee Jones          2019-06-17  739  	if (ret) {
2bc02355f8ba2c Lee Jones          2019-06-17  740  		dev_err(dev, "failed to setup IRQs, err=%d\n", ret);
41717b88abf1ca Krishna Kurapati   2024-03-05  741  		goto clk_disable;
a4333c3a6ba9ca Manu Gautam        2018-05-09  742  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  743  
a4333c3a6ba9ca Manu Gautam        2018-05-09  744  	/*
a4333c3a6ba9ca Manu Gautam        2018-05-09  745  	 * Disable pipe_clk requirement if specified. Used when dwc3
a4333c3a6ba9ca Manu Gautam        2018-05-09  746  	 * operates without SSPHY and only HS/FS/LS modes are supported.
a4333c3a6ba9ca Manu Gautam        2018-05-09  747  	 */
a4333c3a6ba9ca Manu Gautam        2018-05-09  748  	ignore_pipe_clk = device_property_read_bool(dev,
a4333c3a6ba9ca Manu Gautam        2018-05-09  749  				"qcom,select-utmi-as-pipe-clk");
a4333c3a6ba9ca Manu Gautam        2018-05-09  750  	if (ignore_pipe_clk)
a4333c3a6ba9ca Manu Gautam        2018-05-09  751  		dwc3_qcom_select_utmi_clk(qcom);
a4333c3a6ba9ca Manu Gautam        2018-05-09  752  
21188e8d6d7590 Krishna Kurapati   2025-09-07  753  	qcom->mode = usb_get_dr_mode(dev);
21188e8d6d7590 Krishna Kurapati   2025-09-07  754  
21188e8d6d7590 Krishna Kurapati   2025-09-07  755  	if (qcom->mode == USB_DR_MODE_HOST) {
21188e8d6d7590 Krishna Kurapati   2025-09-07  756  		qcom->current_role = USB_ROLE_HOST;
21188e8d6d7590 Krishna Kurapati   2025-09-07  757  	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
21188e8d6d7590 Krishna Kurapati   2025-09-07  758  		qcom->current_role = USB_ROLE_DEVICE;
21188e8d6d7590 Krishna Kurapati   2025-09-07  759  		dwc3_qcom_vbus_override_enable(qcom, true);
21188e8d6d7590 Krishna Kurapati   2025-09-07  760  	} else {
21188e8d6d7590 Krishna Kurapati   2025-09-07  761  		if ((device_property_read_bool(dev, "usb-role-switch")) &&
21188e8d6d7590 Krishna Kurapati   2025-09-07  762  		    (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
21188e8d6d7590 Krishna Kurapati   2025-09-07  763  			qcom->current_role = USB_ROLE_HOST;
21188e8d6d7590 Krishna Kurapati   2025-09-07  764  		else
21188e8d6d7590 Krishna Kurapati   2025-09-07  765  			qcom->current_role = USB_ROLE_DEVICE;
21188e8d6d7590 Krishna Kurapati   2025-09-07  766  	}
21188e8d6d7590 Krishna Kurapati   2025-09-07  767  
21188e8d6d7590 Krishna Kurapati   2025-09-07  768  	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
21188e8d6d7590 Krishna Kurapati   2025-09-07  769  
1881a32fe14df8 Bjorn Andersson    2025-04-14  770  	qcom->dwc.dev = dev;
1881a32fe14df8 Bjorn Andersson    2025-04-14  771  	probe_data.dwc = &qcom->dwc;
1881a32fe14df8 Bjorn Andersson    2025-04-14  772  	probe_data.res = &res;
1881a32fe14df8 Bjorn Andersson    2025-04-14  773  	probe_data.ignore_clocks_and_resets = true;
7298c06d58e23c Frank Li           2025-09-29  774  	probe_data.properties = DWC3_DEFAULT_PROPERTIES;
1881a32fe14df8 Bjorn Andersson    2025-04-14  775  	ret = dwc3_core_probe(&probe_data);
2bc02355f8ba2c Lee Jones          2019-06-17  776  	if (ret)  {
1881a32fe14df8 Bjorn Andersson    2025-04-14  777  		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
41717b88abf1ca Krishna Kurapati   2024-03-05  778  		goto clk_disable;
a4333c3a6ba9ca Manu Gautam        2018-05-09  779  	}
a4333c3a6ba9ca Manu Gautam        2018-05-09  780  
bea46b9815154a Sandeep Maheswaram 2020-07-27  781  	ret = dwc3_qcom_interconnect_init(qcom);
bea46b9815154a Sandeep Maheswaram 2020-07-27  782  	if (ret)
1881a32fe14df8 Bjorn Andersson    2025-04-14  783  		goto remove_core;
bea46b9815154a Sandeep Maheswaram 2020-07-27  784  
e3fafbd8e36530 Johan Hovold       2022-08-04  785  	wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
e3fafbd8e36530 Johan Hovold       2022-08-04  786  	device_init_wakeup(&pdev->dev, wakeup_source);
d9be8d5c5b032e Sandeep Maheswaram 2022-06-13  787  
a4333c3a6ba9ca Manu Gautam        2018-05-09  788  	qcom->is_suspended = false;
a4333c3a6ba9ca Manu Gautam        2018-05-09  789  
a4333c3a6ba9ca Manu Gautam        2018-05-09  790  	return 0;
a4333c3a6ba9ca Manu Gautam        2018-05-09  791  
1881a32fe14df8 Bjorn Andersson    2025-04-14  792  remove_core:
1881a32fe14df8 Bjorn Andersson    2025-04-14  793  	dwc3_core_remove(&qcom->dwc);
a4333c3a6ba9ca Manu Gautam        2018-05-09  794  clk_disable:
e33ebb133a245a Bjorn Andersson    2025-05-08  795  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
a4333c3a6ba9ca Manu Gautam        2018-05-09  796  
79456073227880 Sriram Dash        2025-11-27  797  resources_off:
79456073227880 Sriram Dash        2025-11-27  798  	if (qcom->fw_managed)
79456073227880 Sriram Dash        2025-11-27  799  		dev_pm_opp_set_level(qcom->dev, DWC3_QCOM_FW_MANAGED_EXIT);
79456073227880 Sriram Dash        2025-11-27  800  
a4333c3a6ba9ca Manu Gautam        2018-05-09 @801  	return ret;
a4333c3a6ba9ca Manu Gautam        2018-05-09  802  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-09  8:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-27 10:31 [PATCH 0/2] usb: dwc3: qcom: Add support for firmware-managed resources Sriram Dash
2025-11-27 10:31 ` [PATCH 1/2] dt-bindings: usb: qcom,snps-dwc3: " Sriram Dash
2025-11-27 11:29   ` Rob Herring (Arm)
2025-11-27 12:13   ` Krzysztof Kozlowski
2025-11-27 13:56     ` Krzysztof Kozlowski
2025-12-01 11:35     ` Sriram Dash
2025-11-27 10:31 ` [PATCH 2/2] usb: dwc3: qcom: Support firmware-managed resource states for power management Sriram Dash
2025-11-27 12:14   ` Krzysztof Kozlowski
2025-12-02  5:21   ` Bjorn Andersson
2025-12-02  6:37     ` Sriram Dash
2025-12-02 17:17       ` Bjorn Andersson
2025-12-09  8:48   ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).