[U-Boot] [PATCH] mmc: bcm2835: fix delays in bug workaround
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";
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,
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?
participants (2)
-
Albert ARIBAUD -
Stephen Warren