[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

Hi Peng,

On 4/6/21 7:02 PM, Peng Fan wrote:
>> Subject: RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there
>> are data"
>>
>> Hi Jaehoon
>>
>>> Did you test on latest u-boot?  v2018.01 was too old version.
>>>
>> Yes, we tested on v2020.04, although there is no such issue, but I think it just
>> depends on call sequence timing.
>>
>>> And if my understanding is right, INT_DATA_END needs to set when there
>>> is a data. If there is no data, it doesn't need to set to it. Logically, there is no
>> problem, isn't?
>>>
>> If there is no data, but current command is RESPONSE-WITH-BUSY (like CMD6)
>> type, the INT_DATA_END needs set also, refer sdhci spec explanation for
>> INT_DATA_END bit:
>>
>> Transfer Complete
>> This bit indicates stop of transaction on three cases:
>> ...
>> (2) Completion of a command pairing with response-with-busy (R1b, R5b)
>>
>> So, our modification just within if (cmd->resp_type & MMC_RSP_BUSY)
>> judgment.
> 
> Jaehoon,
> 
> Do you see any issue if revert the patch?

If you're ok, I will test after reverted the patch on tomorrow, and I will share result.
Or I will try to reproduce timeout issue on 410c board. 

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Peng.
> 
>>
>> Best Regards
>> Andy Wu
>>
>>> -----Original Message-----
>>> From: Jaehoon Chung <jh80.chung at gmail.com>
>>> Sent: Monday, March 22, 2021 6:03 PM
>>> To: Wu, Andy <Andy.Wu at sony.com>; jh80.chung at samsung.com; Mo,
>> Yuezhang
>>> <Yuezhang.Mo at sony.com>; u-boot at lists.denx.de
>>> Cc: peng.fan at nxp.com; cpgs at samsung.com
>>> Subject: Re: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when
>>> there are data"
>>>
>>> Hi Andy,
>>>
>>> On 3/18/21 10:59 AM, Andy.Wu at sony.com wrote:
>>>> Hi
>>>>
>>>>> I don't want to revert this commit. Is there any issue without this?
>>>> Without revert commit 17ea3c86, Some board, like Dragonboard 410c
>>>> will meet transfer data timeout error (we used v2018.01):
>>>>
>>>> U-Boot 2018.01 (Nov 26 2020 - 03:31:09 +0000) Qualcomm-DragonBoard
>>>> 410C
>>>>
>>>> DRAM:  986 MiB
>>>> MMC:   sdhci at 07824000: 0, sdhci at 07864000: 1
>>>> sdhci_transfer_data: Transfer data timeout
>>>> mmc_init: -70, time 10645
>>>> *** Warning - No block device, using default environment
>>>>
>>>> And it seems the 17ea3c86 not followed the sdhci specification as
>>>> transfer complete bit should be wait for the BUSY status de-assert.
>>>>
>>>> Kernel side code also wait the transfer complete bit for
>>>> response-with-busy command.
>>>
>>> Did you test on latest u-boot?  v2018.01 was too old version.
>>>
>>> And if my understanding is right, INT_DATA_END needs to set when there
>>> is a data.
>>>
>>> If there is no data, it doesn't need to set to it. Logically, there is no problem,
>> isn't?
>>>
>>> I will check with QC 410C board for clarifying this problem.
>>>
>>>>
>>>>> Without this patch, some SoCs have timeout error with stop command.
>>>> Sorry, we didn't meet this stop command timeout issue, but I guess
>>>> it maybe another issue, and can be fixed with modification limited
>>>> to stop command, not for all response-with-busy command.
>>>>
>>>> Does the SDHCI_QUIRK_BROKEN_R1B can be used for this case?
>>>
>>> Well, it can be used.
>>>
>>> Best Regards,
>>>
>>> Jaehoon Chung
>>>
>>>>
>>>> Best Regards
>>>> Andy Wu
>>>>
>>>>> -----Original Message-----
>>>>> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Jaehoon
>>>>> Chung
>>>>> Sent: Thursday, March 18, 2021 6:44 AM
>>>>> To: Mo, Yuezhang <Yuezhang.Mo at sony.com>; u-boot at lists.denx.de
>>>>> Cc: peng.fan at nxp.com; cpgs at samsung.com
>>>>> Subject: Re: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when
>>>>> there are data"
>>>>>
>>>>> Hi
>>>>>
>>>>> On 3/17/21 3:44 PM, Yuezhang.Mo at sony.com wrote:
>>>>>> This reverts commit 17ea3c862865c0d704646f67dbf8412f9ff54f59.
>>>>>>
>>>>>> In eMMC specification, for the response-with-busy(R1b, R5b)
>>>>>> command, the DAT0 will driven to LOW as BUSY status, and in sdhci
>>>>>> specification, the transfer complete bit should be wait for BUSY
>>>>>> status de-assert.
>>>>>>
>>>>>> All response-with-busy commands don't contain data, the data
>>>>>> judgement is no need.
>>>>>
>>>>> I don't want to revert this commit. Is there any issue without this?
>>>>> Without this patch, some SoCs have timeout error with stop command.
>>>>>
>>>>> To prevent it, it needs to increase timeout value at that time.
>>>>> (Timeout value can't fix each boards, waste time to find proper
>>>>> value, and be performance degradation.)
>>>>>
>>>>> I didn't test without this patch on latest U-boot.
>>>>> But if there is no critical issue, keep it, plz.
>>>>>
>>>>> Best Regards,
>>>>> Jaehoon Chung
>>>>>
>>>>>> Signed-off-by: Yuezhang.Mo <Yuezhang.Mo at sony.com>
>>>>>> ---
>>>>>>   drivers/mmc/sdhci.c | 3 +--
>>>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index
>>>>>> d9ab6a0a83..8568f65b18 100644
>>>>>> --- a/drivers/mmc/sdhci.c
>>>>>> +++ b/drivers/mmc/sdhci.c
>>>>>> @@ -258,8 +258,7 @@ static int sdhci_send_command(struct mmc
>> *mmc,
>>>>> struct mmc_cmd *cmd,
>>>>>>   		flags = SDHCI_CMD_RESP_LONG;
>>>>>>   	else if (cmd->resp_type & MMC_RSP_BUSY) {
>>>>>>   		flags = SDHCI_CMD_RESP_SHORT_BUSY;
>>>>>> -		if (data)
>>>>>> -			mask |= SDHCI_INT_DATA_END;
>>>>>> +		mask |= SDHCI_INT_DATA_END;
>>>>>>   	} else
>>>>>>   		flags = SDHCI_CMD_RESP_SHORT;
>>>>>>
>>>>>>