Hi All
Since patch also reviewed by Jaehoon, how about merge it into mainline?
Best Regards Andy Wu
-----Original Message----- From: Jaehoon Chung jh80.chung@samsung.com Sent: Thursday, May 20, 2021 6:03 AM To: Wu, Andy Andy.Wu@sony.com; peng.fan@nxp.com; jh80.chung@gmail.com; u-boot@lists.denx.de Cc: CPGS cpgs@samsung.com Subject: Re: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
Dear Andy,
On 5/12/21 7:09 AM, Jaehoon Chung wrote:
Dear Andy
On 5/11/21 4:39 PM, Andy.Wu@sony.com wrote:
Hi Jaehoon
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.
Sorry, but is there any update for this comments?
Sorry for replying too late. I had been doing other things. So if you're ok, i will test on my own boards with your patch until this weekend. (If possible, i will also check 410c board.)
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
If a problem is occurred again, I will fix again.
Best Regards, Jaehoon Chung
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: Tuesday, April 6, 2021 7:13 PM To: Peng Fan peng.fan@nxp.com; jh80.chung@gmail.com; u-boot@lists.denx.de Subject: Re: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
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; >>>> } else >>>> flags = SDHCI_CMD_RESP_SHORT; >>>> >>>>