Re: [PATCH 1/2] ARM: dts: renesas: r9a06g032: Describe SPI controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Target-only SPI controllers missing the `spi-slave` property and improperly configured as hosts.
- [Low] The `spi-max-frequency` property is improperly placed on SPI controller nodes.
--

commit 812adfb1b6b8e7de5a970209c3fa9a2a938214f8
Author: Wolfram Sang <wsa+renesas@sang-engineering.com>

ARM: dts: renesas: r9a06g032: Describe SPI controllers

This commit adds device tree nodes for the six SPI controllers on the Renesas
RZ/N1D SoC. The first four controllers are configured as hosts, while the
remaining two are designated as targets.

> diff --git a/arch/arm/boot/dts/renesas/r9a06g032.dtsi b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
> index 442ea26b40f53..e5c76b38a58e4 100644
> --- a/arch/arm/boot/dts/renesas/r9a06g032.dtsi
> +++ b/arch/arm/boot/dts/renesas/r9a06g032.dtsi
> @@ -563,6 +563,96 @@ gic: interrupt-controller@44101000 {
>  				<GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_HIGH)>;
>  		};
>  
> +		/* Controller only */
> +		spi1: spi@50005000 {
> +			compatible = "renesas,r9a06g032-spi", "renesas,rzn1-spi";
> +			reg = <0x50005000 0x200>;
> +			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&sysctrl R9A06G032_CLK_SPI0>, <&sysctrl R9A06G032_HCLK_SPI0>;
> +			clock-names = "ssi_clk", "pclk";
> +			power-domains = <&sysctrl>;
> +			spi-max-frequency = <12500000>;

[Severity: Low]
Does the spi-max-frequency property belong on the controller node?

According to the SPI bindings, spi-max-frequency is meant to specify the
maximum operating frequency of an SPI peripheral child node, rather than the
controller itself.

Setting it on the controller node might be ignored by the driver and could
trigger dtbs_check warnings. This same pattern appears to be repeated for
spi2 through spi6.

> +			num-cs = <4>;
> +			status = "disabled";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +		};

[ ... ]

> +		/* Target only */
> +		spi5: spi@50009000 {
> +			compatible = "renesas,r9a06g032-spi", "renesas,rzn1-spi";
> +			reg = <0x50009000 0x200>;
> +			interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&sysctrl R9A06G032_CLK_SPI4>, <&sysctrl R9A06G032_HCLK_SPI4>;
> +			clock-names = "ssi_clk", "pclk";
> +			power-domains = <&sysctrl>;
> +			spi-max-frequency = <12500000>;
> +			num-cs = <1>;
> +			status = "disabled";
> +			#address-cells = <1>;

[Severity: High]
Since this node is designated as "Target only", should it include the
spi-slave boolean property?

Without spi-slave, the DesignWare SPI core driver evaluates
device_property_read_bool(dev, "spi-slave") as false and unconditionally
configures the hardware as an SPI host. This would cause the controller to
incorrectly attempt to drive the SCK and CS lines.

Additionally, standard SPI slave bindings expect #address-cells = <0> and
generally do not use the num-cs property for slave nodes.

> +			#size-cells = <0>;
> +		};
> +
> +		/* Target only */
> +		spi6: spi@5000a000 {
> +			compatible = "renesas,r9a06g032-spi", "renesas,rzn1-spi";
> +			reg = <0x5000a000 0x200>;
> +			interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&sysctrl R9A06G032_CLK_SPI5>, <&sysctrl R9A06G032_HCLK_SPI5>;
> +			clock-names = "ssi_clk", "pclk";
> +			power-domains = <&sysctrl>;
> +			spi-max-frequency = <12500000>;
> +			num-cs = <1>;
> +			status = "disabled";
> +			#address-cells = <1>;

[Severity: High]
Is this node missing the spi-slave property as well?

Similar to the spi5 node, omitting spi-slave will cause the core driver to
configure this as a host instead of a target.

The num-cs and #address-cells properties here also appear to violate standard
target node bindings.

> +			#size-cells = <0>;
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612080354.57459-4-wsa+renesas@sang-engineering.com?part=1