Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
Tested-by: Steve Rae s...@broadcom.com (does resolve the issue on our board!) On 14-06-27 02:37 AM, Pantelis Antoniou wrote: Hi Eli, On Jun 12, 2014, at 12:41 PM, Eli Billauer wrote: The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. Signed-off-by: Eli Billauer eli.billa...@gmail.com --- drivers/mmc/sdhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3125d13..80f3a91 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -226,6 +226,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, break; if (--retry == 0) break; + udelay(10); } while ((stat mask) != mask); if (retry == 0) { -- 1.7.2.3 Looking at the linux sources is no good, cause linux is interrupt driven. This delay is used because the driver is not interrupt driven, so you have to wait until the interrupt indication is delivered. The only reference to interrupt latency I found is related to tuning and is set to 50ms which I supposed is very pessimistic. I think a timeout of 100ms would be fine. Regards -- Pantelis ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
On Fri, Jun 27, 2014 at 4:37 AM, Pantelis Antoniou pantelis.anton...@gmail.com wrote: Hi Eli, On Jun 12, 2014, at 12:41 PM, Eli Billauer wrote: The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. Signed-off-by: Eli Billauer eli.billa...@gmail.com --- drivers/mmc/sdhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3125d13..80f3a91 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -226,6 +226,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, break; if (--retry == 0) break; + udelay(10); } while ((stat mask) != mask); if (retry == 0) { -- 1.7.2.3 Looking at the linux sources is no good, cause linux is interrupt driven. This delay is used because the driver is not interrupt driven, so you have to wait until the interrupt indication is delivered. The only reference to interrupt latency I found is related to tuning and is set to 50ms which I supposed is very pessimistic. I think a timeout of 100ms would be fine. I suspect the timeout of 100ms is fine (though it's always nice when we tie such numbers to something more concrete than: it works if I make it wait longer). My main point was that this actually *adds* 100ms to the preexisting timeout, instead of making the timeout ~100ms. If we reduced the number of checks and increased the delay, the delay would completely dominate the timeout loop, and total time would become closer to ~100ms. Andy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
Hi Eli, On Jun 12, 2014, at 12:41 PM, Eli Billauer wrote: The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. Signed-off-by: Eli Billauer eli.billa...@gmail.com --- drivers/mmc/sdhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3125d13..80f3a91 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -226,6 +226,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, break; if (--retry == 0) break; + udelay(10); } while ((stat mask) != mask); if (retry == 0) { -- 1.7.2.3 Looking at the linux sources is no good, cause linux is interrupt driven. This delay is used because the driver is not interrupt driven, so you have to wait until the interrupt indication is delivered. The only reference to interrupt latency I found is related to tuning and is set to 50ms which I supposed is very pessimistic. I think a timeout of 100ms would be fine. Regards -- Pantelis ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
On Thu, Jun 12, 2014 at 4:41 AM, Eli Billauer eli.billa...@gmail.com wrote: The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. Signed-off-by: Eli Billauer eli.billa...@gmail.com --- drivers/mmc/sdhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3125d13..80f3a91 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -226,6 +226,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, break; if (--retry == 0) break; + udelay(10); } while ((stat mask) != mask); Hmmm... Is 100ms part of the spec? I like the idea of making the timeout more time-based, but it seems to me that this changes the timeout quite significantly. If it took N ms before, it now takes N + 100 ms. I think, if we want the timeout to be ~100ms, we should use a udelay of 100 or 1000, and then reduce retry accordingly. Andy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
On 19/06/14 19:43, Andy Fleming wrote: On Thu, Jun 12, 2014 at 4:41 AM, Eli Billauereli.billa...@gmail.com wrote: The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. [ snipped patch ] Hmmm... Is 100ms part of the spec? I like the idea of making the timeout more time-based, but it seems to me that this changes the timeout quite significantly. If it took N ms before, it now takes N + 100 ms. I think, if we want the timeout to be ~100ms, we should use a udelay of 100 or 1000, and then reduce retry accordingly. Hi, As I said in the mail preceding this patch, I don't know what the timeout should be. Maybe someone with a better knowledge on MMC could come forward. Regards, Eli Andy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()
The current wait loop just reads the status 1 times, which makes the actual timeout period platform-dependent. The udelay() call within the loop makes the new timeout ~100 ms. Signed-off-by: Eli Billauer eli.billa...@gmail.com --- drivers/mmc/sdhci.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 3125d13..80f3a91 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -226,6 +226,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, break; if (--retry == 0) break; + udelay(10); } while ((stat mask) != mask); if (retry == 0) { -- 1.7.2.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot