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@gmail.com Sent: Monday, March 22, 2021 6:03 PM To: Wu, Andy Andy.Wu@sony.com; jh80.chung@samsung.com; Mo,
Yuezhang
Yuezhang.Mo@sony.com; u-boot@lists.denx.de Cc: peng.fan@nxp.com; cpgs@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@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@07824000: 0, sdhci@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@lists.denx.de On Behalf Of Jaehoon Chung Sent: Thursday, March 18, 2021 6:44 AM To: Mo, Yuezhang Yuezhang.Mo@sony.com; u-boot@lists.denx.de Cc: peng.fan@nxp.com; cpgs@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@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@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;