Re: [PATCH RFC RESEND v5 5/6] arm64: dts: amlogic: Add video decoder driver support for S4 SOCs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch silently introduces an `optee` node which is not mentioned in the commit message, and places it incorrectly at the root of the device tree.
- [Low] The newly added `canvas` and `video-codec` nodes break the expected unit-address sorting within the `soc` simple-bus node.
--

commit 9f1bc8bd76d58f7598508700fcc37888ae1dbf0f
Author: Zhentao Guo <zhentao.guo@amlogic.com>

arm64: dts: amlogic: Add video decoder driver support for S4 SOCs

This commit adds the vdec node to enable Amlogic V4L2 stateless video
decoder support for S4 SOCs. It also introduces an optee node and a canvas
node required for the decoder.

> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> --- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
> @@ -96,6 +96,11 @@ saradc: adc@fe026000 {
>  		status = "disabled";
>  	};
>  
> +	optee {
> +		compatible = "linaro,optee-tz";
> +		method = "smc";
> +	};

[Severity: Medium]
Should this optee node addition be explicitly mentioned in the commit
message? The commit description currently only mentions adding the vdec
node.

Also, should this node be placed under a firmware node instead of at the root
level of the device tree?

>  	soc {
>  		compatible = "simple-bus";
>  		#address-cells = <2>;

[ ... ]

> @@ -907,5 +912,34 @@ emmc: mmc@fe08c000 {
>  			assigned-clocks = <&clkc_periphs CLKID_SD_EMMC_C>;
>  			assigned-clock-rates = <24000000>;
>  		};
> +
> +		canvas: video-lut@fe036048 {
> +			compatible = "amlogic,canvas";
> +			reg = <0x0 0xfe036048 0x0 0x14>;
> +		};
> +
> +		video-codec@fe320000 {

[Severity: Low]
Does appending these nodes break the expected unit-address sorting within the
soc simple-bus node? The canvas node at fe036048 and the video-codec node at
fe320000 appear to be placed at the end of the block, after mmc@fe08c000,
rather than being ordered sequentially.

> +			compatible = "amlogic,s4-vdec";
> +			reg = <0x0 0xfe320000 0x0 0x10000>,
> +			      <0x0 0xfe036000 0x0 0x20>;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-b4-s4-vdec-upstream-v5-0-33bc817f93f4@amlogic.com?part=5