Re: [PATCH 1/2] dt-bindings: soc: qcom: Document CDSP Power Management

[Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>]

On 5/22/2026 4:59 AM, Bjorn Andersson wrote:
> On Wed, May 20, 2026 at 12:35:09AM +0530, Vignesh Viswanathan wrote:
>> Add documentation for the CDSP Power Management driver, which handles
> 
> Your commit message should not describe an action, it should describe
> the problem you're solving.
> 
>> Dynamic Clock and Voltage Scaling (DCVS) requests via SMEM, manages Low
>> Power Mode (LPM) transitions via MPM handshake, and provides virtual
>> regulators for the remoteproc driver to control CDSP power rails.
>>
> 
> You have a node describing the CDSP (remoteproc) already, but it doesn't
> contain all the properties, so you're going to add this sibling node.
> 
> Why don't you describe the remoteproc properly instead?

Sure, will address in next version.

> 
>> Signed-off-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>> ---
>>  .../bindings/soc/qcom/qcom,cdsp-power.yaml         | 138 +++++++++++++++++++++
>>  1 file changed, 138 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml
>> new file mode 100644
>> index 000000000000..f0f89fdeba4e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml
>> @@ -0,0 +1,138 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/qcom/qcom,cdsp-power.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm CDSP Power Management
>> +
>> +maintainers:
>> +  - Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>
>> +
>> +description:
>> +  The CDSP Power Management driver provides power management services for the
>> +  Qualcomm Compute DSP (CDSP) subsystem. It handles Dynamic Clock and Voltage
>> +  Scaling (DCVS) requests via SMEM, manages Low Power Mode (LPM) transitions
>> +  via MPM handshake, and provides virtual regulators that are consumed by the
>> +  CDSP remoteproc driver.
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,cdsp-power
>> +
>> +  reg:
>> +    items:
>> +      - description: MPM (Modem Power Manager) register region
>> +      - description: RSCC (RSC Configuration) register region
>> +
>> +  reg-names:
>> +    items:
>> +      - const: mpm
>> +      - const: rscc
>> +
>> +  interrupts-extended:
>> +    items:
>> +      - description: LPM (Low Power Mode) interrupt from MPM
>> +      - description: DCVS (Dynamic Clock and Voltage Scaling) interrupt from IPCC
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: lpm
>> +      - const: dcvs
>> +
>> +  mboxes:
>> +    maxItems: 1
>> +    description: IPCC mailbox channel for sending DCVS responses to CDSP
>> +
>> +  qcom,smem-item:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      SMEM item ID used for DCVS communication channel between APSS and CDSP.
>> +      This is a platform-specific value that identifies the shared memory region.
>> +
>> +  vdd-cx-supply:
>> +    description:
>> +      Phandle to the CX voltage regulator. This is the actual hardware regulator
>> +      (e.g., from MP8899 PMIC) that supplies power to the CDSP CX rail.
> 
> This isn't the CX supply of the power management block, this is the CX
> supply of the remoteproc - so put it there.

Ack.

> 
>> +
>> +  vdd-mx-supply:
>> +    description:
>> +      Phandle to the MX voltage regulator. This is the actual hardware regulator
>> +      (e.g., from MP8899 PMIC) that supplies power to the CDSP MX rail. Optional
>> +      on boards where MX rail is always-on or not present.
>> +
>> +  regulators:
>> +    type: object
>> +    description:
>> +      Virtual regulators provided by this driver for consumption by the CDSP
>> +      remoteproc driver. These virtual regulators pass through enable/disable
>> +      requests to the actual hardware regulators (vdd-cx-supply, vdd-mx-supply).
> 
> These regulators doesn't exist in reality, they are only here because
> you choose to split the description of your remoteproc implementation in
> two.

Yes, this is because the actual regulator can be controlled by two independent entities,
the standard PAS driver and this CDSP driver when CDSP requests via the SMEM channel.

This entire design is implemented because IPQ9650 does not have AOSS and CDSP cannot control
the regulator supplies, or the MPM power sequences. So CDSP has a channel over SMEM to request
the APSS to control the regulators or the power sequences, and remoteproc driver
also does regulator enable/disable.

As you suggested in the driver's review, will try to squash these into a new remoteproc
driver and post the next version.

> 
>> +
>> +    properties:
>> +      cdsp-vdd-cx:
>> +        type: object
>> +        $ref: /schemas/regulator/regulator.yaml#
>> +        description: Virtual CX regulator for CDSP
>> +        unevaluatedProperties: false
>> +
>> +      cdsp-vdd-mx:
>> +        type: object
>> +        $ref: /schemas/regulator/regulator.yaml#
>> +        description: Virtual MX regulator for CDSP
>> +        unevaluatedProperties: false
>> +
>> +    additionalProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - interrupts-extended
>> +  - interrupt-names
>> +  - mboxes
>> +  - qcom,smem-item
>> +  - vdd-cx-supply
>> +  - regulators
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/mailbox/qcom-ipcc.h>
>> +
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
> 
> It's just an example, when you write bindings drop the 0x0 from base and
> size in your reg and this need goes away.

Ack.

> 
>> +
>> +        cdsp_power: cdsp-power@4ae000 {
>> +            compatible = "qcom,cdsp-power";
>> +            reg = <0x0 0x004ae000 0x0 0x1000>,
> 
> For when you do this properly, please confirm that this is a dedicated
> MPM register region and does not alias with any other nodes.

Yes, this region is dedicated to CDSP MPM.

> 
>> +                  <0x0 0x26018018 0x0 0x4>;
> 
> No, we don't point reg = <> at a single register.

Ack, will address this.
> 
>> +            reg-names = "mpm", "rscc";
>> +
>> +            interrupts-extended = <&intc GIC_SPI 65 IRQ_TYPE_EDGE_RISING 0>,
> 
> 0?

GIC interrupt-controller node has interrupt-cells as 4 in IPQ9650.

> 
>> +                                  <&ipcc IPCC_CLIENT_CDSP
>> +                                         IPCC_MPROC_SIGNAL_PING
>> +                                         IRQ_TYPE_EDGE_RISING>;
>> +            interrupt-names = "lpm", "dcvs";
>> +
>> +            mboxes = <&ipcc IPCC_CLIENT_CDSP IPCC_MPROC_SIGNAL_PING>;
>> +
>> +            qcom,smem-item = <503>;
> 
> Isn't this static for the given remoteproc?

Yes, it is static, will move this within the driver.

Thanks,
Vignesh

> 
> Regards,
> Bjorn
> 
>> +
>> +            vdd-cx-supply = <&ipq9650_s2>;
>> +            vdd-mx-supply = <&ipq9650_s4>;
>> +
>> +            regulators {
>> +                cdsp_vdd_cx: cdsp-vdd-cx {
>> +                    regulator-name = "cdsp-vdd-cx";
>> +                };
>> +
>> +                cdsp_vdd_mx: cdsp-vdd-mx {
>> +                    regulator-name = "cdsp-vdd-mx";
>> +                };
>> +            };
>> +        };
>> +    };
>>
>> -- 
>> 2.43.0
>>