[U-Boot] [PATCH] arm: mmc: increase MMC SDHCI read status timeout
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com --- as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
drivers/mmc/sdhci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 5c71ab8..aa4cd4f 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -127,6 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define CONFIG_SDHCI_READ_STATUS_TIMEOUT 1000
static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) @@ -243,9 +244,9 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break; } while (((stat & mask) != mask) && - (get_timer(start) < CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT)); + (get_timer(start) < CONFIG_SDHCI_READ_STATUS_TIMEOUT));
- if (get_timer(start) >= CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT) { + if (get_timer(start) >= CONFIG_SDHCI_READ_STATUS_TIMEOUT) { if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) return 0; else {
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/mmc/sdhci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 5c71ab8..aa4cd4f 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -127,6 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define CONFIG_SDHCI_READ_STATUS_TIMEOUT 1000
static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) @@ -243,9 +244,9 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break; } while (((stat & mask) != mask) &&
(get_timer(start) < CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT));
(get_timer(start) < CONFIG_SDHCI_READ_STATUS_TIMEOUT));
- if (get_timer(start) >= CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT) {
- if (get_timer(start) >= CONFIG_SDHCI_READ_STATUS_TIMEOUT) { if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) return 0; else {
Viele Grüße, Stefan
Hi Stefan,
On Tue, Jun 28, 2016 at 8:00 AM, Stefan Roese sr@denx.de wrote:
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Thanks for the review... I didn't want to touch the "performance" algorithm related to SDHCI_CMD_DEFAULT_TIMEOUT (which maybe should be in Kconfig). However, the retry loop related to SDHCI_READ_STATUS_TIMEOUT doesn't need to be in Kconfig -- it is just a define. Thanks, Steve
drivers/mmc/sdhci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 5c71ab8..aa4cd4f 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -127,6 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define CONFIG_SDHCI_READ_STATUS_TIMEOUT 1000
static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) @@ -243,9 +244,9 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break; } while (((stat & mask) != mask) &&
(get_timer(start) < CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT));
(get_timer(start) < CONFIG_SDHCI_READ_STATUS_TIMEOUT));
if (get_timer(start) >= CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT) {
if (get_timer(start) >= CONFIG_SDHCI_READ_STATUS_TIMEOUT) { if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) return 0; else {Viele Grüße, Stefan
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On Tue, Jun 28, 2016 at 01:30:09PM -0700, Steve Rae wrote:
Hi Stefan,
On Tue, Jun 28, 2016 at 8:00 AM, Stefan Roese sr@denx.de wrote:
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Thanks for the review... I didn't want to touch the "performance" algorithm related to SDHCI_CMD_DEFAULT_TIMEOUT (which maybe should be in Kconfig). However, the retry loop related to SDHCI_READ_STATUS_TIMEOUT doesn't need to be in Kconfig -- it is just a define.
... so how is this handled in the kernel? I'm assuming some DT property..
Hi Tom,
On Tue, Jun 28, 2016 at 1:34 PM, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2016 at 01:30:09PM -0700, Steve Rae wrote:
Hi Stefan,
On Tue, Jun 28, 2016 at 8:00 AM, Stefan Roese sr@denx.de wrote:
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Thanks for the review... I didn't want to touch the "performance" algorithm related to SDHCI_CMD_DEFAULT_TIMEOUT (which maybe should be in Kconfig). However, the retry loop related to SDHCI_READ_STATUS_TIMEOUT doesn't need to be in Kconfig -- it is just a define.
... so how is this handled in the kernel? I'm assuming some DT property..
-- Tom
( we discussed this in the other email thread; but I'll copy it here... )
It looks like (v.4.6) the code loops for max_loops=16, and it looks like the loop delay is created by a write (which does not exist in the U-Boot code): sdhci_writel(host, mask, SDHCI_INT_STATUS);
Thanks, Steve
On Tue, Jun 28, 2016 at 01:53:52PM -0700, Steve Rae wrote:
Hi Tom,
On Tue, Jun 28, 2016 at 1:34 PM, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2016 at 01:30:09PM -0700, Steve Rae wrote:
Hi Stefan,
On Tue, Jun 28, 2016 at 8:00 AM, Stefan Roese sr@denx.de wrote:
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Thanks for the review... I didn't want to touch the "performance" algorithm related to SDHCI_CMD_DEFAULT_TIMEOUT (which maybe should be in Kconfig). However, the retry loop related to SDHCI_READ_STATUS_TIMEOUT doesn't need to be in Kconfig -- it is just a define.
... so how is this handled in the kernel? I'm assuming some DT property..
-- Tom
( we discussed this in the other email thread; but I'll copy it here... )
I thought I saw this in another thread, thanks.
It looks like (v.4.6) the code loops for max_loops=16, and it looks like the loop delay is created by a write (which does not exist in the U-Boot code): sdhci_writel(host, mask, SDHCI_INT_STATUS);
Maybe we should rewrite the area in question, for the next release to be like the kernel?
On Tue, Jun 28, 2016 at 3:21 PM, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2016 at 01:53:52PM -0700, Steve Rae wrote:
Hi Tom,
On Tue, Jun 28, 2016 at 1:34 PM, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 28, 2016 at 01:30:09PM -0700, Steve Rae wrote:
Hi Stefan,
On Tue, Jun 28, 2016 at 8:00 AM, Stefan Roese sr@denx.de wrote:
Hi Steve,
On 27.06.2016 23:43, Steve Rae wrote:
Otherwise, ocassionally see errors like this: Flashing sparse image at offset 2078720 Flashing Sparse Image sdhci_send_command: Timeout for status update! mmc fail to send stop cmd write_sparse_image: Write failed, block #2181088 [0]
This does not affect the actual writing speed, which is controlled by the default value: CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT
It only increases the retries when reading: SDHCI_INT_STATUS to avoid the timeout error.
Signed-off-by: Steve Rae steve.rae@raedomain.com
as per the discussion in: http://lists.denx.de/pipermail/u-boot/2016-June/258966.html this supercedes: http://patchwork.ozlabs.org/patch/615994/
IIRC, I've suggested to move SDHCI_CMD_DEFAULT_TIMEOUT to Kconfig and use the old value as default value. So that you can overwrite it for your board / platform via your defconfig. But I have no strong feelings here - your current version also works for me and does not "clutter" the Kconfig subsystem with too many values. So:
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
Thanks for the review... I didn't want to touch the "performance" algorithm related to SDHCI_CMD_DEFAULT_TIMEOUT (which maybe should be in Kconfig). However, the retry loop related to SDHCI_READ_STATUS_TIMEOUT doesn't need to be in Kconfig -- it is just a define.
... so how is this handled in the kernel? I'm assuming some DT property..
-- Tom
( we discussed this in the other email thread; but I'll copy it here... )
I thought I saw this in another thread, thanks.
It looks like (v.4.6) the code loops for max_loops=16, and it looks like the loop delay is created by a write (which does not exist in the U-Boot code): sdhci_writel(host, mask, SDHCI_INT_STATUS);Maybe we should rewrite the area in question, for the next release to be like the kernel?
Maybe - I don't know enough about it -- it looks like there are interrupts involved.... ( Who could this be assigned to? ) I was just trying to get it to run without hitting the time out.... Thanks, Steve
-- Tom
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Steve,
@@ -127,6 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define CONFIG_SDHCI_READ_STATUS_TIMEOUT 1000
static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
When I see CONFIG_ prefix, I imagine this parameter can be configurable via Kconfig or board header, but this is not actually overridden.
Nor do I believe it should be moved to Kconfig, because I doubt it is a CONFIG.
Setting such things aside, one important thing is, this patch fixes my problem.
BTW, why did you add "arm: " to the git subject? Is this patch related to ARM?
Otherwise,
Tested-by: Masahiro Yamada yamada.masahiro@socionext.com
Hi Masahiro,
On Wed, Jun 29, 2016 at 4:52 AM, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Steve,
@@ -127,6 +127,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data, #define CONFIG_SDHCI_CMD_MAX_TIMEOUT 3200 #endif #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT 100 +#define CONFIG_SDHCI_READ_STATUS_TIMEOUT 1000
static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
When I see CONFIG_ prefix, I imagine this parameter can be configurable via Kconfig or board header, but this is not actually overridden.
that makes sense....
Nor do I believe it should be moved to Kconfig, because I doubt it is a CONFIG.
correct! so maybe the define should be ? : SDHCI_READ_STATUS_TIMEOUT
Setting such things aside, one important thing is, this patch fixes my problem.
BTW, why did you add "arm: " to the git subject? Is this patch related to ARM?
no (sorry) , I don't think so....
Otherwise,
Tested-by: Masahiro Yamada yamada.masahiro@socionext.com
-- Best Regards Masahiro Yamada
participants (4)
-
Masahiro Yamada -
Stefan Roese -
Steve Rae -
Tom Rini