[PATCH v8 0/3] Enable sdhc inline crypto engine for kodiak and monaco

[PATCH v8 0/3] Enable sdhc inline crypto engine for kodiak and monaco

[Kuldeep Singh]

The series is combination of below 2 series sent in past. Since, both
need to be picked together, combine them and send as one series.
Konrad and Krzysztof, request to review series again as I've made minor
changes only to this series.
- https://lore.kernel.org/lkml/20260608041650.541502-1-neeraj.soni@oss.qualcomm.com/
- https://lore.kernel.org/linux-arm-msm/20260409-ice_emmc_clock_addition-v2-0-90bbcc057361@oss.qualcomm.com/

Document and wire the SDHCI to ICE relationship on Qualcomm platforms
where ICE is modelled as a dedicated DT node.

This series:
- adds the qcom,ice phandle to the SDHCI binding and enforces the
  qcom,ice vs embedded-reg modelling rule,
- enables ICE for kodiak and monaco by wiring SDHC to the dedicated ICE
  node,
- adds interface clock and power-domain requirements for the ICE node in
  affected DTS files.

The ICE node is kept disabled at SoC .dtsi level and enabled in board
.dts files where the corresponding SDHC node is enabled. This keeps the
SoC description reusable and avoids enabling ICE on boards that do not
use that SDHC instance.

How this series was tested:
- make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \
  DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml \
  dt_binding_check
- make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CHECK_DTBS=y \
  qcom/monaco-arduino-monza.dtb qcom/monaco-evk.dtb \
  qcom/qcm6490-fairphone-fp5.dtb qcom/qcm6490-idp.dtb \
  qcom/qcm6490-particle-tachyon.dtb qcom/qcm6490-shift-otter.dtb \
  qcom/qcs6490-radxa-dragon-q6a.dtb qcom/qcs6490-rb3gen2.dtb \
  qcom/qcs6490-thundercomm-minipc-g1iot.dtb \
  qcom/qcs6490-thundercomm-rubikpi3.dtb qcom/qcs8300-ride.dtb \
  qcom/sc7280-crd-r3.dtb qcom/sc7280-herobrine-crd-pro.dtb \
  qcom/sc7280-herobrine-crd.dtb qcom/sc7280-herobrine-evoker-lte.dtb \
  qcom/sc7280-herobrine-evoker.dtb qcom/sc7280-herobrine-herobrine-r1.dtb \
  qcom/sc7280-herobrine-villager-r0.dtb \
  qcom/sc7280-herobrine-villager-r1-lte.dtb \
  qcom/sc7280-herobrine-villager-r1.dtb \
  qcom/sc7280-herobrine-zombie-lte.dtb \
  qcom/sc7280-herobrine-zombie-nvme-lte.dtb \
  qcom/sc7280-herobrine-zombie-nvme.dtb qcom/sc7280-herobrine-zombie.dtb \
  qcom/sc7280-idp.dtb qcom/sc7280-idp2.dtb \
  qcom/sm7325-motorola-dubai.dtb qcom/sm7325-nothing-spacewar.dtb

Changes in v8:
- Added iface clock, clock-names, power-domain and disabled status in
  kodiak and monaco ICE nodes.
- Enabled ICE in board DTS files where SDHC is enabled while keeping SoC
  ICE nodes disabled by default.
- Updated trailers to reflect co-development and sender sign-off order.
- Revalidated with dt_binding_check and CHECK_DTBS for all impacted DTS.
- Link to v7: https://lore.kernel.org/all/20260608041650.541502-1-neeraj.soni@oss.qualcomm.com/

Changes in v7:
- Rebased on latest linux-next (sdhci-msm.yaml renamed to qcom,sdhci-msm.yaml).
- Added links for previous versions.
- Link to v6: https://lore.kernel.org/all/20260310113557.348502-1-neeraj.soni@oss.qualcomm.com/

Changes in v6:
- Wrapped commit message for patch (1/3) as per Linux coding guidelines.
- Signed off the patch (3/3).
- Link to v5: https://lore.kernel.org/all/20260306093332.4193993-1-neeraj.soni@oss.qualcomm.com/

Changes in v5:
- Updated the constraint for SDHCI 'v4' vs rest to reflect the 'qcom,ice'
  constraint.
- Link to v4: https://lore.kernel.org/all/20260217052526.2335759-1-neeraj.soni@oss.qualcomm.com/

Changes in v4:
- Added a new patch (3/3) for device tree changes for Monaco SoC.
- Updated commit subject of cover letter to reflect "monaco".
- Removed the text description of constraints from "description:" for "qcom,ice" and
  wrapped the code.
- Corrected the schema code to reflect the constraint of "qcom,ice" usage properly.
- Link to v3: https://lore.kernel.org/all/20260206112053.3287756-1-neeraj.soni@oss.qualcomm.com/

Changes in v3:
- Described the purpose for phandle in "description:" for "qcom,ice".
- Re-added the "if: required:" description for "qcom,ice" with proper
  encoding.
- Corrected the uppercase for base address and reg address space for ICE DT node.
- Link to v2: https://lore.kernel.org/all/20260114094848.3790487-1-neeraj.soni@oss.qualcomm.com/

Changes in v2:
- Removed the "if: required:" description for "qcom,ice" dt-binding
  as the ICE node is optional.
- Corrected the ICE dt node entry according to the dt-binding description.
- Added test details.
- Link to v1: https://lore.kernel.org/all/20251124111914.3187803-1-neeraj.soni@oss.qualcomm.com/

Changes in v1:
- Updated the dt-binding for ICE node.
- Added the dt node for ICE for kodiak.

---
Neeraj Soni (3):
      dt-bindings: mmc: sdhci-msm: add ICE phandle
      arm64: dts: qcom: kodiak: enable inline crypto engine for SDHC
      arm64: dts: qcom: monaco: enable inline crypto engine for SDHC

 .../devicetree/bindings/mmc/qcom,sdhci-msm.yaml    | 95 +++++++++++++++-------
 arch/arm64/boot/dts/qcom/kodiak.dtsi               | 14 ++++
 arch/arm64/boot/dts/qcom/monaco-evk.dts            |  4 +
 arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi     |  4 +
 arch/arm64/boot/dts/qcom/monaco.dtsi               | 14 ++++
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts           |  4 +
 .../boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts     |  4 +
 arch/arm64/boot/dts/qcom/qcs8300-ride.dts          |  4 +
 arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts  |  4 +
 .../boot/dts/qcom/sc7280-herobrine-evoker.dtsi     |  4 +
 .../dts/qcom/sc7280-herobrine-herobrine-r1.dts     |  4 +
 .../boot/dts/qcom/sc7280-herobrine-villager.dtsi   |  4 +
 .../boot/dts/qcom/sc7280-herobrine-zombie.dtsi     |  4 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |  4 +
 14 files changed, 139 insertions(+), 28 deletions(-)
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260629-ice_emmc_support-b24c84cb5054

Best regards,
--  
Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>

[PATCH v8 1/3] dt-bindings: mmc: sdhci-msm: add ICE phandle

[Kuldeep Singh]

From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

On newer Qualcomm SoCs (starting with sc7280/kodiak), ICE is modelled
as a dedicated DT node. Add the optional qcom,ice phandle to reference
that node from the SDHCI controller.

When qcom,ice is present, disallow an embedded ICE register region in
the SDHCI node to avoid double modelling. Older SoCs without qcom,ice
remain valid.

Co-developed-by: Abel Vesa <abel.vesa@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
Co-developed-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
Signed-off-by: Abhinaba Rakshit <abhinaba.rakshit@oss.qualcomm.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 .../devicetree/bindings/mmc/qcom,sdhci-msm.yaml    | 95 +++++++++++++++-------
 1 file changed, 67 insertions(+), 28 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml
index bd558a11b792..b3fcc1673c10 100644
--- a/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml
+++ b/Documentation/devicetree/bindings/mmc/qcom,sdhci-msm.yaml
@@ -145,6 +145,11 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: platform specific settings for DLL_CONFIG reg.
 
+  qcom,ice:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to the Inline Crypto Engine (ICE) hardware block for this controller.
+
   iommus:
     minItems: 1
     maxItems: 8
@@ -198,35 +203,69 @@ allOf:
             enum:
               - qcom,sdhci-msm-v4
     then:
-      properties:
-        reg:
-          minItems: 2
-          items:
-            - description: Host controller register map
-            - description: SD Core register map
-            - description: CQE register map
-            - description: Inline Crypto Engine register map
-        reg-names:
-          minItems: 2
-          items:
-            - const: hc
-            - const: core
-            - const: cqhci
-            - const: ice
+      if:
+        required:
+          - qcom,ice
+      then:
+        properties:
+          reg:
+            minItems: 2
+            items:
+              - description: Host controller register map
+              - description: SD Core register map
+              - description: CQE register map
+          reg-names:
+            minItems: 2
+            items:
+              - const: hc
+              - const: core
+              - const: cqhci
+      else:
+        properties:
+          reg:
+            minItems: 2
+            items:
+              - description: Host controller register map
+              - description: SD Core register map
+              - description: CQE register map
+              - description: Inline Crypto Engine register map
+          reg-names:
+            minItems: 2
+            items:
+              - const: hc
+              - const: core
+              - const: cqhci
+              - const: ice
     else:
-      properties:
-        reg:
-          minItems: 1
-          items:
-            - description: Host controller register map
-            - description: CQE register map
-            - description: Inline Crypto Engine register map
-        reg-names:
-          minItems: 1
-          items:
-            - const: hc
-            - const: cqhci
-            - const: ice
+      if:
+        required:
+          - qcom,ice
+      then:
+        properties:
+          reg:
+            minItems: 1
+            items:
+              - description: Host controller register map
+              - description: CQE register map
+          reg-names:
+            minItems: 1
+            items:
+              - const: hc
+              - const: cqhci
+      else:
+        properties:
+          reg:
+            minItems: 1
+            items:
+              - description: Host controller register map
+              - description: CQE register map
+              - description: Inline Crypto Engine register map
+          reg-names:
+            minItems: 1
+            items:
+              - const: hc
+              - const: cqhci
+              - const: ice
 
 unevaluatedProperties: false
 

-- 
2.34.1

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

[Kuldeep Singh]

From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

Add the dedicated ICE node for kodiak and reference it from the SDHC
controller via qcom,ice.

Keep the ICE node disabled by default in kodiak.dtsi and enable it in
board DTS files where the corresponding SDHC node is enabled.

Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
Co-developed-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/kodiak.dtsi                       | 14 ++++++++++++++
 arch/arm64/boot/dts/qcom/qcm6490-idp.dts                   |  4 ++++
 arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts      |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts          |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi      |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi    |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-herobrine-zombie.dtsi      |  4 ++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi                   |  4 ++++
 9 files changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
index fa540d8c2615..633ebfd86352 100644
--- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
+++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
@@ -1050,6 +1050,8 @@ sdhc_1: mmc@7c4000 {
 			qcom,dll-config = <0x0007642c>;
 			qcom,ddr-config = <0x80040868>;
 
+			qcom,ice = <&sdhc_ice>;
+
 			mmc-ddr-1_8v;
 			mmc-hs200-1_8v;
 			mmc-hs400-1_8v;
@@ -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>;
+			status = "disabled";
+		};
+
 		gpi_dma0: dma-controller@900000 {
 			#dma-cells = <3>;
 			compatible = "qcom,sc7280-gpi-dma", "qcom,sm6350-gpi-dma";
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index bdc02260f902..1af10068edf5 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -1158,3 +1158,7 @@ &lpass_audiocc {
 	compatible = "qcom,qcm6490-lpassaudiocc";
 	/delete-property/ power-domains;
 };
+
+&sdhc_ice {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts b/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
index bb5a42b038f1..ef126e29f09d 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
+++ b/arch/arm64/boot/dts/qcom/qcs6490-radxa-dragon-q6a.dts
@@ -1093,3 +1093,7 @@ &sdc2_data {
 	bias-pull-up;
 	drive-strength = <10>;
 };
+
+&sdhc_ice {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
index df39a64da923..5b10ce277fcf 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts
@@ -136,6 +136,10 @@ &sdhc_2 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
 
 /*
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
index 0add7a2a099c..1bb0e4b8caac 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
@@ -98,6 +98,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
 
 &ts_rst_conn {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
index b04888a98203..78e0309b699c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dts
@@ -126,6 +126,10 @@ &sdhc_2 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
 
 /*
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
index 38c8a3679fcb..ff26dee6b804 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-villager.dtsi
@@ -88,6 +88,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
 
 &ts_rst_conn {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-zombie.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-zombie.dtsi
index a4fde22e3355..ecf518f2267c 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-zombie.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-zombie.dtsi
@@ -74,6 +74,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */
 
 &ts_rst_conn {
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index 8cac4ce9c851..dfefcad57f7a 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -744,6 +744,10 @@ &sdc2_data {
 	drive-strength = <10>;
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 &tlmm {
 	amp_en: amp-en-state {
 		pins = "gpio63";

-- 
2.34.1

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

[Kuldeep Singh]

From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>

Add the dedicated ICE node for monaco and reference it from the SDHC
controller via qcom,ice.

Keep the ICE node disabled by default in monaco.dtsi and enable it in
board DTS files where the corresponding SDHC node is enabled.

Signed-off-by: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
Co-developed-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/monaco-evk.dts        |  4 ++++
 arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi |  4 ++++
 arch/arm64/boot/dts/qcom/monaco.dtsi           | 14 ++++++++++++++
 arch/arm64/boot/dts/qcom/qcs8300-ride.dts      |  4 ++++
 4 files changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/monaco-evk.dts b/arch/arm64/boot/dts/qcom/monaco-evk.dts
index 9d17ef7d2caf..2c7d6ebc54fa 100644
--- a/arch/arm64/boot/dts/qcom/monaco-evk.dts
+++ b/arch/arm64/boot/dts/qcom/monaco-evk.dts
@@ -705,6 +705,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 &serdes0 {
 	phy-supply = <&vreg_l4a>;
 
diff --git a/arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi b/arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi
index 9b5ed55939b8..62c6f45025c1 100644
--- a/arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco-monza-som.dtsi
@@ -282,6 +282,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 /* Ethernet/SGMII */
 &serdes0 {
 	phy-supply = <&vreg_l5a>;
diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
index e4c8466f941b..71f4bb164ec3 100644
--- a/arch/arm64/boot/dts/qcom/monaco.dtsi
+++ b/arch/arm64/boot/dts/qcom/monaco.dtsi
@@ -4835,6 +4835,8 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			supports-cqe;
 			dma-coherent;
 
+			qcom,ice = <&sdhc_ice>;
+
 			mmc-ddr-1_8v;
 			mmc-hs200-1_8v;
 			mmc-hs400-1_8v;
@@ -4867,6 +4869,18 @@ opp-384000000 {
 			};
 		};
 
+		sdhc_ice: crypto@87c8000 {
+			compatible = "qcom,qcs8300-inline-crypto-engine",
+				     "qcom,inline-crypto-engine";
+			reg = <0x0 0x087c8000 0x0 0x18000>;
+			clocks = <&gcc GCC_SDCC1_ICE_CORE_CLK>,
+				 <&gcc GCC_SDCC1_AHB_CLK>;
+			clock-names = "core",
+				      "iface";
+			power-domains = <&rpmhpd RPMHPD_CX>;
+			status = "disabled";
+		};
+
 		usb_1_hsphy: phy@8904000 {
 			compatible = "qcom,qcs8300-usb-hs-phy",
 				     "qcom,usb-snps-hs-7nm-phy";
diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
index e9a8553a8d82..58c4a328bd3d 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
@@ -726,6 +726,10 @@ &sdhc_1 {
 	status = "okay";
 };
 
+&sdhc_ice {
+	status = "okay";
+};
+
 &tlmm {
 	bt_en_state: bt-en-state {
 		pins = "gpio55";

-- 
2.34.1

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

[sashiko-bot]

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

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

[Konrad Dybcio]

On 6/29/26 9:35 AM, Kuldeep Singh wrote:
> From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
> 
> Add the dedicated ICE node for kodiak and reference it from the SDHC
> controller via qcom,ice.
> 
> Keep the ICE node disabled by default in kodiak.dtsi and enable it in
> board DTS files where the corresponding SDHC node is enabled.

Is there a reason to disable it in the first place?

Keeping it enabled it would ensure that the core is parked
if it was left on by the bootloader

Konrad

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

[Kuldeep Singh]

On 29-06-2026 17:24, Konrad Dybcio wrote:
> On 6/29/26 9:35 AM, Kuldeep Singh wrote:
>> From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
>>
>> Add the dedicated ICE node for kodiak and reference it from the SDHC
>> controller via qcom,ice.
>>
>> Keep the ICE node disabled by default in kodiak.dtsi and enable it in
>> board DTS files where the corresponding SDHC node is enabled.
> 
> Is there a reason to disable it in the first place?
> 
> Keeping it enabled it would ensure that the core is parked
> if it was left on by the bootloader
Yes, observed an issue on kodiak Soc which supports both ufs-ice and
sdhc-ice.

CRYPTO_INLINE_ENGINE flag can be set to m via 2 configs MMC_CRYPTO and
SCSI_UFS_CRYPTO. In default defconfig, SCSI_UFS_CRYPTO is enabled
default and hence, CRYPTO_INLINE_ENGINE is enabled irrespective of
MMC_CRYPTO state.
https://elixir.bootlin.com/linux/v7.1.2/source/arch/arm64/configs/defconfig#L1325
https://elixir.bootlin.com/linux/v7.1.2/source/drivers/mmc/host/Kconfig#L614
https://elixir.bootlin.com/linux/v7.1.2/source/drivers/ufs/host/Kconfig#L62

Kodiak Soc supporting both ufs/sdhc results in initiating both ice
instances which in turn votes the clock in driver. In general, ice clock
suspend/resume sequence is later controller by ufs/sdhc controller once
the corresponding ice instance is initialised.

Since, qcs6490-rb3gen2(having only ufs storage) doesn't have emmc
storage but sdhc-ice DT node still probes the driver, later sdhci-msm
never takes ice instance(as sdhc_1 is disabled) but ice-clocks are
remained voted on.

This results in an issue and hence, trying to fix it by enabling ice
only when corresponding storage controller is enabled be it ufs or sdhc.
ufs/sdhc are default disabled and enabled per DTS, hence following same
convention with ice DT nodes too.

I am planning to update other DTs too with same change.
Suggestions are welcome!

-- 
Regards
Kuldeep

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

[Konrad Dybcio]

On 6/29/26 2:32 PM, Kuldeep Singh wrote:
> On 29-06-2026 17:24, Konrad Dybcio wrote:
>> On 6/29/26 9:35 AM, Kuldeep Singh wrote:
>>> From: Neeraj Soni <neeraj.soni@oss.qualcomm.com>
>>>
>>> Add the dedicated ICE node for kodiak and reference it from the SDHC
>>> controller via qcom,ice.
>>>
>>> Keep the ICE node disabled by default in kodiak.dtsi and enable it in
>>> board DTS files where the corresponding SDHC node is enabled.
>>
>> Is there a reason to disable it in the first place?
>>
>> Keeping it enabled it would ensure that the core is parked
>> if it was left on by the bootloader
> Yes, observed an issue on kodiak Soc which supports both ufs-ice and
> sdhc-ice.
> 
> CRYPTO_INLINE_ENGINE flag can be set to m via 2 configs MMC_CRYPTO and
> SCSI_UFS_CRYPTO. In default defconfig, SCSI_UFS_CRYPTO is enabled
> default and hence, CRYPTO_INLINE_ENGINE is enabled irrespective of
> MMC_CRYPTO state.
> https://elixir.bootlin.com/linux/v7.1.2/source/arch/arm64/configs/defconfig#L1325
> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/mmc/host/Kconfig#L614
> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/ufs/host/Kconfig#L62
> 
> Kodiak Soc supporting both ufs/sdhc results in initiating both ice
> instances which in turn votes the clock in driver. In general, ice clock
> suspend/resume sequence is later controller by ufs/sdhc controller once
> the corresponding ice instance is initialised.
> 
> Since, qcs6490-rb3gen2(having only ufs storage) doesn't have emmc
> storage but sdhc-ice DT node still probes the driver, later sdhci-msm
> never takes ice instance(as sdhc_1 is disabled) but ice-clocks are
> remained voted on.
> 
> This results in an issue and hence, trying to fix it by enabling ice
> only when corresponding storage controller is enabled be it ufs or sdhc.
> ufs/sdhc are default disabled and enabled per DTS, hence following same
> convention with ice DT nodes too.
> 
> I am planning to update other DTs too with same change.
> Suggestions are welcome!

qcom_ice_probe()
  -> qcom_ice_create()
    -> devm_clk_get_optional_enabled()

If we remove the _enabled suffix and put a ice_resume() in ice_get(),
I believe this is no longer an issue

Konrad

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

[Kuldeep Singh]

> [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?

The error reported seems legit.
Adding power-domains property but not implementing pm_runtime ops will
eventually avoid going to power down.
I cross-checked genpd behavior on pakala and observed erroneous behavior.
root@qcom-armv8a:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary |
grep -A 10 "ufs_phy"
gcc_ufs_phy_gdsc                on                              0
    1d88000.crypto             unsupported                      0    SW
    1d84000.ufs                suspended                        0    SW

The fix will be needed in driver and is generic across all platforms
with ufs and sdhc storage.
I will send fix separately for this and not in this series.
-- 
Regards
Kuldeep

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

[Kuldeep Singh]

> qcom_ice_probe()
>   -> qcom_ice_create()
>     -> devm_clk_get_optional_enabled()
> 
> If we remove the _enabled suffix and put a ice_resume() in ice_get(),
> I believe this is no longer an issue

I see your point.
devm_clk_get_optional_enabled turns clocks on, whereas using
devm_clk_get_optioanl will only get handle but don't enable clock.
Please note, clocks are needed at probe to read ice hw version and then
later ufs/sdhc fetch ice instance using of_qcom_ice_get() and control
suspend/resume path.

But my idea is, if storage itself isn't there(like sdhc on
qcs6490-rb3gen2) then why sdhc-ice should even probe and check hardware
version? as there's no significance in even probing ice.

-- 
Regards
Kuldeep

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

[Konrad Dybcio]

On 6/30/26 12:23 PM, Kuldeep Singh wrote:
>> qcom_ice_probe()
>>   -> qcom_ice_create()
>>     -> devm_clk_get_optional_enabled()
>>
>> If we remove the _enabled suffix and put a ice_resume() in ice_get(),
>> I believe this is no longer an issue
> 
> I see your point.
> devm_clk_get_optional_enabled turns clocks on, whereas using
> devm_clk_get_optioanl will only get handle but don't enable clock.
> Please note, clocks are needed at probe to read ice hw version 

Right, but we don't need them afterwards in the probe function. We
can simply gate them. There's a parallel effort to enable runtime PM
in the driver:

https://lore.kernel.org/linux-arm-msm/20260512033750.3393050-3-linlin.zhang@oss.qualcomm.com/

and with that all you'd need after getting the clock is a
pm_runtime_resume_and_get() before the version check, followed by a
pm_runtime_put() right after it


> and then
> later ufs/sdhc fetch ice instance using of_qcom_ice_get() and control
> suspend/resume path.
> 
> But my idea is, if storage itself isn't there(like sdhc on
> qcs6490-rb3gen2) then why sdhc-ice should even probe and check hardware
> version? as there's no significance in even probing ice.

On the developer/customer experience side, would you expect having to
manually enable what's essentially a sub-feature of the storage media
on every single board?

Konrad