[U-Boot] [PATCH] mmc: bcm2835: fix delays in bug workaround

[U-Boot] [PATCH] mmc: bcm2835: fix delays in bug workaround

[Stephen Warren]

The back-to-back-writes workaround in the BCM2835 MMC driver assumed
that get_timer() returned uS. Now that it returns mS, the delay is far
too long. Use udelay() directly to avoid this. Dispense with the
"last_write" code since we now have no way of recording an absolute
time in uS. The difference between two un-averaged tests loading a
zImage is 445 mS vs the original 412 mS, so the difference doesn't
appear too relevant.

Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
 drivers/mmc/bcm2835_sdhci.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
index b0afc3c..240b5ec 100644
--- a/drivers/mmc/bcm2835_sdhci.c
+++ b/drivers/mmc/bcm2835_sdhci.c
@@ -46,7 +46,6 @@
 struct bcm2835_sdhci_host {
 	struct sdhci_host host;
 	uint twoticks_delay;
-	ulong last_write;
 };
 
 static inline struct bcm2835_sdhci_host *to_bcm(struct sdhci_host *host)
@@ -67,11 +66,9 @@ static inline void bcm2835_sdhci_raw_writel(struct sdhci_host *host, u32 val,
 	 * (Which is just as well - otherwise we'd have to nobble the DMA engine
 	 * too)
 	 */
-	while (get_timer(bcm_host->last_write) < bcm_host->twoticks_delay)
-		;
+	udelay(bcm_host->twoticks_delay);
 
 	writel(val, host->ioaddr + reg);
-	bcm_host->last_write = get_timer(0);
 }
 
 static inline u32 bcm2835_sdhci_raw_readl(struct sdhci_host *host, int reg)
@@ -172,7 +169,6 @@ int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
 	 * +1 for hack rounding.
 	 */
 	bcm_host->twoticks_delay = ((2 * 1000000) / MIN_FREQ) + 1;
-	bcm_host->last_write = 0;
 
 	host = &bcm_host->host;
 	host->name = "bcm2835_sdhci";
-- 
1.7.10.4

[U-Boot] [PATCH] mmc: bcm2835: fix delays in bug workaround

[Albert ARIBAUD]

Hi Stephen,

On Thu, 21 Mar 2013 22:02:18 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> The back-to-back-writes workaround in the BCM2835 MMC driver assumed
> that get_timer() returned uS. Now that it returns mS, the delay is far
> too long. Use udelay() directly to avoid this. Dispense with the
> "last_write" code since we now have no way of recording an absolute
> time in uS. The difference between two un-averaged tests loading a
> zImage is 445 mS vs the original 412 mS, so the difference doesn't
> appear too relevant.

I don't entirely get the 'we have no way of recording an absolute time
in us': doesn't get_timer_us() precisely provide this absolute us time
just like 'old' get_timer(base) did? IOW, could you not simply turn
every get_timer(X) into (get_timer_us()-X)?
 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  drivers/mmc/bcm2835_sdhci.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/bcm2835_sdhci.c b/drivers/mmc/bcm2835_sdhci.c
> index b0afc3c..240b5ec 100644
> --- a/drivers/mmc/bcm2835_sdhci.c
> +++ b/drivers/mmc/bcm2835_sdhci.c
> @@ -46,7 +46,6 @@
>  struct bcm2835_sdhci_host {
>  	struct sdhci_host host;
>  	uint twoticks_delay;
> -	ulong last_write;
>  };
>  
>  static inline struct bcm2835_sdhci_host *to_bcm(struct sdhci_host *host)
> @@ -67,11 +66,9 @@ static inline void bcm2835_sdhci_raw_writel(struct sdhci_host *host, u32 val,
>  	 * (Which is just as well - otherwise we'd have to nobble the DMA engine
>  	 * too)
>  	 */
> -	while (get_timer(bcm_host->last_write) < bcm_host->twoticks_delay)
> -		;
> +	udelay(bcm_host->twoticks_delay);
>  
>  	writel(val, host->ioaddr + reg);
> -	bcm_host->last_write = get_timer(0);
>  }
>  
>  static inline u32 bcm2835_sdhci_raw_readl(struct sdhci_host *host, int reg)
> @@ -172,7 +169,6 @@ int bcm2835_sdhci_init(u32 regbase, u32 emmc_freq)
>  	 * +1 for hack rounding.
>  	 */
>  	bcm_host->twoticks_delay = ((2 * 1000000) / MIN_FREQ) + 1;
> -	bcm_host->last_write = 0;
>  
>  	host = &bcm_host->host;
>  	host->name = "bcm2835_sdhci";


Amicalement,
-- 
Albert.

[U-Boot] [PATCH] mmc: bcm2835: fix delays in bug workaround

[Stephen Warren]

On 03/22/2013 12:41 AM, Albert ARIBAUD wrote:
> Hi Stephen,
> 
> On Thu, 21 Mar 2013 22:02:18 -0600, Stephen Warren
> <swarren@wwwdotorg.org> wrote:
> 
>> The back-to-back-writes workaround in the BCM2835 MMC driver assumed
>> that get_timer() returned uS. Now that it returns mS, the delay is far
>> too long. Use udelay() directly to avoid this. Dispense with the
>> "last_write" code since we now have no way of recording an absolute
>> time in uS. The difference between two un-averaged tests loading a
>> zImage is 445 mS vs the original 412 mS, so the difference doesn't
>> appear too relevant.
> 
> I don't entirely get the 'we have no way of recording an absolute time
> in us': doesn't get_timer_us() precisely provide this absolute us time
> just like 'old' get_timer(base) did? IOW, could you not simply turn
> every get_timer(X) into (get_timer_us()-X)?

Well, I could do that, but that's an internal private API inside
timer.c, not something exported. Should I explicitly export it and allow
the driver to use that?