Re: [PATCH 03/11] arm64: dts: qcom: sm8650: Disable Inline Crypto Engine by default

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Disabling the Inline Crypto Engine (ICE) node by default in `sm8650.dtsi` breaks the UFS controller probe on any board that does not explicitly re-enable it. The `ufs_mem_hc` node retains the `qcom,ice = <&ice>;` property, so `of_parse_phandle()` successfully finds the disabled node. However, because the node is disabled, no platform device is created for it, causing `of_find_device_by_node()` to return NULL. As a result, `of_qcom_ice_get()` returns `-ENODEV` instead of the graceful fallback `-EOPNOTSUPP`. The UFS driver treats `-ENODEV` as a fatal error, completely failing the UFS probe.
--

commit 9f42720f6440c4701f3e9fd798297a81e20c12ba
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

arm64: dts: qcom: sm8650: Disable Inline Crypto Engine by default

This commit disables the Inline Crypto Engine (ICE) by default in the SoC
DTSI to prevent the UFS_PHY_GDSC from remaining enabled while the device is
idle. The ICE node is then explicitly enabled in the board-specific DTS
files that use it.

> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index b679fd8afb7d5..384aa2203dfa3 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -4142,6 +4142,8 @@ ice: crypto@1d88000 {
>  			clock-names = "core",
>  				      "iface";
>  			power-domains = <&gcc UFS_PHY_GDSC>;
> +
> +			status = "disabled";

[Severity: High]
Does disabling the ICE node by default break the UFS controller probe on boards
that do not explicitly re-enable it?

The ufs_mem_hc node retains the qcom,ice = <&ice>; property, so
of_parse_phandle() still finds the node. However, since the node is now disabled
by default, no platform device is created for it. 

When the UFS driver probes, of_qcom_ice_get() fails to find the platform device:

drivers/soc/qcom/ice.c:of_qcom_ice_get() {
    ...
	if (!pdev) {
		dev_err(dev, "Cannot find device node %s\n", node->name);
		return ERR_PTR(-ENODEV);
	}
    ...
}

This returns -ENODEV, which is then treated as a fatal error by the UFS driver
instead of a graceful fallback:

drivers/ufs/host/ufs-qcom.c:ufs_qcom_ice_init() {
    ...
	if (IS_ERR(ice)) {
		if (ice != ERR_PTR(-EOPNOTSUPP))
			return PTR_ERR(ice);

		dev_warn(dev, "Disabling inline encryption support\n");
		return 0;
	}
    ...
}

Because the driver only falls back gracefully on -EOPNOTSUPP, receiving -ENODEV
causes the entire UFS probe to fail. Could this lead to an unbootable system for
downstream or future boards that rely on the default DTSI configuration if their
root filesystem is on UFS?

>  		};
>  
>  		cryptobam: dma-controller@1dc4000 {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-b4-dts-qcom-disable-ice-by-default-v1-0-1d6e0c2f5a69@oss.qualcomm.com?part=3