Re: [PATCH v8 1/4] arm64: dts: renesas: r9a08g046: Enable TSU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: renesas: r9a08g046: tsu uses ADC index 0 instead of channel 8
- [Low] arm64: dts: renesas: r9a08g046: adc1 and tsu nodes out of unit-address order
--

commit 3f81876bc97a3da87790da10f43f37d051bfeeea
Author: Biju Das <biju.das.jz@bp.renesas.com>

arm64: dts: renesas: r9a08g046: Enable TSU

This commit adds #cooling-cells to the CPU nodes and describes the ADC1 and
TSU nodes along with a CPU thermal zone and cooling maps on r9a08g046.

> diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> index f33d5d7468ffa..35046c7dac547 100644
> --- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi

[ ... ]

> @@ -374,6 +378,37 @@ scif5: serial@1004e000 {
>  			status = "disabled";
>  		};
>  
> +		adc1: adc@108c0000 {
> +			compatible = "renesas,r9a08g046-adc1";
> +			reg = <0 0x108c0000 0 0x10000>;

[Severity: Low]
Should adc1 (adc@108c0000) and tsu (thermal@108d0000) be placed after gpu
(gpu@108b0000) and before cpg (clock-controller@11010000) rather than between
scif5 (the end of the scif0@100ac000 group) and i2c0 (i2c@100ae000) so the
soc child nodes stay in ascending unit-address order?

> +			interrupts = <GIC_SPI 378 IRQ_TYPE_EDGE_RISING>;
> +			clocks = <&cpg CPG_MOD R9A08G046_ADC1_ADCLK>,
> +				 <&cpg CPG_MOD R9A08G046_ADC1_PCLK>;
> +			clock-names = "adclk", "pclk";
> +			resets = <&cpg R9A08G046_ADC1_PRESETN>,
> +				 <&cpg R9A08G046_ADC1_ADRST_N>;
> +			reset-names = "presetn", "adrst-n";
> +			power-domains = <&cpg>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			#io-channel-cells = <1>;
> +
> +			channel@8 {
> +				reg = <8>;
> +			};
> +		};
> +
> +		tsu: thermal@108d0000 {
> +			compatible = "renesas,r9a08g046-tsu";
> +			reg = <0 0x108d0000 0 0x10000>;
> +			clocks = <&cpg CPG_MOD R9A08G046_TSU_PCLK>;
> +			resets = <&cpg R9A08G046_TSU_PRESETN>;
> +			power-domains = <&cpg>;
> +			#thermal-sensor-cells = <0>;
> +			io-channels = <&adc1 0>;
> +			io-channel-names = "tsu";
> +		};

[Severity: Medium]
Should io-channels in the tsu node reference hardware channel 8 (<&adc1 8>)
matching channel@8 under adc1, with rzg2l_adc_iio_info in
drivers/iio/adc/rzg2l_adc.c implementing .fwnode_xlate?

In arch/arm64/boot/dts/renesas/r9a08g046.dtsi, adc1 defines only channel@8
with reg = <8>, matching Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
and rzg3l_adc1_hw_params in drivers/iio/adc/rzg2l_adc.c. Similarly,
Documentation/devicetree/bindings/thermal/renesas,r9a08g045-tsu.yaml and
arch/arm64/boot/dts/renesas/r9a08g045.dtsi specify io-channels = <&adc 8>.

When rzg3s_thermal_probe() in drivers/thermal/renesas/rzg3s_thermal.c calls
devm_iio_channel_get(dev, "tsu"), __fwnode_iio_channel_get() in
drivers/iio/inkern.c checks for info->fwnode_xlate:

drivers/iio/adc/rzg2l_adc.c:
static const struct iio_info rzg2l_adc_iio_info = {
	.read_raw = rzg2l_adc_read_raw,
	.read_label = rzg2l_adc_read_label,
};

Because rzg2l_adc_iio_info omits .fwnode_xlate, __fwnode_iio_channel_get()
falls back to __fwnode_iio_simple_xlate(), which treats the io-channels cell
as an array index into indio_dev->channels[] (where num_channels is 1 and
chan_array[0].channel is 8) rather than matching chan->channel.

Does setting io-channels = <&adc1 0> bake the Linux IIO driver's internal
array index into the Device Tree ABI instead of the hardware channel number?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-usb_rzg3l-v8-0-ab109c4bd108@bp.renesas.com?part=1