Re: [PATCH 01/22] dt-bindings: media: qcom,venus-common: Allow IOVA reservation memory-region
Vikash Garodia <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
On 8/7/2026 2:19 PM, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The patch updates `qcom,venus-common.yaml` to permit a second `memory-region` phandle for an IOMMU IOVA reservation, but fails to remove a stale `maxItems: 1` constraint from `qcom,sc7180-venus.yaml`. This prevents SC7180 devices from successfully validating if they supply the newly permitted second region. patch #3 in this series incorporates this comment. > -- > > --- Patch [1]: [PATCH 01/22] dt-bindings: media: qcom,venus-common: Allow IOVA reservation memory-region --- > commit 44e20424e211e02ef9914abe4592c185ad0d75ac > Author: Vikash Garodia <[email protected]> > > dt-bindings: media: qcom,venus-common: Allow IOVA reservation memory-region > > Non-pixel stream IDs should not allocate IOVA space from the 0 to 600MB > restricted region, as this region is reserved exclusively for secure > non-pixel. > > In addition to the firmware-loaded codec carveout, platforms need to > declare an IOMMU IOVA reservation (a reserved-memory node with > iommu-addresses) to keep DMA away from restricted IOVA range. > > Permit a second memory-region phandle for this purpose, and describe the > meaning of each entry so the ordering is unambiguous. > >> diff --git a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml >> index 59a3fde846d21..0be2f9119e782 100644 >> --- a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml >> +++ b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml >> @@ -37,7 +37,10 @@ properties: >> maxItems: 20 >> >> memory-region: >> - maxItems: 1 >> + minItems: 1 >> + items: >> + - description: Firmware-loaded codec carveout >> + - description: IOMMU IOVA reservation region > > [Severity: Medium] > Since this patch expands the common schema to allow two memory-region items, > should the local constraints in related variant schemas also be updated? > > For example, the local constraint in qcom,sc7180-venus.yaml restricts the > items to 1: > > Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml:properties.memory-region { > ... > memory-region: > maxItems: 1 > ... > } > > Because dt-schema evaluates a logical AND between local constraints and > the included $ref schemas, won't SC7180 device trees fail dt_binding_check > validation if they declare the newly permitted IOVA reservation region? > >> >> power-domains: >> minItems: 1 >