This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: XSEngine initial support
- From: Andrew Lunn <andrew at lunn dot ch>
- To: Kurt Stremerch <kurt dot stremerch at exys dot be>
- Cc: ecos-patches at sources dot redhat dot com
- Date: Wed, 23 Feb 2005 22:02:10 +0100
- Subject: Re: XSEngine initial support
- References: <IBEKLCLJABNPKPBCGAHIIEOCCLAA.kurt.stremerch@exys.be>
On Wed, Feb 23, 2005 at 04:49:29PM +0100, Kurt Stremerch wrote:
> Hi,
>
> I added initial support for the Exys XSEngine (PXA255) hardware board (incl.
> Flash and LAN).
Thanks. I have some comments.
Quite a lot of the files are based on other peoples work. It would be
good if you left the authors name in either the Author(s) or
Contributers field.
The indentation looks wrong in places in the patch. Generally we use
spaces not tabs. You might want to expand the tabs to spaces which i
suspect will fix the problems.
The HAL CDL CYG_HAL_STARTUP documentation mentions ROMRAM startup, but
you don't list this as a legal_value. It would be good to fix this
discrepancy.
BOGUS.{ldi|h|mlt}? There is no need for this. The cdl engine will not allow
values other than ROM and RAM.
hal_platform_setup.h is missing the #####DESCRIPTIONBEGIN#### section
in the header.
+// #define PLATFORM_EXTRAS <cyg/hal/hal_platform_extras.h>
should be removed. I don't like dead code like this in comments.
+cdl_option CYGBLD_REDBOOT_MIN_IMAGE_SIZE {
+ user_value 0x00080000
+ inferred_value 0x40000
+};
This looks a bit strange.
Otherwise it looks good.
> PS. The copyright assignment is on its way.
Great.
Andrew