Re: [PATCH v5] ufs: ufs-qcom: Align programming sequence of Shared ICE for UFS controller v5

Manivannan Sadhasivam
2025-09-05T13:13:45Z

On Mon, Aug 18, 2025 at 09:39:05AM GMT, Palash Kambar wrote:
> 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>

- Mani

>
> ---
> changes from V1:
> 1) Incorporated feedback from Konrad and Manivannan by adding a delay
> between ICE reset assertion and deassertion.
> 2) Removed magic numbers and replaced them with meaningful constants.
>
> changes from V2:
> 1) Addressed Manivannan's comment and moved change to ufs_qcom_resume.
>
> changes from V3:
> 1) Addressed Manivannan's comments and added bit field values and
> updated patch description.
>
> change from V4:
> 1) Addressed Konrad's comment and fixed reset bit to zero.
> ---
> 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 444a09265ded..242f8d479d4a 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 6840b7526cf5..81e2c2049849 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,
>
> --
> 2.34.1
>

--
மணிவண்ணன் சதாசிவம்