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

[Jaehoon Chung]

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;
>>>
>>>