Re: [PATCH v9 01/14] dt-bindings: media: qcom,glymur-iris: Add glymur video codec
Vishnu Reddy <[email protected]> Wed, 5 Aug 2026 18:22:15 +0530
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Hi Rob, On 7/21/2026 7:50 PM, Rob Herring wrote: > On Mon, Jul 20, 2026 at 8:00 PM Vishnu Reddy > <[email protected]> wrote: >> >> On 7/16/2026 10:24 PM, Rob Herring wrote: >>> On Thu, Jul 16, 2026 at 08:37:17AM +0530, Vishnu Reddy wrote: >>>> On 7/15/2026 7:52 PM, [email protected] wrote: >>>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: >>>>> - [Low] Unnecessary `#address-cells` and `#size-cells` properties on a node without addressable children. >>>>> -- >>>>> >>>>> --- Patch [1]: [PATCH v9 01/14] dt-bindings: media: qcom,glymur-iris: Add glymur video codec --- >>>>> commit 8ca344fa9395fcf1d13bd1ea93de8bee21a5c176 >>>>> 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. It introduces a new dual-core architecture requiring additional >>>>> power domains and clocks, and updates the maxItems limits in the common >>>>> venus schema to accommodate these changes. >>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml b/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml >>>>>> new file mode 100644 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/media/qcom,glymur-iris.yaml >>>>>> @@ -0,0 +1,255 @@ >>>>> [ ... ] >>>>>> + interconnect-names: >>>>>> + items: >>>>>> + - const: cpu-cfg >>>>>> + - const: video-mem >>>>>> + >>>>>> + '#address-cells': >>>>>> + const: 2 >>>>>> + >>>>>> + '#size-cells': >>>>>> + const: 2 >>>>> [Severity: Low] >>>>> Are these properties necessary here? The schema strictly enforces const: 2 >>>>> for #address-cells and #size-cells at the root node, but none of the child >>>>> nodes (non-pixel, pixel, firmware) define a reg property, and the parent >>>>> does not define a ranges property. >>>>> >>>>> Will this cause the DeviceTree compiler (dtc) to emit an >>>>> avoid_unnecessary_addr_size warning when compiling device trees using this >>>>> node? >>>>> >>>>>> + >>>>>> + non-pixel: >>>>>> + type: object >>>>> [ ... ] >>>>>> + 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] >>>>> Does the example need #address-cells and #size-cells? It demonstrates their >>>>> use without any child nodes containing reg properties, which directly causes a >>>>> dtc warning during dt_binding_check validation. >>>> Without #address-cells and #size-cells, getting the below kernel warning prints. >>>> [ 6.953615] Missing '#address-cells' in /soc@0/video-codec@aa00000 >>>> [ 6.968288] WARNING: drivers/of/base.c:115 at of_bus_n_addr_cells+0xa4/0xf4 >>>> >>>> [ 7.440909] Missing '#size-cells' in /soc@0/video-codec@aa00000 >>>> [ 7.447044] WARNING: drivers/of/base.c:142 at of_bus_n_size_cells+0xa4/0xf8 >>> Then the kernel is wrong. >> Hi Rob, >> >> Looking at the iris DT node from the link below: >> https://lore.kernel.org/all/[email protected]/ >> >> The iris is the parent node and non_pixel is the child node, which does not >> have a reg property, but does have a memory-region reference containing >> iommu-addresses. During of_translate_dma_region, #address-cells and >> #size-cells are required from the parent node for address translation. > iommu-addresses in the reserved memory node would use > #address-cells/#size-cells from that node. Currently, linux kernel does not use #address-cells/#size-cells from the reserved memory node. Changing it something like below can make it to use from that node as you suggested. diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index a18bb60f6f3d..e0d3f3728afd 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -251,7 +251,7 @@ void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) if (of_dma_is_coherent(dev->of_node)) prot |= IOMMU_CACHE; - maps = of_translate_dma_region(np, maps, &iova, &length); + maps = of_translate_dma_region(it.node, maps, &iova, &length); if (length == 0) { Kindly let me know your comment on this. > IMO, you shouldn't even have 'pixel' and 'non-pixel' nodes. Their > contents belong in the parent. > > Rob