[U-Boot] PATCH mtd CFI flash: timeout calculation underflow if imprecise 1kHz timer: fix
From be54cb97ca26bcbbc1a908d1f2a5447b6639dc59 Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Thu, 6 Aug 2009 11:40:52 +0200 Subject: [PATCH] drivers/mtd/cfi_flash: precision and underflow problem in tout calculation
With old configuration it could happen tout=0 if CONFIG_SYS_HZ<1000 solved using an unsigned long long
Signed-off-by: Renato Andreola renato.andreola@imagos.it --- drivers/mtd/cfi_flash.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 81ac5d3..6bcd102 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -659,8 +659,10 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector, { ulong start;
-#if CONFIG_SYS_HZ != 1000 - tout *= CONFIG_SYS_HZ/1000; +#if CONFIG_SYS_HZ != 1000 + unsigned long long ull; + ull = tout*CONFIG_SYS_HZ + CONFIG_SYS_HZ/2; + tout = ull/1000; /* Compute: tout *= CONFIG_SYS_HZ/1000; */ #endif
/* Wait for command completion */
Dear Renato Andreola,
In message 4A7AA72C.8010704@imagos.it you wrote:
From be54cb97ca26bcbbc1a908d1f2a5447b6639dc59 Mon Sep 17 00:00:00 2001 From: Renato Andreola renato.andreola@imagos.it Date: Thu, 6 Aug 2009 11:40:52 +0200 Subject: [PATCH] drivers/mtd/cfi_flash: precision and underflow problem in tout calculation
With old configuration it could happen tout=0 if CONFIG_SYS_HZ<1000 solved using an unsigned long long
I don't like this implementation - using LL divisions just pulls in more dependencies on libgcc code and the like, and blows up the code size.
-#if CONFIG_SYS_HZ != 1000
- tout *= CONFIG_SYS_HZ/1000;
+#if CONFIG_SYS_HZ != 1000
- unsigned long long ull;
- ull = tout*CONFIG_SYS_HZ + CONFIG_SYS_HZ/2;
- tout = ull/1000; /* Compute: tout *= CONFIG_SYS_HZ/1000; */
I think rounding up should be sufficient here, i. e. something like:
tout *= DIV_ROUND_UP(CONFIG_SYS_HZ,1000);
?
Best regards,
Wolfgang Denk
With old configuration it could happen tout=0 if CONFIG_SYS_HZ<1000 solved using an unsigned long long
I had the same problem with an ancient version of a vendor-ported u-boot. There my CFG_HZ was 100, so the timeout was 0.
I used this, which avoids the preprocessor conditional. Since the HZ value is a compile-time constant, the compiler chooses the if or else branch and doesn't spit a run-time conditional.2
if (CFG_HZ > 100000) tout *= CFG_HZ/1000; /* for a big HZ, avoid overflow */ else tout = tout * CFG_HZ / 1000 + 1;
/alessandro
participants (3)
-
Alessandro Rubini -
Renato Andreola -
Wolfgang Denk