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?
Best Regards
Andy Wu
> -----Original Message-----
> From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Tuesday, April 6, 2021 7:13 PM
> To: Peng Fan <peng....@nxp.com>; jh80.ch...@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.ch...@gmail.com>
> >>> Sent: Monday, March 22, 2021 6:03 PM
> >>> To: Wu, Andy <andy...@sony.com>; jh80.ch...@samsung.com; Mo,
> >> Yuezhang
> >>> <yuezhang...@sony.com>; u-boot@lists.denx.de
> >>> Cc: peng....@nxp.com; c...@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...@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-boun...@lists.denx.de> On Behalf Of Jaehoon
> >>>>> Chung
> >>>>> Sent: Thursday, March 18, 2021 6:44 AM
> >>>>> To: Mo, Yuezhang <yuezhang...@sony.com>; u-boot@lists.denx.de
> >>>>> Cc: peng....@nxp.com; c...@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...@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...@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;
> >>>>>>
> >>>>>>
