[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Yuezhang.Mo at sony.com]

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.

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;
 
-- 
2.25.1

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Andy.Wu at sony.com]

Reviewed-by: Andy Wu <Andy.Wu@sony.com>

Best Regards
Andy Wu

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of
> Yuezhang.Mo at sony.com
> Sent: Wednesday, March 17, 2021 2:45 PM
> To: u-boot at lists.denx.de
> Cc: peng.fan at nxp.com; panto at antoniou-consulting.com
> Subject: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are
> data"
> 
> 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.
> 
> 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;
> 
> --
> 2.25.1

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

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 <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;
>  
> 

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Andy.Wu at sony.com]

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.

> 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?

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 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 <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;
> >
> >
> 

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

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 <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 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 <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;
>>>
>>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Andy.Wu at sony.com]

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.

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 at samsung.com; Mo, Yuezhang
> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
> >>>
> >>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Peng Fan]

> 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.chung@gmail.com>
> > Sent: Monday, March 22, 2021 6:03 PM
> > To: Wu, Andy <Andy.Wu@sony.com>; jh80.chung at samsung.com; Mo,
> Yuezhang
> > <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
> > >>>
> > >>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

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 at samsung.com; Mo,
>> Yuezhang
>>> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
>>>>>>
>>>>>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

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 at samsung.com; Mo,
>> Yuezhang
>>> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
>>>>>>
>>>>>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Andy.Wu at sony.com]

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-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 at gmail.com;
> u-boot at 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 at samsung.com; Mo,
> >> Yuezhang
> >>> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
> >>>>>>
> >>>>>>

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

Dear Andy

On 5/11/21 4:39 PM, Andy.Wu at 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.)

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 at gmail.com;
>> u-boot at 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 at samsung.com; Mo,
>>>> Yuezhang
>>>>> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
>>>>>>>>
>>>>>>>>
> 

[PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Jaehoon Chung]

Dear Andy,

On 5/12/21 7:09 AM, Jaehoon Chung wrote:
> Dear Andy
> 
> On 5/11/21 4:39 PM, Andy.Wu at 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 at gmail.com;
>>> u-boot at 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 at samsung.com; Mo,
>>>>> Yuezhang
>>>>>> <Yuezhang.Mo@sony.com>; 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 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 <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 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 <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;
>>>>>>>>>
>>>>>>>>>
>>
> 
> 
> 

RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Andy.Wu]

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;
> >>>>>>>>>
> >>>>>>>>>
> >>
> >
> >
> >
> 

RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"

[Yuezhang.Mo]

Hi Peng Fan,

Jaehoon approved this patch.
Do you have more comment for merging?

-----Original Message-----
From: Peng Fan <peng.fan@nxp.com> 
Sent: Tuesday, April 6, 2021 6:03 PM
To: Wu, Andy <Andy.Wu@sony.com>; jh80.chung@gmail.com; jh80.chung@samsung.com; Mo, Yuezhang <Yuezhang.Mo@sony.com>; u-boot@lists.denx.de
Cc: cpgs@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.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;
> > >>>
> > >>>