[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;
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@sony.com Sent: Wednesday, March 17, 2021 2:45 PM To: u-boot@lists.denx.de Cc: peng.fan@nxp.com; panto@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;-- 2.25.1
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;
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.
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@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;
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;
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@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;
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;
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;
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;
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@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; > >
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.)
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; >> >>
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; >>> >>>
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; >>>> >>>>
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;
participants (5)
-
Andy.Wu＠sony.com -
Jaehoon Chung -
Jaehoon Chung -
Peng Fan -
Yuezhang.Mo＠sony.com