Dear "Eric Millbrandt",
In message A88094362DE0AE49A118AB9B4EB3612403F9E020@dekaexchange.deka.local you wrote:
Add support for the DEKA R&D galaxy5200 board. The patch is based off of the mpc5xxx branch.
Please don't. Please follow the instructions and provide patches ONLY against "master" resp "next".
This e-mail and the information, including any attachments, it contains = are intended to be a confidential communication only to the person or = entity to whom it is addressed and may contain information that is = privileged. If the reader of this message is not the intended recipient, = you are hereby notified that any dissemination, distribution or copying = of this communication is strictly prohibited. If you have received this = communication in error, please immediately notify the sender and destroy = the original message.
I see. Well, in this case we are unfortunately not able to even look at your patch.
Content-Transfer-Encoding: base64 Content-Description: V1-Add-support-for-the-galaxy5200.patch Content-Disposition: attachment; filename="V1-Add-support-for-the-galaxy5200.patch"
RnJvbSA5MDYwNWZkMWM0MzEyMjAxYjMwMWY2NjAyYTVlNjk5MTA1ZmJkNDNiIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBFcmljIE1pbGxicmFuZHQgPGVtaWxsYnJhbmR0QGRla2FyZXNl YXJjaC5jb20+CkRhdGU6IFdlZCwgMTIgQXVnIDIwMDkgMTI6NDM6NTkgLTA0MDAKU3ViamVjdDog W1BBVENIXSBBZGQgc3VwcG9ydCBmb3IgdGhlIERFS0EgUmVzZWFyY2ggYW5kIERldmVsb3BtZW50 IGdhbGF4eTUyMDAgYm9hcmQuCgpTaWduZWQtb2ZmLWJ5OiBFcmljIE1pbGxicmFuZHQgPGVtaWxs YnJhbmR0QGRla2FyZXNlYXJjaC5jb20+Ci0tLQogTUFJTlRBSU5FUlMgICAgICAgICAgICAgICAg
...
And I cannot read this either. It doesn't look like plain text to me.
Please make sure to follow the instructions in http://www.denx.de/wiki/U-Boot/Patches
--- a/Makefile +++ b/Makefile @@ -557,6 +557,16 @@ digsy_mtc_RAMBOOT_config: unconfig } @$(MKCONFIG) -a digsy_mtc ppc mpc5xxx digsy_mtc
+galaxy5200_config \ +galaxy5200_LOWBOOT_config: unconfig
- @mkdir -p $(obj)include $(obj)board/galaxy5200
- @ >$(obj)include/config.h
- @[ -z "$(findstring LOWBOOT_,$@)" ] || \
{ echo "TEXT_BASE = 0xFE000000" >$(obj)board/galaxy5200/config.tmp ; \echo "... with LOWBOOT configuration" ; \}- @$(MKCONFIG) -a galaxy5200 ppc mpc5xxx galaxy5200
Please do not add such scripting to the Makefile; do this in your board config file instead; for an example, please see http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/65499
+void init_ide_reset (void) +{
- debug ("init_ide_reset\n");
- /* Configure TIMER_5 as GPIO output for ATA reset */
- volatile struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)MPC5XXX_GPT;
Please do not put declarations in the middle of the code, only at the start of a block.
+void ide_set_reset (int idereset) +{
- debug ("ide_reset(%d)\n", idereset);
- /* Configure TIMER_5 as GPIO output for ATA reset */
- volatile struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)MPC5XXX_GPT;
Ditto.
- if (idereset) {
gpt[5].emsr = MPC5XXX_GPT_GPIO_OUT0 | MPC5XXX_GPT_TMS_GPIO;/* Make a delay. MPC5200 spec says 25 usec min */udelay(500000);
Why do you exceed the needed delay by a factor of 20,000 ?
- } else {
gpt[5].emsr = MPC5XXX_GPT_GPIO_OUT1 | MPC5XXX_GPT_TMS_GPIO;- }
Don't we need a delay here, too?
+#define CONFIG_EXTRA_ENV_SETTINGS \
- "filesize=0\0" \
- "baudrate=115200\0" \
- "stdin=serial\0" \
- "stdout=serial\0" \
- "stderr=serial\0" \
Are you sure you need these?
- "ethaddr=unconfigured\0" \
- "hostname=unconfigured\0" \
PLease delete these. It is counterproductive to put incorrect values there. Rather leave these undefined.
+#define CONFIG_BOOTCOMMAND "boot"
What are you trying to do here? "boot" means: "run CONFIG_BOOTCOMMAND", which then will "run CONFIG_BOOTCOMMAND", which then will ... ?
+#define CONFIG_SYS_FLASH_BASE 0xfe000000 +/* The flash size is autoconfigured, but cpu/mpc5xxx/cpu_init.c needs this
- variable defined */
Incorrect multiline comment style. Please fix globally.
+/*---------------------------------------------------------------------------
- Miscellaneous configurable options
+---------------------------------------------------------------------------*/ +#define CONFIG_SYS_LONGHELP /* undef to save memory */ +#define CONFIG_SYS_PROMPT "uboot> " /* Monitor Command Prompt */
+#define CONFIG_CMDLINE_EDITING 1 /* add command line history */
+#define CONFIG_SYS_CACHELINE_SIZE 32 /* For MPC5xxx CPUs */ +#if defined(CONFIG_CMD_KGDB) +#define CONFIG_SYS_CACHELINE_SHIFT 5 /* log base 2 of the above value */ +#endif
+#if defined(CONFIG_CMD_KGDB) +#define CONFIG_SYS_CBSIZE 1024 /* Console I/O Buffer Size */ +#else +#define CONFIG_SYS_CBSIZE 512 /* Console I/O Buffer Size */ +#endif +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
/* Print Buffer Size */+#define CONFIG_SYS_MAXARGS 32 /* max number of command args */ +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE /* Boot Argument Buffer Size */
+#define CONFIG_SYS_MEMTEST_START 0x00100000 /* memtest works on */ +#define CONFIG_SYS_MEMTEST_END 0x00f00000 /* 1 ... 15 MB in DRAM */
+#define CONFIG_SYS_LOAD_ADDR 0x400000 /* default load address */ +#define CONFIG_SYS_HZ 1000 /* decrementer freq: 1 ms ticks */
How about some vertical alignment of the comments?
+#define CONFIG_DISPLAY_BOARDINFO 1
+#define CONFIG_SYS_HUSH_PARSER 1 +#define CONFIG_SYS_PROMPT_HUSH_PS2 "uboot> "
I consider it a really bad idea to have PS1 and PS2 the same. Please fix.
+#ifdef CONFIG_BOOTP_MASK +#undef CONFIG_BOOTP_MASK +#endif
Don't do this. Either it's not needed, or it hushes up bugs.
There are also some minor coding style issues (trailing white space).
Please clean up and resubmit (following the rules as mentioned above).
Thanks.
Best regards,
Wolfgang Denk