Håvard Skinnemoen wrote:

On 7/31/07, Dirk Behme dirk.behme@googlemail.com wrote:

...

In lib_arm __udivsi3 and friends are available, but __udivdi3 is missing. There is a fix by modifying nand_util.c

http://sourceforge.net/mailarchive/forum.php?thread_name=468D2650.10603%40rf...

to not do any 64bit divisions any more. Now, I wonder what is the correct fix for this? Should lib_arm provide __udivdi3 as well or should nand_util.c be fixed as in above link to avoid 64bit divisions?

I can't speak for ARM, but I believe this is a problem for most architectures. In general, I think we should seriously consider moving lib_avr32/div64.c into lib_generic and start to use it for 64-bit division instead of relying on libgcc. U-Boot NG has already done this.

Something like in attachment?

In this particular case, I think it's just ridiculously expensive to do _three_ 64-bit divisions just to implement a simple progress bar and they should all go away. It looks like Patrice's patch might give a bit weird results when crossing a 4G boundary but I don't see any easy way around it. Perhaps we should just drop the whole percentage complete thing and just show a nice N / M fraction?

Sounds good! Do you have some code for this?

Dirk

Index: uboot/include/div64.h =================================================================== --- /dev/null +++ uboot/include/div64.h @@ -0,0 +1,39 @@ +#ifndef _ASM_GENERIC_DIV64_H +#define _ASM_GENERIC_DIV64_H +/* + * Copyright (C) 2003 Bernardo Innocenti bernie@develer.com + * Based on former asm-ppc/div64.h and asm-m68knommu/div64.h + * + * The semantics of do_div() are: + * + * uint32_t do_div(uint64_t *n, uint32_t base) + * { + * uint32_t remainder = *n % base; + * *n = *n / base; + * return remainder; + * } + * + * NOTE: macro parameter n is evaluated multiple times, + * beware of side effects! + */ + +#include <linux/types.h> + +extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); + +/* The unnecessary pointer compare is there + * to check for type safety (n must be 64bit) + */ +# define do_div(n,base) ({ \ + uint32_t __base = (base); \ + uint32_t __rem; \ + (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ + if (((n) >> 32) == 0) { \ + __rem = (uint32_t)(n) % __base; \ + (n) = (uint32_t)(n) / __base; \ + } else \ + __rem = __div64_32(&(n), __base); \ + __rem; \ + }) + +#endif /* _ASM_GENERIC_DIV64_H */ Index: uboot/lib_generic/div64.c =================================================================== --- /dev/null +++ uboot/lib_generic/div64.c @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2003 Bernardo Innocenti bernie@develer.com + * + * Based on former do_div() implementation from asm-parisc/div64.h: + * Copyright (C) 1999 Hewlett-Packard Co + * Copyright (C) 1999 David Mosberger-Tang davidm@hpl.hp.com + * + * + * Generic C version of 64bit/32bit division and modulo, with + * 64bit result and 32bit remainder. + * + * The fast case for (n>>32 == 0) is handled inline by do_div(). + * + * Code generated for this function might be very inefficient + * for some CPUs. __div64_32() can be overridden by linking arch-specific + * assembly versions such as arch/ppc/lib/div64.S and arch/sh/lib/div64.S. + */ + +#include <linux/types.h> + +uint32_t __div64_32(uint64_t *n, uint32_t base) +{ + uint64_t rem = *n; + uint64_t b = base; + uint64_t res, d = 1; + uint32_t high = rem >> 32; + + /* Reduce the thing a bit first */ + res = 0; + if (high >= base) { + high /= base; + res = (uint64_t) high << 32; + rem -= (uint64_t) (high*base) << 32; + } + + while ((int64_t)b > 0 && b < rem) { + b = b+b; + d = d+d; + } + + do { + if (rem >= b) { + rem -= b; + res += d; + } + b >>= 1; + d >>= 1; + } while (d); + + *n = res; + return rem; +} Index: uboot/lib_generic/Makefile =================================================================== --- uboot.orig/lib_generic/Makefile +++ uboot/lib_generic/Makefile @@ -27,7 +27,7 @@ LIB = $(obj)libgeneric.a

COBJS = bzlib.o bzlib_crctable.o bzlib_decompress.o \ bzlib_randtable.o bzlib_huffman.o \ - crc32.o ctype.o display_options.o ldiv.o sha1.o \ + crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o \ string.o vsprintf.o zlib.o

SRCS := $(COBJS:.o=.c) Index: uboot/lib_avr32/Makefile =================================================================== --- uboot.orig/lib_avr32/Makefile +++ uboot/lib_avr32/Makefile @@ -29,7 +29,7 @@ LIB = $(obj)lib$(ARCH).a

SOBJS = memset.o

-COBJS = board.o interrupts.o avr32_linux.o div64.o +COBJS = board.o interrupts.o avr32_linux.o

SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) Index: uboot/include/asm-avr32/div64.h =================================================================== --- uboot.orig/include/asm-avr32/div64.h +++ /dev/null @@ -1,39 +0,0 @@ -#ifndef _ASM_GENERIC_DIV64_H -#define _ASM_GENERIC_DIV64_H -/* - * Copyright (C) 2003 Bernardo Innocenti bernie@develer.com - * Based on former asm-ppc/div64.h and asm-m68knommu/div64.h - * - * The semantics of do_div() are: - * - * uint32_t do_div(uint64_t *n, uint32_t base) - * { - * uint32_t remainder = *n % base; - * *n = *n / base; - * return remainder; - * } - * - * NOTE: macro parameter n is evaluated multiple times, - * beware of side effects! - */ - -#include <linux/types.h> - -extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); - -/* The unnecessary pointer compare is there - * to check for type safety (n must be 64bit) - */ -# define do_div(n,base) ({ \ - uint32_t __base = (base); \ - uint32_t __rem; \ - (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ - if (((n) >> 32) == 0) { \ - __rem = (uint32_t)(n) % __base; \ - (n) = (uint32_t)(n) / __base; \ - } else \ - __rem = __div64_32(&(n), __base); \ - __rem; \ - }) - -#endif /* _ASM_GENERIC_DIV64_H */ Index: uboot/lib_avr32/div64.c =================================================================== --- uboot.orig/lib_avr32/div64.c +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (C) 2003 Bernardo Innocenti bernie@develer.com - * - * Based on former do_div() implementation from asm-parisc/div64.h: - * Copyright (C) 1999 Hewlett-Packard Co - * Copyright (C) 1999 David Mosberger-Tang davidm@hpl.hp.com - * - * - * Generic C version of 64bit/32bit division and modulo, with - * 64bit result and 32bit remainder. - * - * The fast case for (n>>32 == 0) is handled inline by do_div(). - * - * Code generated for this function might be very inefficient - * for some CPUs. __div64_32() can be overridden by linking arch-specific - * assembly versions such as arch/ppc/lib/div64.S and arch/sh/lib/div64.S. - */ - -#include <linux/types.h> - -#include <asm/div64.h> - -uint32_t __div64_32(uint64_t *n, uint32_t base) -{ - uint64_t rem = *n; - uint64_t b = base; - uint64_t res, d = 1; - uint32_t high = rem >> 32; - - /* Reduce the thing a bit first */ - res = 0; - if (high >= base) { - high /= base; - res = (uint64_t) high << 32; - rem -= (uint64_t) (high*base) << 32; - } - - while ((int64_t)b > 0 && b < rem) { - b = b+b; - d = d+d; - } - - do { - if (rem >= b) { - rem -= b; - res += d; - } - b >>= 1; - d >>= 1; - } while (d); - - *n = res; - return rem; -} Index: uboot/drivers/nand/nand_util.c =================================================================== --- uboot.orig/drivers/nand/nand_util.c +++ uboot/drivers/nand/nand_util.c @@ -37,6 +37,7 @@ #include <command.h> #include <watchdog.h> #include <malloc.h> +#include <div64.h>

#include <nand.h> #include <jffs2/jffs2.h> @@ -208,10 +209,10 @@ int nand_erase_opts(nand_info_t *meminfo }

if (!opts->quiet) { - int percent = (int) - ((unsigned long long) + unsigned long long n =(unsigned long long) (erase.addr+meminfo->erasesize-opts->offset) - * 100 / erase_length); + * 100; + int percent = (int)do_div(n, erase_length);

/* output progress message only at whole percent * steps to reduce the number of messages printed @@ -475,10 +476,9 @@ int nand_write_opts(nand_info_t *meminfo imglen -= readlen;

if (!opts->quiet) { - int percent = (int) - ((unsigned long long) - (opts->length-imglen) * 100 - / opts->length); + unsigned long long n = (unsigned long long) + (opts->length-imglen) * 100; + int percent = (int)do_div(n, opts->length); /* output progress message only at whole percent * steps to reduce the number of messages printed * on (slow) serial consoles @@ -651,10 +651,9 @@ int nand_read_opts(nand_info_t *meminfo, }

if (!opts->quiet) { - int percent = (int) - ((unsigned long long) - (opts->length-imglen) * 100 - / opts->length); + unsigned long long n = (unsigned long long) + (opts->length-imglen) * 100; + int percent = (int)do_div(n ,opts->length); /* output progress message only at whole percent * steps to reduce the number of messages printed * on (slow) serial consoles

Wolfgang Denk wrote:

In message 46AF8CF0.7090600@googlemail.com you wrote:

...

Something like in attachment?

Probably not.

Thanks for commenting!

...

+/* The unnecessary pointer compare is there

• • to check for type safety (n must be 64bit)
• */

+# define do_div(n,base) ({ \

• uint32_t __base = (base); \
• uint32_t __rem; \
• (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
• if (((n) >> 32) == 0) { \

• __rem = (uint32_t)(n) % __base;		\
  

• (n) = (uint32_t)(n) / __base;		\
  

• } else \

• __rem = __div64_32(&(n), __base);	\
  

• __rem; \
• })

CodingStyle: Generally, inline functions are preferable to macros resembling functions.

I know that this is no excuse for bad coding style, but please note that this stuff is already part of U-Boot, see lib_avr32/div64.c and include/asm-avr32/div64.h. My patch only moves the *unmodfied* files to lib_generic for general use, as proposed by Håvard. Same as U-Boot NG did ;)

...

Index: uboot/lib_generic/Makefile

--- uboot.orig/lib_generic/Makefile +++ uboot/lib_generic/Makefile @@ -27,7 +27,7 @@ LIB = $(obj)libgeneric.a

COBJS = bzlib.o bzlib_crctable.o bzlib_decompress.o \ bzlib_randtable.o bzlib_huffman.o \

•  crc32.o ctype.o display_options.o ldiv.o sha1.o \
  

•  crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o \
  

  string.o vsprintf.o zlib.o

Why should I link this code and increase the memory footprint for all boards, when 99% of them don't need this?

Sorry if I misunderstood anything here. But with putting do_div/__div64_32 to a *library* boards use it only if they need it? Here, if they explicitly use do_div()? If they don't use do_div(), the memory footprint isn't increased because it simply isn't used?

Please find below some tests with omap5912osk_config which doesn't use do_div/__div64_32. The first numbers are with clean recent git, the second with my patch with div64 stuff moved to lib_generic. As expected, library size increases but image size stays the same. Anything wrong with this?

Rejected.

What's your proposal then with the 64bit division issue in nand_util.c?

Best regards

Dirk

==> Clean recent git:

make omap5912osk_config make ll u-boot.bin

... 100016 ... u-boot.bin

ll lib_generic/libgeneric.a

... 102740 ... lib_generic/libgeneric.a

arm-elf-ar -t lib_generic/libgeneric.a

bzlib.o bzlib_crctable.o bzlib_decompress.o bzlib_randtable.o bzlib_huffman.o crc32.o ctype.o display_options.o ldiv.o sha1.o string.o vsprintf.o zlib.o

==> Clean recent git + div64_generic_patch.txt:

make omap5912osk_config make ll u-boot.bin

... 100016 ... u-boot.bin

ll lib_generic/libgeneric.a

... 106208 ... lib_generic/libgeneric.a

arm-elf-ar -t lib_generic/libgeneric.a

bzlib.o bzlib_crctable.o bzlib_decompress.o bzlib_randtable.o bzlib_huffman.o crc32.o ctype.o display_options.o div64.o ldiv.o sha1.o string.o vsprintf.o zlib.o