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

[Andy.Wu at sony.com]

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.

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