Re: [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Adding the three NPU clocks globally relaxes the maximum allowed clocks to 5 for all Rockchip IOMMUs, disabling strict validation for older hardware.
- [Medium] The failure to introduce a distinct compatible string for the NPU MMU hardware variant prevents the schema from enforcing its mandatory extra clocks.
--

--- Patch [3]: [RFC PATCH v5 v5 3/8] dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set ---
commit 1dfa4f75475ad14a765f82a9e6d6d985e8af62dc
Author: Jiaxing Hu <[email protected]>

dt-bindings: iommu: rockchip: allow the RK3576 NPU MMU clock set

This commit expands the clocks and clock-names properties in the Rockchip IOMMU
binding to allow up to 5 items to support the RK3576 NPU MMU. It maintains a
minimum item count of 2 to keep existing device trees valid.

> diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> index 6ce41d11ff5e5..a3cedcaaa9792 100644
> --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> @@ -42,14 +42,22 @@ properties:
>      minItems: 1
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: Core clock
>        - description: Interface clock
> +      - description: Compute clock, RK3576 NPU MMUs only
> +      - description: Convolution buffer core clock, RK3576 NPU MMUs only
> +      - description: Convolution buffer interface clock, RK3576 NPU MMUs only

[Severity: Medium]
Does expanding the items list globally cause a regression by relaxing the
maximum allowed clocks to 5 for all Rockchip IOMMUs?

If older hardware variants specify 3, 4, or 5 clocks in their device trees,
they will now silently pass schema validation. Would it be better to restrict
the maximum clocks to 2 for older variants using an allOf: if-then
conditional block?

Additionally, because the generic rockchip,rk3576-iommu compatible string is
used for both standard and NPU MMUs, the schema forces minItems: 2 globally.
Does the lack of a distinct compatible string for the NPU MMU hardware variant
introduce a regression where the schema is prevented from enforcing its
mandatory extra clocks?

If a device tree node for the RK3576 NPU MMU is missing its required extra
clocks, won't it silently pass validation but fail at runtime by dropping
register writes?

>  
>    clock-names:
> +    minItems: 2
>      items:
>        - const: aclk
>        - const: iface
> +      - const: npu
> +      - const: aclk_cbuf
> +      - const: hclk_cbuf

[Severity: Medium]
Similar to the clocks property above, does widening the clock-names property
without conditional constraints cause a regression that allows invalid
configurations for older hardware variants to silently pass validation?

Should an allOf: if-then block be used here as well?

>  
>    "#iommu-cells":
>      const: 0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3