This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: XSEngine initial support


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]