[PATCH v4 1/7] scsi: ufs: core: Add UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE

[Shawn Lin]

Hi Mani,

在 2024/11/7 23:51, Manivannan Sadhasivam 写道:
> On Mon, Nov 04, 2024 at 03:31:55PM +0800, Shawn Lin wrote:
>> HCE on Rockchip SoC is different from both of ufshcd_hba_execute_hce()
>> and UFSHCI_QUIRK_BROKEN_HCE case. It need to do dme_reset and dme_enable
>> after enabling HCE. So in order not to abuse UFSHCI_QUIRK_BROKEN_HCE, add
>> a new quirk UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE, to deal with that
>> limitation.
>>
>> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
>> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com>
>> ---
>>
>> Changes in v4:
>> - fix typo
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>>   drivers/ufs/core/ufshcd.c | 17 +++++++++++++++++
>>   include/ufs/ufshcd.h      |  6 ++++++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 7cab1031..4084bf9 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -4819,6 +4819,7 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba)
>>   {
>>   	int retry_outer = 3;
>>   	int retry_inner;
>> +	int ret;
>>   
>>   start:
>>   	if (ufshcd_is_hba_active(hba))
>> @@ -4865,6 +4866,22 @@ static int ufshcd_hba_execute_hce(struct ufs_hba *hba)
>>   	/* enable UIC related interrupts */
>>   	ufshcd_enable_intr(hba, UFSHCD_UIC_MASK);
>>   
>> +	/*
>> +	 * Do dme_reset and dme_enable if a UFS host controller needs
>> +	 * this procedure to actually finish HCE.
>> +	 */
>> +	if (hba->quirks & UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE) {
>> +		ret = ufshcd_dme_reset(hba);
>> +		if (!ret) {
>> +			ret = ufshcd_dme_enable(hba);
>> +			if (ret)
>> +				dev_err(hba->dev,
>> +					"Failed to do dme_enable after HCE.\n");
> 
> Don't you need to return failure for this and below error paths? Probably you
> need to skip post change notification as well in the case of failure.

Oops, my bad.

> 
>> +		} else {
>> +			dev_err(hba->dev, "Failed to do dme_reset after HCE.\n");
>> +		}
>> +	}
>> +
>>   	ufshcd_vops_hce_enable_notify(hba, POST_CHANGE);
> 
> Is it possible for you to carry out dme_reset() and dme_enable() in the post
> change notifier of the rockchip glue driver? I'm trying to see if we can avoid
> having the quirk which is only specific to Rockchip.
> 

I will check and test this approach. Thanks.

> - Mani
> 
>>   
>>   	return 0;
>> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
>> index a95282b..e939af8 100644
>> --- a/include/ufs/ufshcd.h
>> +++ b/include/ufs/ufshcd.h
>> @@ -685,6 +685,12 @@ enum ufshcd_quirks {
>>   	 * single doorbell mode.
>>   	 */
>>   	UFSHCD_QUIRK_BROKEN_LSDBS_CAP			= 1 << 25,
>> +
>> +	/*
>> +	 * This quirk needs to be enabled if host controller need to
>> +	 * do dme_reset and dme_enable after hce.
>> +	 */
>> +	UFSHCI_QUIRK_DME_RESET_ENABLE_AFTER_HCE		= 1 << 26,
>>   };
>>   
>>   enum ufshcd_caps {
>> -- 
>> 2.7.4
>>
>