Re: [PATCH v2 2/3] dt-bindings: remoteproc: qcom,nord-pas: Document Nord PAS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema fails to enforce the 3 power domains (CX, MX, NSP) required for CDSP variants.
- [Low] The `#define IPCC_MPROC_ADSP0` macro in the example lacks a value, resulting in malformed cell arrays after C preprocessing.
--

--- Patch [2]: [PATCH v2 2/3] dt-bindings: remoteproc: qcom,nord-pas: Document Nord PAS ---
commit de7d4d143ab4797af6353c2a43b42cf33e30c7ad
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

dt-bindings: remoteproc: qcom,nord-pas: Document Nord PAS

Add a dedicated binding for the Qualcomm Nord SoC PAS devices: ADSP
and CDSPs. Nord ADSP uses CX and MX power domains while the CDSPs
add an NSP domain on top of that.

> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
> new file mode 100644
> index 0000000000000..c1d60ca6c63cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,nord-pas.yaml
> @@ -0,0 +1,167 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,nord-pas.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - qcom,nord-adsp-pas
> +      - qcom,nord-cdsp0-pas
> +      - qcom,nord-cdsp1-pas
> +      - qcom,nord-cdsp2-pas
> +      - qcom,nord-cdsp3-pas
> +
> +  power-domains:
> +    minItems: 2
> +    items:
> +      - description: CX power domain
> +      - description: MX power domain
> +      - description: NSP power domain

[ ... ]

> +allOf:
> +  - $ref: /schemas/remoteproc/qcom,pas-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - qcom,nord-adsp-pas
> +    then:
> +      properties:
> +        power-domains:
> +          maxItems: 2
> +        power-domain-names:
> +          maxItems: 2

[Severity: Medium]
Since the global power-domains property defines minItems: 2, doesn't this
allow CDSP variants to incorrectly pass schema validation if they omit the
NSP power domain?

Should there be a corresponding constraint enforcing minItems: 3 for the CDSP
variants?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/mailbox/qcom-ipcc.h>
> +    #include <dt-bindings/power/qcom,rpmhpd.h>
> +    #define IPCC_MPROC_ADSP0

[ ... ]

> +        glink-edge {
> +            interrupts-extended = <&ipcc IPCC_MPROC_ADSP0
> +                                         IPCC_MPROC_SIGNAL_GLINK_QMP
> +                                         IRQ_TYPE_EDGE_RISING>;
> +            mboxes = <&ipcc IPCC_MPROC_ADSP0 IPCC_MPROC_SIGNAL_GLINK_QMP>;

[Severity: Low]
Because the IPCC_MPROC_ADSP0 macro defined above lacks an assigned numeric
value, won't it be replaced with an empty string during C preprocessing?

Could this cause the interrupts-extended and mboxes arrays to silently drop a
required cell?

> +
> +            label = "adsp";
> +            qcom,remote-pid = <2>;
> +
> +            /* ... */
> +        };
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724103848.233804-1-shengchao.guo@oss.qualcomm.com?part=2