[U-Boot-Users] Re: The i386 patch

[Wolfgang Denk]

Dear Daniel,

in message <20021113094454.G22221 at jupiter.omicron.se> you wrote:
> 
> I realize that I threw the figure around yesterdag it was 83kb rather than 
> 38...
> Todays, cleanup version measure in at 62kb.
> 
> I hope you'll like it :)

Thanks again for the patch.

> Wolfgang:
> This patch touch common code in a few places, nothing to controversion, I 
> hope.



I tried to merge your patch with the current U-Boot code; I still get
a _LOT_ of warnings,  and  unresolved  references  from  the  strings
functions, but I think you can pick up here...



A few comments:

* Please, do not add trailing white space to existing code!

* Always make sure to run the MAKEALL  script;  this  catches  errors
  like this one:

-               void *to = (void *)ntohl(hdr->ih_load);
+               void *to = (void *)ntohl(hdr->ih_load));

* Do not use C++ style comments



Comments on specific parts of the patch:

common/cmd_bootm.c:

        x86 should _not_ need any special code here. Do you think  it
        is  possible to add support for "standard" mkimage format and
        get rid of your new #ifdef's?



	Added with minor modifications:

	@@ -145,10 +163,11 @@
			SHOW_BOOT_PROGRESS (-2);
			return 1;
		}
	+       hdr->ih_hcrc = htonl(checksum);
		SHOW_BOOT_PROGRESS (3);

	I don't understand why this could be needed; omitted.


common/cmd_ide.c:

	+#ifdef __PPC__
	 #define        ATA_CURR_BASE(dev)      (CFG_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev
	 )])
	+#endif
	...
	+#ifdef __PPC__
	 static void input_swap_data(int dev, ulong *sect_buf, int words);
	+#endif


	Are you sure this is PPC only?


ppcboot/examples/syscall.S:

	Are you sure there is only a "ret" needed?



include/asm-arm/string.h:
lib_generic/string.c:

        Why do you modify ARM and generic files? I want to keep  this
        file as close as possible in sync with standard Linux code.

	*** NOT accepted ***

        This breaks include/asm-i386/string.h - can you please try to
        fix this instead?



I've checked the current state in on the U-Boot CVS server.  The  tag
is  LABEL_2002_11_18_0115 . Can you please have a look at it and help
to clean it up?

TIA!

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Severe culture shock results when experts from another protocol suite
[...] try to read OSI documents. The term "osified" is used to  refer
to  such  documents. [...] Any relationship to the word "ossified" is
purely intentional.                                - Marshall T. Rose