Re: [PATCH v9 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 146df0b74de51d5a0fb5f5d16a2e1f1177beaca7
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 nodes and a 50 MHz OPP
table to the IPQ5210 device tree.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> index 36f874e57b425..052c2f3df94db 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> @@ -181,6 +181,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)>;
> @@ -459,6 +467,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>;
> @@ -495,6 +515,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() ->
dev_pm_opp_set_rate() to fail with -ERANGE when geni_se_clk_freq_match()
picks a source clock rate above 50 MHz?

Both GCC_QUPV3_WRAP_SE0_CLK and GCC_QUPV3_WRAP_SE4_CLK use
ftbl_gcc_qupv3_wrap_se0_clk_src, which exposes five rates above 50 MHz:

drivers/clk/qcom/gcc-ipq5210.c:
	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() populates se->clk_perf_tbl via clk_round_rate()
without consulting the device OPP table, so geni_se_clk_freq_match() will
select 51.2..64 MHz source rates whenever an integer divider yields a
better match for the requested SPI bus speed (for example, 30 MHz =
60 MHz / 2 or 15 MHz = 60 MHz / 4).  When get_spi_clk_cfg() then calls
dev_pm_opp_set_rate(mas->dev, sclk_freq), _find_freq_ceil() returns
-ERANGE because the highest OPP in spi_opp_table_50mhz is 50 MHz, aborting
the SPI transfer.

Since spi_opp_table_50mhz carries no voltage, level, or bandwidth votes,
operating-points-v2 can be dropped (as on ipq5424.dtsi) so
dev_pm_opp_set_rate() falls back to clk_set_rate(), or the OPP ceiling can
be raised to 64 MHz if the SPI controller supports those source rates.

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