diff mbox series

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

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

Commit Message

Chen-Yu Tsai July 12, 2026, 3 a.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 12, 2026, 3:08 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The newly added `<&de2_sram 1>` phandle is silently ignored by the OS because the SRAM driver hardcodes parsing to only the first array element.
- [Medium] Assigning `de2_sram` to the Video Engine creates an unresolvable resource conflict with the Display Engine, guaranteeing a probe failure if the SRAM driver were to parse it.
--

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

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

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.

> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index 72ce1a75647bf..88c6e3e105c07 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>;

[Severity: Medium]
Will this second phandle actually be parsed by the OS?

Looking at the driver code in drivers/soc/sunxi/sunxi_sram.c, it seems
the parser hardcodes the index to 0:

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 need to be updated to parse multiple SRAM elements so
this alias window is actually closed?


[Severity: Medium]
If the driver is updated to parse this second element, will it cause a probe
failure due to a resource conflict?

The de2_sram region is already claimed by the Display Engine (bus@1000000).
Looking at sunxi_sram_claim(), the API enforces strict exclusive ownership:

drivers/soc/sunxi/sunxi_sram.c:sunxi_sram_claim() {
	...
	if (sram_desc->claimed) {
		spin_unlock(&sram_lock);
		return -EBUSY;
	}
	...
}

If both the Display Engine and Video Engine try to claim de2_sram, won't the
second device to probe be rejected with -EBUSY?

>  			iommus = <&iommu 3>;
>  		};
>
diff mbox series

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>;
 		};