Dear Stefano Babic,
In message <1328350963-30989-2-git-send-email-sba...@denx.de> you wrote:
> From: Simon Schwarz <simonschwarz...@googlemail.com>
>
> This adds a spl command to the u-boot.
>
> Related config:
> CONFIG_CMD_CPL
> activate/deactivate the command
CONFIG_CMD_CPL??? Is this a typo, and you mean CONFIG_CMD_SPL ?
> CONFIG_CMD_SPL_NAND_OFS
> Offset in NAND to use
What about other storage devices?
...
> +/* Calls bootm with the parameters given */
> +static int call_bootm(int argc, char * const argv[], char *subcommand[])
> +{
> + char *bootm_argv[5];
> + char command[] = "do_bootm";
...
> +}
Why is this needed? Why don't you run do_bootm() directly?
> +static int spl_export_fdt(int argc, char * const argv[])
> +{
...
> + /* inspect paramters and execute bootm */
> + argc--;
> + argv++;
> + if (call_bootm(argc, argv, subcommand))
> + return -1;
> +
> + printf("Argument image is now in RAM: 0x%p\n",
> + (void *)images.ft_addr);
> + return 0;
> +#else
> + printf("Das U-Boot was build without fdt support - aborting\n");
> + return -1;
> +#endif
> +}
> +
> +/* assemble the bootm patameters for atags creation */
> +static int spl_export_atags(int argc, char * const argv[])
> +{
...
> + /* inspect parameters and execute bootm */
> + argc--;
> + argv++;
> + if (call_bootm(argc, argv, subcommand))
> + return -1;
> +
> + printf("Argument image is now in RAM at: 0x%p\n",
> + (void *)gd->bd->bi_boot_params);
> + return 0;
> +#endif
> + printf("Das U-Boot was build without ATAGS support - aborting\n");
> + return -1;
> +}
This is basicly identical code, only the data structures differ.
Please use a common function, and pass (a pointer to) the data as
argument.
> + if (c) {
> + cmd = (int)c->cmd;
> + switch (cmd) {
> + case SPL_EXPORT_FDT:
> + argc--;
> + argv++;
> + return spl_export_fdt(argc, argv);
> + break;
> + case SPL_EXPORT_ATAGS:
> + argc--;
> + argv++;
> + return spl_export_atags(argc, argv);
> + break;
> + default:
This will also eliminate this switch() - you just have to set the
pointer to the respective data to pass.
> +/*
> + * Arguments:
> + * 1: subcommand
> + * 2: image_type
> + * 3: nand_offset
> + * 4: kernel_addr
> + * 5: initrd_addr
> + * 6: fdt_adr
> + */
I think it is broken to restrict this by design to NAND booting only.
This is a command that attemtps to deal with SPL booting in general,
so it should support all possible kinds of boot media - NOR, NAND,
MMC, SPI flash, USB, ...
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: w...@denx.de
That said, there may be good reasons for what you did beyond obsequi-
ous sycophantic parody. Perhaps you might be so kind as to elucidate.
-- Tom Christiansen in <5ldjbm$jtk$1...@csnews.cs.colorado.edu>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
