Re: [PATCH V10] scsi: ufs: qcom: Enable UFS Shared ICE Feature

[Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>]

On 17-Jan-25 10:22 PM, Manivannan Sadhasivam wrote:
> On Thu, Jan 09, 2025 at 12:33:52PM +0530, Ram Kumar Dwivedi wrote:
> 
> [...]
> 
>>  static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
>> @@ -439,6 +465,7 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
>>  		err = ufs_qcom_check_hibern8(hba);
>>  		ufs_qcom_enable_hw_clk_gating(hba);
>>  		ufs_qcom_ice_enable(host);
>> +		ufs_qcom_config_ice_allocator(host);
> 
> This should be moved before ufs_qcom_ice_enable(), no?

Hi Mani,

This sequence is as per HPG guidelines.

Thanks,
Ram.

> 
>>  		break;
>>  	default:
>>  		dev_err(hba->dev, "%s: invalid status %d\n", __func__, status);
>> @@ -932,6 +959,14 @@ static void ufs_qcom_set_host_params(struct ufs_hba *hba)
>>  	host_params->hs_tx_gear = host_params->hs_rx_gear = ufs_qcom_get_hs_gear(hba);
>>  }
>>  
>> +static void ufs_qcom_set_host_caps(struct ufs_hba *hba)
>> +{
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +
>> +	if (host->hw_ver.major >= 0x5)
>> +		host->caps |= UFS_QCOM_CAP_ICE_CONFIG;
>> +}
>> +
>>  static void ufs_qcom_set_caps(struct ufs_hba *hba)
>>  {
>>  	hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
>> @@ -940,6 +975,8 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
>>  	hba->caps |= UFSHCD_CAP_WB_EN;
>>  	hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE;
>>  	hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
>> +
>> +	ufs_qcom_set_host_caps(hba);
>>  }
>>  
>>  /**
>> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
>> index b9de170983c9..2975a9e545fa 100644
>> --- a/drivers/ufs/host/ufs-qcom.h
>> +++ b/drivers/ufs/host/ufs-qcom.h
>> @@ -76,6 +76,12 @@ enum {
>>  	UFS_MEM_CQIS_VS		= 0x8,
>>  };
>>  
>> +/* QCOM UFS host controller Shared ICE registers */
>> +enum {
>> +	REG_UFS_MEM_ICE_CONFIG			= 0x260C,
>> +	REG_UFS_MEM_ICE_NUM_CORE		= 0x2664,
>> +};
>> +
> 
> No, I asked for this change:
> 
> ```
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index b9de170983c9..9d1c9da51688 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -50,6 +50,9 @@ enum {
>          */
>         UFS_AH8_CFG                             = 0xFC,
>  
> +       REG_UFS_MEM_ICE_CONFIG                  = 0x260C,
> +       REG_UFS_MEM_ICE_NUM_CORE                = 0x2664,
> +
>         REG_UFS_CFG3                            = 0x271C,
>  
>         REG_UFS_DEBUG_SPARE_CFG                 = 0x284C,
> ```
> 
>>  #define UFS_CNTLR_2_x_x_VEN_REGS_OFFSET(x)	(0x000 + x)
>>  #define UFS_CNTLR_3_x_x_VEN_REGS_OFFSET(x)	(0x400 + x)
>>  
>> @@ -110,6 +116,9 @@ enum {
>>  /* bit definition for UFS_UFS_TEST_BUS_CTRL_n */
>>  #define TEST_BUS_SUB_SEL_MASK	GENMASK(4, 0)  /* All XXX_SEL fields are 5 bits wide */
>>  
>> +/* bit definition for UFS Shared ICE config */
> 
> 'bit definition for REG_UFS_MEM_ICE_CONFIG register'

Hi Mani,

I have addressed this in the latest patchset.

Thanks,
Ram.

> 
> - Mani
>