Re: [U-Boot] [PATCH] mmc: sdhci: Fixed timeout for sdhci_send_command()

2014-07-04 Thread Steve Rae

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()

2014-07-01 Thread Andy Fleming
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()

2014-06-27 Thread Pantelis Antoniou
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()

2014-06-19 Thread Andy Fleming
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()

2014-06-19 Thread Eli Billauer

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()

2014-06-12 Thread Eli Billauer
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