[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Dirk Behme]

On ARM (don't know for other architectures ;) ) compiling and linking 
nand_util.c results on recent git in

~/uboot/drivers/nand/nand_util.c:657: undefined reference to `__udivdi3'
drivers/nand/libnand.a(nand_util.o): In function `nand_write_opts':
~/uboot/drivers/nand/nand_util.c:481: undefined reference to `__udivdi3'
drivers/nand/libnand.a(nand_util.o): In function `nand_erase_opts':
~/uboot/drivers/nand/nand_util.c:214: undefined reference to `__udivdi3'

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%40rfo.atmel.com&forum_name=u-boot-users

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?

Best regards

Dirk

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Håvard Skinnemoen]

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%40rfo.atmel.com&forum_name=u-boot-users
>
> 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.

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?

H?vard

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Dirk Behme]

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%40rfo.atmel.com&forum_name=u-boot-users
>>
>>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


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: div64_generic_patch.txt
Url: http://lists.denx.de/pipermail/u-boot/attachments/20070731/30b15fe6/attachment.txt 

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Wolfgang Denk]

In message <46AF8CF0.7090600@googlemail.com> you wrote:
>
> Something like in attachment?

Probably not.

> +/* 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.

> 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?


Rejected.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We Americans, we're a simple people... but piss us  off,  and  we'll
bomb your cities."           - Robin Williams, _Good Morning Vietnam_

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Dirk Behme]

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
 >

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Wolfgang Denk]

In message <46B0C928.3020001@googlemail.com> you wrote:
>
> > 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 ;)

Yes, I understood thos. Just thought I might talk you into even more
improvements :-)

> >> 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? 

Right. Objects from a librariry get only pulled into the linked image
when they are referenced somewhere. Objects explicitely listed on the
command line get always included.

I failed to see that this was an object list for another library.
Sorry for the false alarm.

> Anything wrong with this?

No. I was wrong.

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

I withdraw the reject.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Sometimes a feeling is all we humans have to go on.
	-- Kirk, "A Taste of Armageddon", stardate 3193.9

[U-Boot-Users] ARM missing __udivdi3 in lib_arm or fix 64bit division in nand_util.c?

[Wolfgang Denk]

In message <46B0C928.3020001@googlemail.com> you wrote:
>
> What's your proposal then with the 64bit division issue in nand_util.c?

Please repost the patch in correct form, i. e. with Signed-off-by:
line added... the merge window is open.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Madness takes its toll. Please have exact change.