Re: [PATCH v8 2/3] arm64: dts: qcom: kodiak: enable inline crypto engine for SDHC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Moving ICE to a standalone DT node with a dedicated `power-domains` property permanently prevents the core `SC7280_CX` domain from suspending.
--

commit 069378a35dbb2b4d5a77519ee0c458e99459094d
Author: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

arm64: dts: qcom: kodiak: enable inline crypto engine for SDHC

This commit adds the dedicated inline crypto engine (ICE) node for kodiak and
references it from the SDHC controller. It keeps the ICE node disabled by
default in kodiak.dtsi and enables it in the respective board DTS files.

> diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
> index fa540d8c2615d..633ebfd86352d 100644
> --- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
[ ... ]
> @@ -1076,6 +1078,18 @@ opp-384000000 {
>  			};
>  		};
>  
> +		sdhc_ice: crypto@7c8000 {
> +			compatible = "qcom,sc7280-inline-crypto-engine",
> +				     "qcom,inline-crypto-engine";
> +			reg = <0x0 0x007c8000 0x0 0x18000>;
> +			clocks = <&gcc GCC_SDCC1_ICE_CORE_CLK>,
> +				 <&gcc GCC_SDCC1_AHB_CLK>;
> +			clock-names = "core",
> +				      "iface";
> +			power-domains = <&rpmhpd SC7280_CX>;

[Severity: High]
This is a pre-existing issue, but does adding the power-domains property here
permanently prevent the SC7280_CX domain from suspending?

The genpd framework automatically powers on the SC7280_CX domain during the
qcom-ice driver's probe sequence. However, the qcom-ice driver in
drivers/soc/qcom/ice.c lacks pm_runtime support.

Without a call to pm_runtime_enable(), the device's disable_depth remains 1,
causing pm_runtime_suspended() to unconditionally return false. When
genpd_power_off() evaluates the domain's state to determine if it can be
suspended, it sees the qcom-ice device as active and aborts the power-off
sequence.

Could this keep the domain active indefinitely and prevent the SoC from
entering deep sleep states like CX retention or power collapse?

> +			status = "disabled";
> +		};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-ice_emmc_support-v8-0-1a26e1717b85@oss.qualcomm.com?part=2