Re: [PATCH v10 01/14] dt-bindings: media: qcom,glymur-iris: Add glymur video codec
Vishnu Reddy <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
On 7/26/2026 11:30 AM, [email protected] wrote: > 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 <[email protected]> > > 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. iris_resv has iommu-addresses, which requires #address-cells and #size-cells on the parent node. >> + non-pixel { >> + iommus = <&apps_smmu 0x1940 0x0000>, >> + <&apps_smmu 0x1944 0x0000>, >> + <&apps_smmu 0x19e0 0x0000>; >> + memory-region = <&iris_resv>; >> + };