arm64: dts: qcom: sm8150: Add standalone ICE node for UFS
Subsystems (5)
cover arm64: dts: qcom: sm8150: Add standalone ICE node for UFS
Hi everyone,
This small series brings the Qualcomm Inline Crypto Engine (ICE) to the SM8150 (Snapdragon 855) platform, following the same standalone-node style already used on SM8250 and newer SoCs.
The first patch documents the new compatible in the binding, and the second converts the UFS node to reference a dedicated ICE node instead of embedding the registers directly.
I've tested this on a Google Pixel 4 (google-flame) and everything works:/sys/block/sda/queue/crypto now advertises AES-256-XTS and ext4 reports encryption as "supported". The ICE self-test completes cleanly during boot.
Happy to adjust anything or add more details if needed. Thanks for reviewing!
Andrew Gigena (2):
dt-bindings: crypto: ice: Document sm8150 inline crypto engine
arm64: dts: qcom: sm8150: Use standalone ICE node for UFS
--- .../devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml | 1 + arch/arm64/boot/dts/qcom/sm8150.dtsi | 21 ++++++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-)
To: Abel Vesa <[email protected]>
To: Herbert Xu <[email protected]>
To: "David S. Miller" <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Bjorn Andersson <[email protected]>
To: Konrad Dybcio <[email protected]>
---
Andrew Gigena (2):
dt-bindings: crypto: ice: Document sm8150 inline crypto engine
arm64: dts: qcom: sm8150: Use standalone ICE node for UFS
.../bindings/crypto/qcom,inline-crypto-engine.yaml | 1 + arch/arm64/boot/dts/qcom/sm8150.dtsi | 25 ++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261104-sm8150-ice-rfc-0b4b2c035e80
Best regards,
Signature · 1 line
Andrew Gigena <[email protected]>
PATCH 1/2 dt-bindings: crypto: ice: Document sm8150 inline crypto engine
Document the compatible used for the inline crypto engine found on SM8150.
--- Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml | 1 + 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml +1 -0
@@ -23,6 +23,7 @@ properties: - qcom,sa8775p-inline-crypto-engine - qcom,sc7180-inline-crypto-engine - qcom,sc7280-inline-crypto-engine + - qcom,sm8150-inline-crypto-engine - qcom,shikra-inline-crypto-engine - qcom,sm8450-inline-crypto-engine - qcom,sm8550-inline-crypto-engine
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH 1/2] dt-bindings: crypto: ice: Document sm8150 inline crypto engine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] dt-bindings: crypto: ice: sm8150 standalone ICE conflicts with qcom,ufs.yaml - [Low]…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: crypto: ice: sm8150 standalone ICE conflicts with qcom,ufs.yaml
- [Low] dt-bindings: crypto: ice: qcom,sm8150-inline-crypto-engine out of sort order --
commit 8ecc5836840659392f33486ed20f5aa5057a1a4c
Author: Andrew Gigena <[email protected]>
dt-bindings: crypto: ice: Document sm8150 inline crypto engine
This commit documents the qcom,sm8150-inline-crypto-engine compatible string in Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml.
Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml properties: · line 26
Quoted 9 lines
Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml +1
-0@@ -23,6 +23,7 @@ properties: - qcom,sa8775p-inline-crypto-engine - qcom,sc7180-inline-crypto-engine - qcom,sc7280-inline-crypto-engine + - qcom,sm8150-inline-crypto-engine
[Severity: Medium]
Does adding a standalone ICE compatible for SM8150 conflict with the existing qcom,sm8150-ufshc binding in Documentation/devicetree/bindings/ufs/qcom,ufs.yaml?
On SM8150, the inline crypto engine is associated with the UFS controller (GCC only provides GCC_UFS_PHY_ICE_CORE_CLK, with no SDCC ICE clock in include/dt-bindings/clock/qcom,gcc-sm8150.h).
In Documentation/devicetree/bindings/ufs/qcom,ufs.yaml, qcom,sm8150-ufshc is defined as using the legacy embedded ICE representation:
if:
properties:
compatible:
contains:
enum:
- qcom,msm8996-ufshc
- qcom,sdm845-ufshc
- qcom,sm6350-ufshc
- qcom,sm8150-ufshc
then:
properties:
clocks:
minItems: 9
maxItems: 9
clock-names:
items:
- const: core_clk
...
- const: ice_core_clk
reg:
minItems: 2
reg-names:
items:
- const: std
- const: ice
In the same qcom,ufs.yaml schema, the conditional rule for nodes using the standalone ICE binding via qcom,ice requires at most 1 reg entry and 8 clocks, which is mutually exclusive with the qcom,sm8150-ufshc constraints:
if:
required:
- qcom,ice
then:
properties:
clocks:
maxItems: 8
clock-names:
maxItems: 8
reg:
maxItems: 1
reg-names:
maxItems: 1
When ufshc@1d84000 in arch/arm64/boot/dts/qcom/sm8150.dtsi is converted to a standalone qcom,sm8150-inline-crypto-engine node referenced via qcom,ice and the embedded ice register range and ice_core_clk are removed from the UFS node, won't this fail dtbs_check against qcom,ufs.yaml and break Device Tree ABI compatibility with existing kernels expecting the embedded ICE registers and clock on qcom,sm8150-ufshc?
Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml properties: · line 27
Quoted 1 line
Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml +0
-0- qcom,shikra-inline-crypto-engine
[Severity: Low]
This isn't a bug, but the compatible enum in properties:compatible in Documentation/devicetree/bindings/crypto/qcom,inline-crypto-engine.yaml is sorted alphabetically. Should qcom,sm8150-inline-crypto-engine be placed after qcom,shikra-inline-crypto-engine and before qcom,sm8450-inline-crypto-engine?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH 2/2 arm64: dts: qcom: sm8150: Use standalone ICE node for UFS
In reply to Andrew GigenaConvert SM8150 UFS to use a standalone ICE node.
Move the ICE register region and clock from the UFS controller node into a dedicated ICE node. Reference the new node from the UFS controller using the qcom,ice property.
This brings the SM8150 device tree in line with the standalone ICE representation used on newer Qualcomm platforms.
--- arch/arm64/boot/dts/qcom/sm8150.dtsi | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
arch/arm64/boot/dts/qcom/sm8150.dtsi +16 -9
@@ -2071,9 +2071,7 @@ pcie1_phy: phy@1c0e000 { ufs_mem_hc: ufshc@1d84000 { compatible = "qcom,sm8150-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; - reg = <0 0x01d84000 0 0x2500>, - <0 0x01d90000 0 0x8000>; - reg-names = "std", "ice"; + reg = <0 0x01d84000 0 0x2500>; interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; phys = <&ufs_mem_phy>; phy-names = "ufsphy";
@@ -2092,8 +2090,7 @@ ufs_mem_hc: ufshc@1d84000 { "ref_clk", "tx_lane0_sync_clk", "rx_lane0_sync_clk", - "rx_lane1_sync_clk", - "ice_core_clk"; + "rx_lane1_sync_clk"; clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
@@ -2102,8 +2099,7 @@ ufs_mem_hc: ufshc@1d84000 { <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>, - <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>, - <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>; freq-table-hz = <37500000 300000000>, <0 0>,
@@ -2112,8 +2108,8 @@ ufs_mem_hc: ufshc@1d84000 { <0 0>, <0 0>, <0 0>, - <0 0>, - <0 300000000>; + <0 0>; + qcom,ice = <&ice>; status = "disabled"; };
@@ -2139,6 +2135,17 @@ ufs_mem_phy: phy@1d87000 { status = "disabled"; }; + ice: crypto@1d90000 { + compatible = "qcom,sm8150-inline-crypto-engine", + "qcom,inline-crypto-engine"; + reg = <0 0x01d90000 0 0x8000>; + clocks = <&gcc GCC_UFS_PHY_ICE_CORE_CLK>, + <&gcc GCC_UFS_PHY_AHB_CLK>; + clock-names = "core", + "iface"; + power-domains = <&gcc UFS_PHY_GDSC>; + }; + cryptobam: dma-controller@1dc4000 { compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0"; reg = <0 0x01dc4000 0 0x24000>;
Signature · 1 line
2.56.0
[email protected]
AI reviewRe: [PATCH 2/2] arm64: dts: qcom: sm8150: Use standalone ICE node for UFS
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] arm64: dts: qcom: sm8150: ufs_mem_hc violates qcom,ufs.yaml schema --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] arm64: dts: qcom: sm8150: ufs_mem_hc violates qcom,ufs.yaml schema --
commit 44282b2e2d420ebb8b4be7b6dd11cd5ecade575e
Author: Andrew Gigena <[email protected]>
arm64: dts: qcom: sm8150: Use standalone ICE node for UFS
This commit converts the SM8150 UFS controller node to use a standalone Inline Crypto Engine (ICE) node referenced via the qcom,ice property, moving the ICE register region and clock out of ufs_mem_hc.
Quoted 36 lines
arch/arm64/boot/dts/qcom/sm8150.dtsi +3
-7@@ -2071,9 +2071,7 @@ pcie1_phy: phy@1c0e000 { ufs_mem_hc: ufshc@1d84000 { compatible = "qcom,sm8150-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; - reg = <0 0x01d84000 0 0x2500>, - <0 0x01d90000 0 0x8000>; - reg-names = "std", "ice"; + reg = <0 0x01d84000 0 0x2500>; interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>; phys = <&ufs_mem_phy>; phy-names = "ufsphy";@@ -2092,8 +2090,7 @@ ufs_mem_hc: ufshc@1d84000 { "ref_clk", "tx_lane0_sync_clk", "rx_lane0_sync_clk", - "rx_lane1_sync_clk", - "ice_core_clk"; + "rx_lane1_sync_clk"; clocks = <&gcc GCC_UFS_PHY_AXI_CLK>, <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,@@ -2102,8 +2099,7 @@ ufs_mem_hc: ufshc@1d84000 { <&rpmhcc RPMH_CXO_CLK>, <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>, <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>, - <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>, - <&gcc GCC_UFS_PHY_ICE_CORE_CLK>; + <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>; freq-table-hz = <37500000 300000000>, <0 0>,
arch/arm64/boot/dts/qcom/sm8150.dtsi ufs_mem_hc: ufshc@1d84000 { · line 2112
Quoted 7 lines
arch/arm64/boot/dts/qcom/sm8150.dtsi +2
-2@@ -2112,8 +2108,8 @@ ufs_mem_hc: ufshc@1d84000 { <0 0>, <0 0>, <0 0>, - <0 0>, - <0 300000000>; + <0 0>; + qcom,ice = <&ice>;
[Severity: Low]
Does converting ufs_mem_hc here without updating the devicetree binding cause dtbs_check failures on SM8150 and SA8155P boards?
In Documentation/devicetree/bindings/ufs/qcom,ufs.yaml, the allOf entry matching qcom,sm8150-ufshc still requires 9 clocks (including ice_core_clk), 2 reg entries, and reg-names:
- if:
properties:
compatible:
contains:
enum:
- qcom,sdm845-ufshc
- qcom,sm6350-ufshc
- qcom,sm8150-ufshc
then:
properties:
clocks:
minItems: 9
maxItems: 9
clock-names:
items:
...
- const: ice_core_clk
reg:
minItems: 2
reg-names:
minItems: 2
required:
- reg-names
Meanwhile, the rule for qcom,ice in the same binding requires maxItems: 1 for reg and maxItems: 8 for clocks, which conflicts with the qcom,sm8150-ufshc constraints above.
Should qcom,sm8150-ufshc be updated in the UFS binding schema (for example, moved to Documentation/devicetree/bindings/ufs/qcom,sc7180-ufshc.yaml alongside qcom,sm8250-ufshc and qcom,sm8450-ufshc) before or along with this change?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2