Re: [PATCH v3 2/7] dt-bindings: media: sun4i-a10-video-engine: Fix SRAM count for H6

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The binding enforces a second SRAM item for the H6 SoC, but the in-tree device tree is not updated, causing validation failures.

Pre-existing issues:
- [High] The device tree binding requires claiming a second SRAM alias for the H6 video engine, but the SRAM driver hardcodes parsing only the first region.
--

--- Patch [2]: [PATCH v3 2/7] dt-bindings: media: sun4i-a10-video-engine: Fix SRAM count for H6 ---
commit b3f1ab3ef227838179ad2332d41536525f39d18a
Author: Chen-Yu Tsai <wens@kernel.org>

dt-bindings: media: sun4i-a10-video-engine: Fix SRAM count for H6

On the H6, the SRAM C region includes an alias of part of the VE SRAM
used by the video engine. This commit adds a second SRAM region phandle
requirement to the video engine for the H6 to claim this alias window.

> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
> index b4c0fabb38211..e01588a02628d 100644
> --- a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-video-engine.yaml
[ ... ]
> @@ -91,6 +95,21 @@ allOf:
>          iommus:
>            maxItems: 1
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - allwinner,sun50i-h6-video-engine
> +    then:
> +      properties:
> +        allwinner,sram:
> +          minItems: 2

[Severity: High]
This is a pre-existing issue, but does adding the DT binding property solve the
hardware aliasing issue without implementing the necessary driver parser changes?

Looking at sunxi_sram_of_parse() in drivers/soc/sunxi/sunxi_sram.c, it
hardcodes index 0 when parsing the 'allwinner,sram' property:

drivers/soc/sunxi/sunxi_sram.c:sunxi_sram_of_parse() {
    ...
    ret = of_parse_phandle_with_fixed_args(node, "allwinner,sram", 1, 0,
    ...
}

When the Cedrus driver probes, it calls sunxi_sram_claim(), which relies on
sunxi_sram_of_parse() and only claims the single region returned, leaving the
alias window unprotected. 

Without claiming the alias window, could another subsystem claim and use the
overlapping SRAM region concurrently, leading to silent memory corruption?

[Severity: Medium]
Will this new constraint cause validation failures (make dtbs_check) for the
existing in-tree device tree?

It appears the H6 device tree node video-codec@1c0e000 in
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi currently only provides a single
SRAM item:

arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi:video-codec@1c0e000 {
    ...
    allwinner,sram = <&ve_sram 1>;
    ...
}

Should the in-tree device tree be updated alongside this binding change to
satisfy the new minItems constraint?

> +    else:
> +      properties:
> +        allwinner,sram:
> +          maxItems: 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260712030011.3778169-1-wens@kernel.org?part=2