From: Palash Kambar quic_pkambar@quicinc.com
[ Upstream commit 3126b5fd02270380cce833d06f973a3ffb33a69b ]
Disabling the AES core in Shared ICE is not supported during power collapse for UFS Host Controller v5.0, which may lead to data errors after Hibern8 exit. To comply with hardware programming guidelines and avoid this issue, issue a sync reset to ICE upon power collapse exit.
Hence follow below steps to reset the ICE upon exiting power collapse and align with Hw programming guide.
a. Assert the ICE sync reset by setting both SYNC_RST_SEL and SYNC_RST_SW bits in UFS_MEM_ICE_CFG
b. Deassert the reset by clearing SYNC_RST_SW in UFS_MEM_ICE_CFG
Signed-off-by: Palash Kambar quic_pkambar@quicinc.com Reviewed-by: Manivannan Sadhasivam mani@kernel.org Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Reasoning and code-specific analysis: - Fixes real data errors: The commit addresses data corruption “after Hibern8 exit” on Qualcomm UFS Host Controller v5.0 when the Shared ICE (Inline Crypto Engine) AES core state isn’t supported across power collapse. This is a user-visible, serious bug that directly affects storage reliability. - Small, localized change: The patch only touches the QCOM UFS variant and adds a precise reset sequence in the resume path, tightly scoped to the problematic hardware revision.
What changed - New hardware register and bit definitions: - Adds `UFS_MEM_ICE_CFG` (0x2600) to the QCOM vendor register map: drivers/ufs/host/ufs-qcom.h:85 - Adds ICE sync reset bit definitions local to the source: - `UFS_ICE_SYNC_RST_SEL` and `UFS_ICE_SYNC_RST_SW`: drivers/ufs/host/ufs-qcom.c:41-42 - Reset sequence on resume for UFS v5.0.0: - After enabling lane clocks (drivers/ufs/host/ufs-qcom.c:755-757), if the link is not active and the controller version is exactly 5.0.0, issue an ICE sync reset: - Assert reset by setting both `UFS_ICE_SYNC_RST_SEL | UFS_ICE_SYNC_RST_SW` into `UFS_MEM_ICE_CFG`: drivers/ufs/host/ufs- qcom.c:759-764 - Read back, clear both bits, sleep 50–100 µs to allow flops to settle, write back, and read again: drivers/ufs/host/ufs- qcom.c:764-773 - The gating condition confines the behavior to the exact affected hardware: `host->hw_ver.major == 5 && host->hw_ver.minor == 0 && host->hw_ver.step == 0` and only when the link is not active: drivers/ufs/host/ufs-qcom.c:759-763 - Correct ordering with ICE reinit: - The reset happens before `ufs_qcom_ice_resume(host)` (drivers/ufs/host/ufs-qcom.c:776), and `ufs_qcom_ice_resume()` calls `qcom_ice_resume()` which reinitializes HWKM and waits for BIST (drivers/soc/qcom/ice.c:274-287). This ensures a clean reinit after the reset.
Why this is safe for stable - Minimal risk, bounded scope: - The behavior only triggers for a specific hardware revision (v5.0.0) and only on a particular PM transition condition (link not active), minimizing regression risk to other platforms. - The register access is vendor-specific and does not affect other subsystems. - The added delay is tiny (50–100 µs), and the change is otherwise a single MMIO reset sequence. - Clearly a bug fix, not a feature: - No new capabilities or architectural changes. It aligns with the hardware programming guide to prevent data errors. - Maintains correct init sequence: - Reset is performed before ICE resume and HWKM init, ensuring keys and state are reprogrammed after reset. The resume path remains coherent.
Stable tree criteria - Important bugfix: Prevents data corruption on affected hardware. - Small and contained: Limited to `drivers/ufs/host/ufs-qcom.c` and `drivers/ufs/host/ufs-qcom.h`. - No broad side effects: Strict hardware version gating with link state check. - No API/ABI changes or architectural refactors.
Conclusion - This is a strong backport candidate that fixes a real, user-impacting bug with minimal and well-scoped changes.
drivers/ufs/host/ufs-qcom.c | 21 +++++++++++++++++++++ drivers/ufs/host/ufs-qcom.h | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 9574fdc2bb0fd..3ea6b08d2b526 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -38,6 +38,9 @@ #define DEEMPHASIS_3_5_dB 0x04 #define NO_DEEMPHASIS 0x0
+#define UFS_ICE_SYNC_RST_SEL BIT(3) +#define UFS_ICE_SYNC_RST_SW BIT(4) + enum { TSTBUS_UAWM, TSTBUS_UARM, @@ -751,11 +754,29 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) { struct ufs_qcom_host *host = ufshcd_get_variant(hba); int err; + u32 reg_val;
err = ufs_qcom_enable_lane_clks(host); if (err) return err;
+ if ((!ufs_qcom_is_link_active(hba)) && + host->hw_ver.major == 5 && + host->hw_ver.minor == 0 && + host->hw_ver.step == 0) { + ufshcd_writel(hba, UFS_ICE_SYNC_RST_SEL | UFS_ICE_SYNC_RST_SW, UFS_MEM_ICE_CFG); + reg_val = ufshcd_readl(hba, UFS_MEM_ICE_CFG); + reg_val &= ~(UFS_ICE_SYNC_RST_SEL | UFS_ICE_SYNC_RST_SW); + /* + * HW documentation doesn't recommend any delay between the + * reset set and clear. But we are enforcing an arbitrary delay + * to give flops enough time to settle in. + */ + usleep_range(50, 100); + ufshcd_writel(hba, reg_val, UFS_MEM_ICE_CFG); + ufshcd_readl(hba, UFS_MEM_ICE_CFG); + } + return ufs_qcom_ice_resume(host); }
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h index e0e129af7c16b..88e2f322d37d8 100644 --- a/drivers/ufs/host/ufs-qcom.h +++ b/drivers/ufs/host/ufs-qcom.h @@ -60,7 +60,7 @@ enum { UFS_AH8_CFG = 0xFC,
UFS_RD_REG_MCQ = 0xD00, - + UFS_MEM_ICE_CFG = 0x2600, REG_UFS_MEM_ICE_CONFIG = 0x260C, REG_UFS_MEM_ICE_NUM_CORE = 0x2664,