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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema explicitly defines and requires 'interrupts-extended' instead of the standard 'interrupts' property.
- [Low] The example uses a 4-cell interrupt specifier for the ARM GIC, which only accepts 3 cells.
--

commit 4faf2370824eaf5bd43284e69168484a422966ae
Author: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com>

dt-bindings: soc: qcom: Document CDSP Power Management

This commit adds device tree binding documentation for the CDSP Power
Management driver. It describes the properties needed for DCVS requests via
SMEM, LPM transitions via MPM handshakes, and virtual regulators used by the
remoteproc driver.

> 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 0000000000000..f0f89fdeba4ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,cdsp-power.yaml
> @@ -0,0 +1,138 @@
[ ... ]
> +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:

[Severity: Medium]
Should this property be defined as interrupts instead?

The core dt-schema tooling automatically permits and validates
interrupts-extended usage in device trees against the standard interrupts
schema definition.

By explicitly defining interrupts-extended, this forces device tree
authors to use the extended format and prevents them from using the
standard interrupts property with an interrupt-parent.

> +    items:
> +      - description: LPM (Low Power Mode) interrupt from MPM
> +      - description: DCVS (Dynamic Clock and Voltage Scaling) interrupt from IPCC
[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts-extended

[Severity: Medium]
Should this require the standard interrupts property rather than
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>;
> +
> +        cdsp_power: cdsp-power@4ae000 {
> +            compatible = "qcom,cdsp-power";
> +            reg = <0x0 0x004ae000 0x0 0x1000>,
> +                  <0x0 0x26018018 0x0 0x4>;
> +            reg-names = "mpm", "rscc";
> +
> +            interrupts-extended = <&intc GIC_SPI 65 IRQ_TYPE_EDGE_RISING 0>,

[Severity: Low]
Does this ARM GIC interrupt specifier have an extra cell?

The standard ARM GIC binding expects exactly 3 cells for the specifier type,
number, and flags.

The trailing 0 appears to be an extra 4th cell that might cause a cell-count
mismatch during validation.

> +                                  <&ipcc IPCC_CLIENT_CDSP
> +                                         IPCC_MPROC_SIGNAL_PING
> +                                         IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "lpm", "dcvs";
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cdsp-power-v1-0-85eb9501a1cd@oss.qualcomm.com?part=1