Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom,shikra-pas: Add iommus constraints

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: remoteproc: shikra-pas: iommus applied to mpss-pas in allOf
--

commit 53aa8f92faa854bf45005817e5eaa528a79c90a8
Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>

dt-bindings: remoteproc: qcom,shikra-pas: Add iommus constraints

This commit adds iommus property constraints to the Shikra PAS remoteproc
binding schema for booting at EL2 without a hypervisor.

> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,shikra-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,shikra-pas.yaml
> index 34a2f15e93616..8c8a7616d4787 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,shikra-pas.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,shikra-pas.yaml
> @@ -112,6 +112,8 @@ allOf:
>            maxItems: 1
>          firmware-name:
>            maxItems: 1
> +        iommus:
> +          maxItems: 1
>          power-domains:
>            items:
>              - description: CX power domain
> @@ -134,6 +136,9 @@ allOf:
>            minItems: 2
>          firmware-name:
>            minItems: 2
> +        iommus:
> +          minItems: 3
> +          maxItems: 3

[Severity: Medium]
The commit message states that iommus constraints are added for
qcom,shikra-cdsp-pas and qcom,shikra-lpaicp-pas. However, in the allOf
section of Documentation/devicetree/bindings/remoteproc/qcom,shikra-pas.yaml,
the first conditional block matches both qcom,shikra-cdsp-pas and
qcom,shikra-mpss-pas:

  - if:
      properties:
        compatible:
          enum:
            - qcom,shikra-cdsp-pas
            - qcom,shikra-mpss-pas
    then:
      properties:
        ...
        iommus:
          maxItems: 1

Does placing iommus in this shared conditional block unintentionally allow
iommus on qcom,shikra-mpss-pas?

Also, since commit 95697c560209 ("dt-bindings: remoteproc: qcom,pas-common:
Drop iommus") removed iommus from qcom,pas-common.yaml, iommus is no longer
declared in top-level properties. Should iommus be defined with its broadest
constraints (minItems: 1, maxItems: 3) in the top-level properties block and
then constrained per compatible in allOf (with maxItems: 1 for
qcom,shikra-cdsp-pas, minItems: 3 for qcom,shikra-lpaicp-pas, and
iommus: false for qcom,shikra-mpss-pas)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-shikra-el2-v4-0-97e7019debff@oss.qualcomm.com?part=2