* [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-04-06 10:02 ` Peng Fan
@ 2021-04-06 11:11 ` Jaehoon Chung
2021-04-06 11:13 ` Jaehoon Chung
2021-09-06 10:13 ` Yuezhang.Mo
2 siblings, 0 replies; 14+ messages in thread
From: Jaehoon Chung @ 2021-04-06 11:11 UTC (permalink / raw)
To: u-boot
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;
>>>>>>
>>>>>>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-04-06 10:02 ` Peng Fan
2021-04-06 11:11 ` Jaehoon Chung
@ 2021-04-06 11:13 ` Jaehoon Chung
2021-05-11 7:39 ` Andy.Wu at sony.com
2021-09-06 10:13 ` Yuezhang.Mo
2 siblings, 1 reply; 14+ messages in thread
From: Jaehoon Chung @ 2021-04-06 11:13 UTC (permalink / raw)
To: u-boot
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;
>>>>>>
>>>>>>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-04-06 11:13 ` Jaehoon Chung
@ 2021-05-11 7:39 ` Andy.Wu at sony.com
2021-05-11 22:09 ` Jaehoon Chung
0 siblings, 1 reply; 14+ messages in thread
From: Andy.Wu at sony.com @ 2021-05-11 7:39 UTC (permalink / raw)
To: u-boot
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;
> >>>>>>
> >>>>>>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-05-11 7:39 ` Andy.Wu at sony.com
@ 2021-05-11 22:09 ` Jaehoon Chung
2021-05-19 22:03 ` Jaehoon Chung
0 siblings, 1 reply; 14+ messages in thread
From: Jaehoon Chung @ 2021-05-11 22:09 UTC (permalink / raw)
To: u-boot
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;
>>>>>>>>
>>>>>>>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-05-11 22:09 ` Jaehoon Chung
@ 2021-05-19 22:03 ` Jaehoon Chung
2021-07-07 6:49 ` Andy.Wu
0 siblings, 1 reply; 14+ messages in thread
From: Jaehoon Chung @ 2021-05-19 22:03 UTC (permalink / raw)
To: u-boot
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;
>>>>>>>>>
>>>>>>>>>
>>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-05-19 22:03 ` Jaehoon Chung
@ 2021-07-07 6:49 ` Andy.Wu
0 siblings, 0 replies; 14+ messages in thread
From: Andy.Wu @ 2021-07-07 6:49 UTC (permalink / raw)
To: jh80.chung, peng.fan, jh80.chung, u-boot; +Cc: cpgs
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;
> >>>>>>>>>
> >>>>>>>>>
> >>
> >
> >
> >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] Revert "mmc: sdhci: set to INT_DATA_END when there are data"
2021-04-06 10:02 ` Peng Fan
2021-04-06 11:11 ` Jaehoon Chung
2021-04-06 11:13 ` Jaehoon Chung
@ 2021-09-06 10:13 ` Yuezhang.Mo
2 siblings, 0 replies; 14+ messages in thread
From: Yuezhang.Mo @ 2021-09-06 10:13 UTC (permalink / raw)
To: peng.fan; +Cc: cpgs, Andy.Wu, jh80.chung, jh80.chung, u-boot
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;
> > >>>
> > >>>
^ permalink raw reply [flat|nested] 14+ messages in thread