Re: [PATCH v8 10/11] arm64: dts: qcom: ipq5210: Add SPI node definitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] arm64: dts: qcom: ipq5210: 50 MHz SPI OPP cap breaks
  get_spi_clk_cfg()
--

commit 725b83be0f0b59dd5d0c719a4664cb25c7baa4d8
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>

arm64: dts: qcom: ipq5210: Add SPI node definitions

This commit adds the spi0 and spi1 QUPv3 GENI SPI controller nodes and a
50 MHz OPP table to ipq5210.dtsi.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> index 35cd558819691..61a58fd5d4ce3 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> @@ -168,6 +168,14 @@ opp-32000000 {
>  		};
>  	};
>  
> +	spi_opp_table_50mhz: opp-table-spi50mhz {
> +		compatible = "operating-points-v2";
> +
> +		opp-50000000 {
> +			opp-hz = /bits/ 64 <50000000>;
> +		};
> +	};
> +
>  	pmu {
>  		compatible = "arm,cortex-a53-pmu";
>  		interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> @@ -434,6 +442,18 @@ qupv3: geniqup@1ac0000 {
>  			#address-cells = <2>;
>  			#size-cells = <2>;
>  
> +			spi0: spi@1a80000 {
> +				compatible = "qcom,geni-spi";
> +				reg = <0x0 0x01a80000 0x0 0x4000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				clocks = <&gcc GCC_QUPV3_WRAP_SE0_CLK>;
> +				clock-names = "se";
> +				interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;
> +				operating-points-v2 = <&spi_opp_table_50mhz>;
> +				status = "disabled";
> +			};
> +
>  			uart1: serial@1a84000 {
>  				compatible = "qcom,geni-debug-uart";
>  				reg = <0x0 0x01a84000 0x0 0x4000>;
> @@ -470,6 +490,18 @@ i2c1: i2c@1a8c000 {
>  				status = "disabled";
>  			};
>  
> +			spi1: spi@1a90000 {
> +				compatible = "qcom,geni-spi";
> +				reg = <0x0 0x01a90000 0x0 0x4000>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				clocks = <&gcc GCC_QUPV3_WRAP_SE4_CLK>;
> +				clock-names = "se";
> +				interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>;
> +				operating-points-v2 = <&spi_opp_table_50mhz>;
> +				status = "disabled";
> +			};
> +
>  			i2c2: i2c@1a94000 {
>  				compatible = "qcom,geni-i2c";
>  				reg = <0x0 0x01a94000 0x0 0x4000>;

[Severity: Medium]
Will capping spi_opp_table_50mhz at 50 MHz cause get_spi_clk_cfg() to fail
with -ERANGE on spi0 and spi1?

Both GCC_QUPV3_WRAP_SE0_CLK and GCC_QUPV3_WRAP_SE4_CLK are backed by
ftbl_gcc_qupv3_wrap_se0_clk_src, which exposes rates above 50 MHz:

drivers/clk/qcom/gcc-ipq5210.c:ftbl_gcc_qupv3_wrap_se0_clk_src[] {
    ...
	F(50000000, P_GPLL0_OUT_MAIN, 16, 0, 0),
	F(51200000, P_GPLL0_OUT_MAIN, 1, 8, 125),
	F(56000000, P_GPLL0_OUT_MAIN, 1, 7, 100),
	F(58986175, P_GPLL0_OUT_MAIN, 1, 16, 217),
	F(60000000, P_GPLL0_OUT_MAIN, 1, 3, 40),
	F(64000000, P_GPLL0_OUT_MAIN, 12.5, 0, 0),
    ...
}

geni_se_clk_tbl_get() builds se->clk_perf_tbl via clk_round_rate() on the
SE clock without filtering against the device OPP table, so all entries up
to 64 MHz are recorded.  When a client requests a valid bus speed such as
30 MHz, get_spi_clk_cfg() -> geni_se_clk_freq_match() selects the 60 MHz
source clock with a divider of 2 and passes sclk_freq = 60000000 to
dev_pm_opp_set_rate(), which fails with -ERANGE because the highest OPP in
spi_opp_table_50mhz is 50000000 Hz, aborting the SPI transfer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-08-dts-v8-0-d91401955039@oss.qualcomm.com?part=10