Re: [RFC PATCH 01/11] dt-bindings: media: qcom,sm8550-iris: Add vpu sub nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `oneOf` constraint allows invalid hybrid configurations with both top-level `iommus` and a single stream subnode.
- [Low] Unnecessary `#address-cells` and `#size-cells` properties added to the parent node will trigger a `dtc` warning because no child nodes have `reg` or `ranges` properties.
--

--- Patch [1]: [RFC PATCH 01/11] dt-bindings: media: qcom,sm8550-iris: Add vpu sub nodes ---
commit 952135a9f5f9889eefcc3ad627cf18f885374f2d
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

dt-bindings: media: qcom,sm8550-iris: Add vpu sub nodes

VPU hardwares have a limitation where VPU streams are associated with
dedicated addressable address range. To address this limitation, the subset
of stream/s are now represented as sub nodes, so that they can be associated
to the respective addressable range.

> diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> index 0400ca1bff05d..bf4d24ce90bd3 100644
> --- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
> @@ -91,6 +91,51 @@ properties:
>    opp-table:
>      type: object
>  
> +  '#address-cells':
> +    const: 2
> +
> +  '#size-cells':
> +    const: 2
> +

[Severity: Low]
Does the addition of #address-cells and #size-cells properties to the parent
node trigger a dtc warning?

Since none of the new child nodes (non-pixel, pixel, firmware, or opp-table)
use a reg or ranges property, the avoid_unnecessary_addr_size compiler warning
might be triggered during dt_binding_check.

[ ... ]

> @@ -98,9 +143,15 @@ required:
>    - interconnect-names
>    - resets
>    - reset-names
> -  - iommus
>    - dma-coherent
>  
> +oneOf:
> +  - required:
> +      - iommus
> +  - required:
> +      - non-pixel
> +      - pixel
> +

[Severity: Medium]
Will this oneOf constraint allow invalid hybrid configurations to pass
validation?

If a devicetree incorrectly specifies a top-level iommus property alongside
only one of the subnodes (such as non-pixel), the first branch evaluates to
true and the second branch evaluates to false. Because exactly one branch is
true, the oneOf condition is satisfied instead of rejecting the hybrid state.

Should the branches explicitly forbid the properties of the other branch to
prevent this?

[ ... ]

> @@ -177,12 +228,21 @@ examples:
>          resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>          reset-names = "bus";
>  
> -        iommus = <&apps_smmu 0x1940 0x0000>,
> -                 <&apps_smmu 0x1947 0x0000>;
>          dma-coherent;
>  
>          operating-points-v2 = <&iris_opp_table>;
>  
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        iris_non_pixel: non-pixel {

[Severity: Low]
Does explicitly setting these properties in the example cause dt_binding_check
to emit a warning due to the lack of reg properties in the child nodes?

> +            iommus = <&apps_smmu 0x1940 0x0000>;
> +            memory-region = <&iris_resv>;
> +        };
> +
> +        iris_pixel: pixel {
> +            iommus = <&apps_smmu 0x1947 0x0000>;
> +        };
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vpu_iommu_iova_handling-v1-0-72bb62cb2dfd@oss.qualcomm.com?part=1