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

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

Posted by Palash Kambar 1 month, 3 weeks ago

Disable of AES core in Shared ICE is not supported during power
collapse for UFS Host Controller V5.0.

Hence follow below steps to reset the ICE upon exiting power collapse
and align with Hw programming guide.

a. Write 0x18 to UFS_MEM_ICE_CFG
b. Write 0x0 to UFS_MEM_ICE_CFG

Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>

---
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.
---
 drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
 drivers/ufs/host/ufs-qcom.h |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 444a09265ded..60bf5e60b747 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_RESET_ASSERT_VALUE	0x18
+#define UFS_ICE_RESET_DEASSERT_VALUE	0x00
+
 enum {
 	TSTBUS_UAWM,
 	TSTBUS_UARM,
@@ -756,6 +759,17 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	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_RESET_ASSERT_VALUE, UFS_MEM_ICE);
+		ufshcd_readl(hba, UFS_MEM_ICE);
+		usleep_range(50, 100);
+		ufshcd_writel(hba, UFS_ICE_RESET_DEASSERT_VALUE, UFS_MEM_ICE);
+		ufshcd_readl(hba, UFS_MEM_ICE);
+	}
+
 	return ufs_qcom_ice_resume(host);
 }
 
diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
index 6840b7526cf5..cc1324ce05c7 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				= 0x2600,
 	REG_UFS_MEM_ICE_CONFIG			= 0x260C,
 	REG_UFS_MEM_ICE_NUM_CORE		= 0x2664,
 
-- 
2.34.1

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

Posted by Manivannan Sadhasivam 1 month, 3 weeks ago

On Tue, Aug 12, 2025 at 02:47:14PM GMT, Palash Kambar wrote:
> Disable of AES core in Shared ICE is not supported during power
> collapse for UFS Host Controller V5.0.
> 

Could you please add more info on the issue observed?

> Hence follow below steps to reset the ICE upon exiting power collapse
> and align with Hw programming guide.
> 
> a. Write 0x18 to UFS_MEM_ICE_CFG
> b. Write 0x0 to UFS_MEM_ICE_CFG
> 

Please be explicit about the fields you are writing to.

> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
> 
> ---
> 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.
> ---
>  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>  drivers/ufs/host/ufs-qcom.h |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 444a09265ded..60bf5e60b747 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_RESET_ASSERT_VALUE	0x18
> +#define UFS_ICE_RESET_DEASSERT_VALUE	0x00

Please define the actual bits as per the documentation, not the value you are
writing. Here, you are changing two fields:

ICE_SYNC_RST_SEL BIT(3)
ICE_SYNC_RST_SW BIT(4)

> +
>  enum {
>  	TSTBUS_UAWM,
>  	TSTBUS_UARM,
> @@ -756,6 +759,17 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	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_RESET_ASSERT_VALUE, UFS_MEM_ICE);
> +		ufshcd_readl(hba, UFS_MEM_ICE);
> +		usleep_range(50, 100);

Please add a comment above the delay to make it clear that the delay is not as
per the doc:
		/*
		 * 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.
		 */

> +		ufshcd_writel(hba, UFS_ICE_RESET_DEASSERT_VALUE, UFS_MEM_ICE);
> +		ufshcd_readl(hba, UFS_MEM_ICE);
> +	}
> +
>  	return ufs_qcom_ice_resume(host);
>  }
>  
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index 6840b7526cf5..cc1324ce05c7 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				= 0x2600,

As the internal doc, this register is called UFS_MEM_ICE_CFG.

- Mani

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

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

Posted by Palash Kambar 1 month, 3 weeks ago


On 8/13/2025 3:25 PM, Manivannan Sadhasivam wrote:
> On Tue, Aug 12, 2025 at 02:47:14PM GMT, Palash Kambar wrote:
>> Disable of AES core in Shared ICE is not supported during power
>> collapse for UFS Host Controller V5.0.
>>
> 
> Could you please add more info on the issue observed?

Sure Mani.

> 
>> Hence follow below steps to reset the ICE upon exiting power collapse
>> and align with Hw programming guide.
>>
>> a. Write 0x18 to UFS_MEM_ICE_CFG
>> b. Write 0x0 to UFS_MEM_ICE_CFG
>>
> 
> Please be explicit about the fields you are writing to.
> 
>> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
>>
>> ---
>> 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.
>> ---
>>  drivers/ufs/host/ufs-qcom.c | 14 ++++++++++++++
>>  drivers/ufs/host/ufs-qcom.h |  2 +-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 444a09265ded..60bf5e60b747 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_RESET_ASSERT_VALUE	0x18
>> +#define UFS_ICE_RESET_DEASSERT_VALUE	0x00
> 
> Please define the actual bits as per the documentation, not the value you are
> writing. Here, you are changing two fields:
> 
> ICE_SYNC_RST_SEL BIT(3)
> ICE_SYNC_RST_SW BIT(4)

ok Mani.

>> +
>>  enum {
>>  	TSTBUS_UAWM,
>>  	TSTBUS_UARM,
>> @@ -756,6 +759,17 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	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_RESET_ASSERT_VALUE, UFS_MEM_ICE);
>> +		ufshcd_readl(hba, UFS_MEM_ICE);
>> +		usleep_range(50, 100);
> 
> Please add a comment above the delay to make it clear that the delay is not as
> per the doc:

Sure.

> 		/*
> 		 * 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.
> 		 */
> 
>> +		ufshcd_writel(hba, UFS_ICE_RESET_DEASSERT_VALUE, UFS_MEM_ICE);
>> +		ufshcd_readl(hba, UFS_MEM_ICE);
>> +	}
>> +
>>  	return ufs_qcom_ice_resume(host);
>>  }
>>  
>> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
>> index 6840b7526cf5..cc1324ce05c7 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				= 0x2600,
> 
> As the internal doc, this register is called UFS_MEM_ICE_CFG.

Ok will update the register name.


> - Mani
>