Dear Graeme Russ,
Note: all comments made to part 'a' of this patch apply here, too.
In message <[EMAIL PROTECTED]> you wrote:
> Split to meet mailing list size limit
This should not be part of the git commit message, so please move
below the '---' line.
> Initial addition of eNET files - builds clean but will not run until
> additional i386 code changes are made
>
> Signed-off-by: Graeme Russ <[EMAIL PROTECTED]>
>
> --
These should be 3 (three) dashes:
---
And any comments that are not intended for the commit message must go
*below* that line..
> diff --git a/board/eNET/fpga.c b/board/eNET/fpga.c
> new file mode 100644
> index 0000000..a3e4677
> --- /dev/null
> +++ b/board/eNET/fpga.c
> @@ -0,0 +1,149 @@
> +/*
> + * (C) Copyright 2002
> + * Wolfgang Grandegger, DENX Software Engineering, [EMAIL PROTECTED]
Is this correct and complete?
> +#include "fpga.h"
> +
> +static u8 fpga_init(void);
> +static u8 fpga_write(void *buf, size_t bsize);
> +static u8 fpga_finalise(void);
> +static void fpga_close(void);
> +
> +
> +int fpga_load(int devnum, void *buf, size_t bsize )
> +{
> + u8 ret = FPGA_SUCCESS;
Do you really have to copy that code?
> diff --git a/board/eNET/hardware.h b/board/eNET/hardware.h
> new file mode 100644
> index 0000000..eab612c
> --- /dev/null
> +++ b/board/eNET/hardware.h
> @@ -0,0 +1,13 @@
> +/*
> + * hardware.h
> + *
> + * Created on: 17/09/2008
> + * Author: graeme
> + */
> +
> +#ifndef HARDWARE_H_
> +#define HARDWARE_H_
> +
> +#include "hardware_defs.h"
> +
> +#endif /* HARDWARE_H_ */
Um... A header file that just includes a single othe rheader file does
not make sense to me here.
> diff --git a/include/asm-i386/ic/sc520_defs.h
> b/include/asm-i386/ic/sc520_defs.h
> new file mode 100644
> index 0000000..c8f6311
> --- /dev/null
> +++ b/include/asm-i386/ic/sc520_defs.h
> @@ -0,0 +1,1489 @@
> +/*
> + * sc520_defs.h
> + *
> + * Created on: 17/09/2008
> + * Author: graeme
> + */
Do we really need all these 1500 lines of definitions?
Really?
...
> +/* General-Purpose CMOS RAM Mask */
> +
> +#define RTC_CMOS_REG_X 0xFF /* CMOS RAM
> Location */
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +#endif /* _ASM_IC_SC520_DEFS_H_ */
Too many empty lines - max. 2 allowed. Here, not more tna 1 should be
used.
This applies to some other files as well.
Please clean up, rebase against current code, and resubmit.
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: [EMAIL PROTECTED]
"It follows that any commander in chief who undertakes to carry out a
plan which he considers defective is at fault; he must put forth his
reasons, insist of the plan being changed, and finally tender his
resignation rather than be the instrument of his army's downfall."
- Napoleon, "Military Maxims and Thought"
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
