Re: [PATCH v2 3/4] arm64: dts: renesas: r9a09g057: Add IVC and ISP nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: renesas: swapped vclk and hclk clock/reset mapping in mali-c55 ISP node
- [High] arm64: dts: renesas: missing power-domains property in mali-c55 ISP node
--

commit f1f95e2c7f1fc7d73c0351518cd19bf0ac773a31
Author: Daniel Scally <dan.scally@ideasonboard.com>

arm64: dts: renesas: r9a09g057: Add IVC and ISP nodes

Add the Input Video Control Block and Arm Mali-C55 ISP to the device
tree file for the RZ/V2H(P) SoC.

> diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> index 639693d464a77..f04180c35866a 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
[ ... ]
> +		isp: isp@16080000 {
> +			compatible = "arm,mali-c55";
> +			reg = <0 0x16080000 0 0x80000>;

[Severity: High]
Does this node need a power-domains property?

The isp node appears to omit power-domains = <&cpg>;. On Renesas SoCs, this
property attaches the device to the Clock Pulse Generator (CPG) generic power
domain, which is responsible for clearing the BUS_MSTOP bits during
pm_runtime_resume. 

Without it, pm_runtime_resume_and_get() in the driver might not trigger the
genpd domain hooks to clear MSTOP, which could cause register accesses to hang
the bus or leave the hardware inaccessible.

> +			clocks = <&cpg CPG_MOD 0xe2>, <&cpg CPG_MOD 0xe4>,
> +				 <&cpg CPG_MOD 0xe5>;
> +			clock-names = "vclk", "aclk", "hclk";

[Severity: High]
Are the vclk and hclk clock mappings swapped here?

The arm,mali-c55 driver expects vclk to be the video clock and hclk to be the
AHB register clock. Here, vclk is assigned to CPG_MOD 0xe2 (isp_0_reg_aclk)
and hclk is assigned to CPG_MOD 0xe5 (isp_0_isp_sclk). 

This appears to reverse their roles and could cause the driver to configure
the wrong clock frequencies.

> +			interrupts = <GIC_SPI 859 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 865 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 866 IRQ_TYPE_EDGE_RISING>;
> +			resets = <&cpg 0xd2>, <&cpg 0xd1>, <&cpg 0xd3>;
> +			reset-names = "vresetn", "aresetn", "hresetn";

[Severity: High]
Are the vresetn and hresetn reset mappings also swapped?

Similar to the clocks, this maps the register reset (0xd2 ISP_0_REG_ARESETN)
to vresetn and the ISP video reset (0xd3) to hresetn. This could lead to
reset domains being incorrectly managed, resulting in hardware initialization
failures.

> +			status = "disabled";
> +
> +			port {
> +				isp_in: endpoint {
> +					remote-endpoint = <&ivc_out>;
> +				};
> +			};
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-mali-c55-renesas-dts-v2-0-69f728a474a2@kernel.org?part=3