From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Date: Mon, 22 Mar 2021 19:02:56 +0900 Subject: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data" In-Reply-To: References: <1891546521.01616028481381.JavaMail.epsvc@epcpadp4> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 On Behalf Of Jaehoon Chung >> Sent: Thursday, March 18, 2021 6:44 AM >> To: Mo, Yuezhang ; 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 >>> --- >>> 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; >>> >>>