[v2,5/7] arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine

Message ID 20260711135959.3616919-6-wens@kernel.org (mailing list archive)
State New
Headers
Series arm64: allwinner: h616: Support Video Engine |

Commit Message

Chen-Yu Tsai July 11, 2026, 1:59 p.m. UTC
The SRAM C region contains a partial alias to the VE SRAM already
referenced by the video engine. To avoid access through this alias
window, the region should also be claimed by the video engine.

Add a reference to the SRAM C region to the video engine node.

Fixes: b542570e5605 ("arm64: dts: allwinner: h6: Add Video Engine node")
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

sashiko-bot@kernel.org July 11, 2026, 2:08 p.m. UTC | #1
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding a second phandle to the `allwinner,sram` property has no effect at runtime because the sunxi_sram driver infrastructure hardcodes parsing to only the first index.
--

--- Patch [5]: [PATCH v2 5/7] arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine ---
commit 8b2bcf328faa458d85d11e3cef06ddda86e2e10e
Author: Chen-Yu Tsai <wens@kernel.org>

arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine

This commit adds a reference to the SRAM C region in the video engine node
to prevent access through a partial alias window.

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -205,7 +205,7 @@ video-codec@1c0e000 {
>  			clock-names = "ahb", "mod", "ram";
>  			resets = <&ccu RST_BUS_VE>;
>  			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> -			allwinner,sram = <&ve_sram 1>;
> +			allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;

[Severity: Medium]
Will the second phandle be silently ignored during runtime?

Looking at the cedrus driver, it calls sunxi_sram_claim() during probe.
However, the underlying infrastructure appears to hardcode index 0 when
parsing this property:

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

Does the driver infrastructure need to be updated to parse multiple indices
before this device tree addition takes effect?

>  			iommus = <&iommu 3>;
>  		};
  
Chen-Yu Tsai July 11, 2026, 2:51 p.m. UTC | #2
On Sat, Jul 11, 2026 at 10:08 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Adding a second phandle to the `allwinner,sram` property has no effect at runtime because the sunxi_sram driver infrastructure hardcodes parsing to only the first index.
> --
>
> --- Patch [5]: [PATCH v2 5/7] arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine ---
> commit 8b2bcf328faa458d85d11e3cef06ddda86e2e10e
> Author: Chen-Yu Tsai <wens@kernel.org>
>
> arm64: dts: allwinner: sun50i-h6: Add missing SRAM region for video engine
>
> This commit adds a reference to the SRAM C region in the video engine node
> to prevent access through a partial alias window.
>
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> > @@ -205,7 +205,7 @@ video-codec@1c0e000 {
> >                       clock-names = "ahb", "mod", "ram";
> >                       resets = <&ccu RST_BUS_VE>;
> >                       interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > -                     allwinner,sram = <&ve_sram 1>;
> > +                     allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
>
> [Severity: Medium]
> Will the second phandle be silently ignored during runtime?
>
> Looking at the cedrus driver, it calls sunxi_sram_claim() during probe.
> However, the underlying infrastructure appears to hardcode index 0 when
> parsing this property:
>
> drivers/soc/sunxi/sunxi_sram.c:sunxi_sram_of_parse() {
>     ...
>     ret = of_parse_phandle_with_fixed_args(node, "allwinner,sram", 1, 0,
>                                            &args);

Yeah, no. Sashiko's context needs to be updated. This was changed in
commit be99eb936b4f ("soc: sunxi: sram: Support claiming multiple regions
per device") in v7.2-rc1.

>     ...
> }
>
> Does the driver infrastructure need to be updated to parse multiple indices
> before this device tree addition takes effect?
>
> >                       iommus = <&iommu 3>;
> >               };
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260711135959.3616919-1-wens@kernel.org?part=5
  

Patch

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 72ce1a75647b..88c6e3e105c0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -205,7 +205,7 @@  video-codec@1c0e000 {
 			clock-names = "ahb", "mod", "ram";
 			resets = <&ccu RST_BUS_VE>;
 			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
-			allwinner,sram = <&ve_sram 1>;
+			allwinner,sram = <&ve_sram 1>, <&de2_sram 1>;
 			iommus = <&iommu 3>;
 		};