In message 200509021814.08001.pantelis.antoniou@gmail.com you wrote:
As you well know, as part of the ongoing cleanup of the linux trees regarding PPC, it has been decreed that the preferred way to pass bootloader information shall be the flat OF tree.
Hear, hear!
The following patch (broken in two parts), implements just that.
No, it does not. It does more than this on one hand, and less than reqired on the other.
Please clean up and resubmit:
diff --git a/common/Makefile b/common/Makefile --- a/common/Makefile +++ b/common/Makefile @@ -51,7 +51,7 @@ COBJS = main.o ACEX1K.o altera.o bedbug. memsize.o miiphybb.o miiphyutil.o \ s_record.o serial.o soft_i2c.o soft_spi.o spartan2.o \ usb.o usb_kbd.o usb_storage.o \
virtex2.o xilinx.o
virtex2.o xilinx.o ft_build.o
Lists shall be kept sorted. Don't be lazy and just append at the end, please.
--- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c
...
- /*
* Linux Kernel Parameters:* r3: ptr to board info data* r4: initrd_start or 0 if no initrd* r5: initrd_end - unused if r4 is 0* r6: Start of command line string* r7: End of command line string*/- (*kernel) ((bd_t *)of_flat_tree, initrd_start, initrd_end, cmd_start, cmd_end);
Is it board info data or an OF tree? I think the comment needs to be adjusted to match the code.
diff --git a/common/ft_build.c b/common/ft_build.c new file mode 100644 --- /dev/null +++ b/common/ft_build.c
...
- /* paste the bd_t at the end of the flat tree */
- end = (char *)blob +
be32_to_cpu(((struct boot_param_header *)blob)->totalsize);- memcpy(end, bd, sizeof(*bd));
+#ifdef __powerpc__
I understand that *all* of this is PPC specific, so why do you #ifdef just this part? IMHO all of this should disappear for all non-PPC versions?
- p = ft_get_prop(blob, "/u-boot-bd_t/memstart", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/memsize", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/flashstart", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/flashsize", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/flashoffset", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/sramstart", &len);
...
- p = ft_get_prop(blob, "/u-boot-bd_t/sramsize", &len);
...
That's alot or repeated string constants, which just waste memory in the data segment. Any way for a leaner implementation?
While we are at it: did you check how much this patch adds to the memory footprint?
+#if defined(CONFIG_MPC5xxx)
- p = ft_get_prop(blob, "/u-boot-bd_t/mbar_base", &len);
- if (p != NULL)
*p = cpu_to_be32(bd->bi_mbar_base);+#endif
...
+#if defined(CONFIG_MPC5xxx)
- p = ft_get_prop(blob, "/u-boot-bd_t/ipbfreq", &len);
- if (p != NULL)
*p = cpu_to_be32(bd->bi_ipbfreq);- p = ft_get_prop(blob, "/u-boot-bd_t/pcifreq", &len);
- if (p != NULL)
*p = cpu_to_be32(bd->bi_pcifreq);+#endif
Maybe you can sort these things to minimize the groups of #ifdef's needed?
+#if defined(CONFIG_8xx)
- clock = gd->cpu_clk;
+#elif defined(CONFIG_8260)
- clock = gd->brg_clk;
+#else
- clock = 0;
+#endif
- p = ft_get_prop(blob, "/cpus/" OF_CPU "/clock-frequency", &len);
- if (p != NULL)
*p = cpu_to_be32(clock);
clock = 0 for anything but 8xx and 8260???
- p = ft_get_prop(blob, "/cpus/" OF_CPU "/timebase-frequency", &len);
- if (p != NULL)
*p = cpu_to_be32(clock / 4); /* XXX I'm just guessing */
... and probably wrong?
There is no CHANGELOG entry.
Rejected.
Content-Type: text/plain; charset="us-ascii"; name="of-uboot-stxxtc.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="of-uboot-stxxtc.patch"
There is no CHANGELOG entry.
Rejected.
Best regards,
Wolfgang Denk