Re: [PATCH v10 01/14] dt-bindings: media: qcom,glymur-iris: Add glymur video codec

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree binding incorrectly defines and requires `#address-cells` and `#size-cells`, forcing conformant device trees to trigger dtc warnings.
--

--- Patch [1]: [PATCH v10 01/14] dt-bindings: media: qcom,glymur-iris: Add glymur video codec ---
commit 6a8af4611de5993b1c0f3d458ae92b3f17956b81
Author: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

dt-bindings: media: qcom,glymur-iris: Add glymur video codec

This commit adds the device tree binding for the Qualcomm Glymur Iris video
codec, a dual-core video IP. It increases the maxItems constraints for clocks
and power-domains in the common venus schema to accommodate the new
architecture.

> diff --git a/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml b/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml
> new file mode 100644
> index 0000000000000..1bb16206d859b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml
[ ... ]
> +  interconnect-names:
> +    items:
> +      - const: cpu-cfg
> +      - const: video-mem
> +
> +  '#address-cells':
> +    const: 2
> +
> +  '#size-cells':
> +    const: 2
> +

[Severity: Low]
Does this binding unnecessarily define #address-cells and #size-cells?

Since none of the permitted child nodes (non-pixel, pixel, video-firmware, or
opp-table) have a reg property, and no ranges property is defined, wouldn't
enforcing these properties unconditionally trigger the dtc warning
avoid_unnecessary_addr_size?

> +  non-pixel:
> +    type: object
[ ... ]
> +required:
> +  - clocks
> +  - clock-names
> +  - compatible
> +  - dma-coherent
> +  - interconnects
> +  - interconnect-names
> +  - "#address-cells"
> +  - "#size-cells"

[Severity: Low]
Because these properties are marked as required, any conformant device tree
is forced to include them, which will result in the dtc warnings mentioned
above.

> +  - non-pixel
> +  - pixel
> +  - power-domains
[ ... ]
> +        resets = <&gcc_video_axi0c_clk_ares>,
> +                 <&gcc_video_axi0_clk_ares>,
> +                 <&videocc_mvs0c_freerun_clk_ares>,
> +                 <&videocc_mvs0_freerun_clk_ares>,
> +                 <&gcc_video_axi1_clk_ares>,
> +                 <&videocc_mvs1_freerun_clk_ares>;
> +        reset-names = "core_bus",
> +                      "vcodec0_bus",
> +                      "core",
> +                      "vcodec0_core",
> +                      "vcodec1_bus",
> +                      "vcodec1_core";
> +
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +

[Severity: Low]
Are these properties in the example block propagating this pattern? 

By demonstrating them here, it enforces adding properties that aren't used by
the child nodes.

> +        non-pixel {
> +            iommus = <&apps_smmu 0x1940 0x0000>,
> +                     <&apps_smmu 0x1944 0x0000>,
> +                     <&apps_smmu 0x19e0 0x0000>;
> +            memory-region = <&iris_resv>;
> +        };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-glymur-v10-0-de451559e88b@oss.qualcomm.com?part=1