Re: [PATCH 2/4] arm64: dts: renesas: r9a09g056: Add ADC nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: undefined CPG module clocks and reset IDs in ADC nodes
- [High] arm64: dts: adc1 and adc2 nodes incorrectly share adc0 clocks and resets
- [Medium] arm64: dts: missing #io-channel-cells property in ADC nodes
--

commit 227934acdf450f4bc0b48d576add5c164d83a5ed
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

arm64: dts: renesas: r9a09g056: Add ADC nodes

This commit adds three ADC device nodes (adc0, adc1, and adc2) to the
Renesas RZ/V2N SoC device tree.

> diff --git a/arch/arm64/boot/dts/renesas/r9a09g056.dtsi b/arch/arm64/boot/dts/renesas/r9a09g056.dtsi
> index 77c2221a9e2a3..f03855f705312 100644
> --- a/arch/arm64/boot/dts/renesas/r9a09g056.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a09g056.dtsi
> @@ -626,6 +626,150 @@ wdt1: watchdog@14400000 {
>  			status = "disabled";
>  		};
>  
> +		adc0: adc@11c00000 {
> +			compatible = "renesas,r9a09g056-adc", "renesas,r9a09g057-adc";
> +			reg = <0 0x11c00000 0 0x400>;
> +			interrupts = <GIC_SPI 918 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 919 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 920 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "adi", "gbadi", "gcadi",
> +					  "cmpai", "cmpbi";
> +			clocks = <&cpg CPG_MOD 0x108>, <&cpg CPG_MOD 0x107>;
> +			clock-names = "adclk", "pclk";
> +			resets = <&cpg 0xf6>;

[Severity: High]
Are these CPG module clock IDs (0x107, 0x108) and reset ID (0xf6) defined
in the r9a09g056-cpg.c clock driver?

If they are not implemented, the rzg2l_adc driver will fail to probe or
infinitely defer when it calls clk_get() and
devm_reset_control_get_exclusive_deasserted() during probe.

> +			power-domains = <&cpg>;
> +			dmas = <&dmac0 0x24c8>, <&dmac1 0x24c8>, <&dmac2 0x24c8>,
> +			       <&dmac3 0x24c8>, <&dmac4 0x24c8>;
> +			dma-names = "rx", "rx", "rx", "rx", "rx";
> +			status = "disabled";
> +
> +			#address-cells = <1>;
> +			#size-cells = <0>;

[Severity: Medium]
Should there be an #io-channel-cells = <1>; property defined here?

Without it, the ADC nodes cannot function as IIO providers via device tree
phandles. Any consumer driver trying to reference the ADC via an io-channels
property will fail to parse when calling iio_channel_get() to look up its
provider.

[ ... ]
> +		adc1: adc@11c02800 {
[ ... ]
> +			clocks = <&cpg CPG_MOD 0x108>, <&cpg CPG_MOD 0x107>;
> +			clock-names = "adclk", "pclk";
> +			resets = <&cpg 0xf6>;
[ ... ]
> +		adc2: adc@11c02c00 {
[ ... ]
> +			clocks = <&cpg CPG_MOD 0x108>, <&cpg CPG_MOD 0x107>;
> +			clock-names = "adclk", "pclk";
> +			resets = <&cpg 0xf6>;

[Severity: High]
Do adc1 and adc2 use the exact same clocks and reset lines as adc0?

Since the rzg2l_adc driver requests exclusive reset lines via
devm_reset_control_get_exclusive_deasserted(), probing adc1 and adc2 will
fail with -EBUSY because adc0 already holds the exclusive reset line.

If they were shared instead, resetting one active ADC instance would
inadvertently reset the others, leading to state corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929124829.237997-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2