Hi Peng Fan,
Jaehoon approved this patch.
Do you have more comment for merging?
-----Original Message-----
From: Peng Fan <peng....@nxp.com>
Sent: Tuesday, April 6, 2021 6:03 PM
To: Wu, Andy <andy...@sony.com>; jh80.ch...@gmail.com; jh80.ch...@samsung.com;
Mo, Yuezhang <yuezhang...@sony.com>; u-boot@lists.denx.de
Cc: c...@samsung.com
Subject: RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are
data"
> 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?
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;
> > >>>
> > >>>
