Re: [PATCH v9 06/11] arm64: dts: qcom: ipq5210: Add ICE support to SDHCI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] arm64: dts: qcom: ipq5210: 300 MHz ICE OPP table attached to
  sdhc node
--

commit 26d7894a0ee84572c70f5300978aa25be22d94a5
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>

arm64: dts: qcom: ipq5210: Add ICE support to SDHCI

This commit adds the SDHCI inline crypto engine (ICE) node, a 300 MHz OPP
table, and CQE support for the IPQ5210 SDHCI controller, and enables ICE on
RDP504.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> index f3d688fded802..267255d2980a3 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> @@ -164,6 +164,14 @@ opp-1416000000 {
>  		};
>  	};
>  
> +	sdhc_opp_table_300mhz: opp-table-300mhz {
> +		compatible = "operating-points-v2";
> +
> +		opp-300000000 {
> +			opp-hz = /bits/ 64 <300000000>;
> +		};
> +	};
> +
>  	pmu {
>  		compatible = "arm,cortex-a53-pmu";
>  		interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> @@ -456,7 +464,25 @@ sdhc: mmc@7804000 {
>  			clock-names = "iface",
>  				      "core",
>  				      "xo";
> +
> +			qcom,ice = <&sdhc_ice>;
> +
>  			non-removable;
> +			operating-points-v2 = <&sdhc_opp_table_300mhz>;
> +			supports-cqe;
> +
> +			status = "disabled";
> +		};
> +
> +		sdhc_ice: crypto@7808000 {
> +			compatible = "qcom,ipq5210-inline-crypto-engine",
> +				     "qcom,inline-crypto-engine";
> +			reg = <0x0 0x07808000 0x0 0x2000>;
> +
> +			clocks = <&gcc GCC_SDCC1_ICE_CORE_CLK>,
> +				 <&gcc GCC_SDCC1_AHB_CLK>;
> +			clock-names = "core",
> +				      "iface";
>  
>  			status = "disabled";
>  		};

[Severity: Medium]
Is sdhc_opp_table_300mhz attached to the wrong node here?

sdhci_msm_probe() calls devm_pm_opp_set_clkname(&pdev->dev, "core") and
devm_pm_opp_of_add_table(&pdev->dev), which binds operating-points-v2 on
sdhc to sdhc's "core" clock (GCC_SDCC1_APPS_CLK).  In
drivers/clk/qcom/gcc-ipq5210.c, ftbl_gcc_sdcc1_apps_clk_src only supports
rates up to 200 MHz, whereas 300 MHz is the rate in
ftbl_gcc_sdcc1_ice_core_clk_src for GCC_SDCC1_ICE_CORE_CLK on sdhc_ice.

Attaching the 300 MHz OPP table to sdhc gives GCC_SDCC1_APPS_CLK an
unreachable 300 MHz OPP while leaving GCC_SDCC1_ICE_CORE_CLK on sdhc_ice
with no rate configuration (qcom_ice_create() only enables its "core" clock
and does not parse an OPP table).

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