[U-Boot-Users] [PATCH libfdt 3/3] More clean up of cmd_bootm.c
Kim Phillips
kim.phillips at freescale.com
Tue May 29 23:05:07 CEST 2007
On Sat, 26 May 2007 08:54:50 -0400
Jerry Van Baren <gvb.uboot at gmail.com> wrote:
> Removed redundant calls to fdt_chosen(), fdt_env(), and fdt_bd_t()
> Fixed most overlength lines. Some lines remain longer than 80 characters,
> but it isn't obvious to the author how to wrap them in a readable way.
>
> Signed-off-by: Gerald Van Baren <vanbaren at cideas.com>
> ---
> common/cmd_bootm.c | 98 ++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index 25b9d74..786ff6e 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -514,6 +514,19 @@ fixup_silent_linux ()
> }
> #endif /* CONFIG_SILENT_CONSOLE */
>
> +/*
> + * Some FDT-related defines to reduce clutter in the main code.
> + */
> +#if defined(CONFIG_OF_FLAT_TREE)
> +#define FDT_VALIDATE \
> + (*((ulong *)(of_flat_tree + sizeof(image_header_t))) != OF_DT_HEADER)
> +#endif
> +#if defined(CONFIG_OF_LIBFDT)
> +#define FDT_VALIDATE \
> + (fdt_check_header(of_flat_tree + sizeof(image_header_t)) != 0)
> +#endif
> +
I'd actually prefer to see what the code is doing, where it's doing it.
Please read linux-2.6/Documentation/CodingStyle, chapter 12, while
you're at it.
> +
> #ifdef CONFIG_PPC
> static void __attribute__((noinline))
> do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> @@ -748,11 +761,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> if(argc > 3) {
> of_flat_tree = (char *) simple_strtoul(argv[3], NULL, 16);
> hdr = (image_header_t *)of_flat_tree;
> -#if defined(CONFIG_OF_LIBFDT)
> - if (fdt_check_header(of_flat_tree) == 0) {
> -#else
> - if (*(ulong *)of_flat_tree == OF_DT_HEADER) {
> -#endif
> + if (FDT_VALIDATE) {
> #ifndef CFG_NO_FLASH
> if (addr2info((ulong)of_flat_tree) != NULL)
> of_data = (ulong)of_flat_tree;
> @@ -763,7 +772,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>
> if ((ntohl(hdr->ih_load) < ((unsigned long)hdr + ntohl(hdr->ih_size) + sizeof(hdr))) &&
> ((ntohl(hdr->ih_load) + ntohl(hdr->ih_size)) > (unsigned long)hdr)) {
> - puts ("ERROR: Load address overwrites Flat Device Tree uImage\nMust RESET board to recover\n");
> + puts ("ERROR: Load address overwrites the "
> + "Flat Device Tree uImage.\n"
> + "Must RESET the board to recover.\n");
you have overly verbose error messages. How's something simple like
"dt image overwritten - reset the board." ? Gets the point across, saves
some readability in the code.
> do_reset (cmdtp, flag, argc, argv);
> }
>
> @@ -773,7 +784,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> header.ih_hcrc = 0;
>
> if(checksum != crc32(0, (uchar *)&header, sizeof(image_header_t))) {
> - puts ("ERROR: Flat Device Tree header checksum is invalid\nMust RESET board to recover\n");
> + puts ("ERROR: The Flat Device Tree header "
> + "checksum is invalid.\n"
> + "Must RESET the board to recover.\n");
ditto.
> do_reset (cmdtp, flag, argc, argv);
> }
>
> @@ -781,25 +794,28 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> addr = (ulong)((uchar *)(hdr) + sizeof(image_header_t));
>
> if(checksum != crc32(0, (uchar *)addr, ntohl(hdr->ih_size))) {
> - puts ("ERROR: Flat Device Tree checksum is invalid\nMust RESET board to recover\n");
> + puts ("ERROR: The Flat Device Tree checksum "
> + "is invalid.\n"
> + "Must RESET the board to recover.\n");
"error: invalid FDT checksum. Reset the board" ?
> do_reset (cmdtp, flag, argc, argv);
> }
> printf("OK\n");
>
> if (ntohl(hdr->ih_type) != IH_TYPE_FLATDT) {
> - puts ("ERROR: uImage not Flat Device Tree type\nMust RESET board to recover\n");
> + puts ("ERROR: uImage is not a "
> + "Flat Device Tree type.\n"
> + "Must RESET the board to recover.\n");
ditto.
> do_reset (cmdtp, flag, argc, argv);
> }
> if (ntohl(hdr->ih_comp) != IH_COMP_NONE) {
> - puts ("ERROR: uImage is not uncompressed\nMust RESET board to recover\n");
> + puts ("ERROR: uImage is not uncompressed.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> -#if defined(CONFIG_OF_LIBFDT)
> - if (fdt_check_header(of_flat_tree + sizeof(image_header_t)) != 0) {
> -#else
> - if (*((ulong *)(of_flat_tree + sizeof(image_header_t))) != OF_DT_HEADER) {
> -#endif
> - puts ("ERROR: uImage data is not a flat device tree\nMust RESET board to recover\n");
> + if (FDT_VALIDATE) {
> + puts ("ERROR: uImage data is not "
> + "a Flat Device Tree.\n"
> + "Must RESET the board to recover.\n");
ditto.
> do_reset (cmdtp, flag, argc, argv);
> }
>
> @@ -808,10 +824,11 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> ntohl(hdr->ih_size));
> of_flat_tree = (char *)ntohl(hdr->ih_load);
> } else {
> - puts ("Did not find a flat flat device tree\nMust RESET board to recover\n");
> + puts ("Did not find a flat Flat Device Tree.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> - printf (" Booting using flat device tree at 0x%x\n",
> + printf (" Booting using the Flat Device Tree at 0x%x\n",
> of_flat_tree);
> } else if ((hdr->ih_type==IH_TYPE_MULTI) && (len_ptr[1]) && (len_ptr[2])) {
> u_long tail = ntohl(len_ptr[0]) % 4;
> @@ -835,12 +852,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> of_data += 4 - tail;
> }
>
> -#if defined(CONFIG_OF_LIBFDT)
> - if (fdt_check_header((void *)of_data) != 0) {
> -#else
> - if (((struct boot_param_header *)of_data)->magic != OF_DT_HEADER) {
> -#endif
> - puts ("ERROR: image is not a flat device tree\nMust RESET board to recover\n");
> + if (FDT_VALIDATE) {
> + puts ("ERROR: image is not a Flat Device Tree.\n"
> + "Must RESET the board to recover.\n");
ditto.
> do_reset (cmdtp, flag, argc, argv);
> }
>
> @@ -849,7 +863,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> #else
> if (((struct boot_param_header *)of_data)->totalsize != ntohl(len_ptr[2])) {
> #endif
> - puts ("ERROR: flat device tree size does not agree with image\nMust RESET board to recover\n");
> + puts ("ERROR: Flat Device Tree size does not "
> + "agree with the image size.\n"
> + "Must RESET the board to recover.\n");
ditto.
> do_reset (cmdtp, flag, argc, argv);
> }
> }
> @@ -944,25 +960,30 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> of_start, of_start + of_len - 1);
> err = fdt_open_into((void *)of_data, (void *)of_start, of_len);
> if (err != 0) {
> - printf ("libfdt: %s " __FILE__ " %d\n", fdt_strerror(err), __LINE__);
> + puts ("ERROR: Failed to move the Flat Device Tree.\n"
> + "Must RESET the board to recover.\n");
> + do_reset (cmdtp, flag, argc, argv);
> }
> /*
> * Add the chosen node if it doesn't exist, add the env and bd_t
> * if the user wants it (the logic is in the subroutines).
> */
> if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {
> - puts ("ERROR: Failed creating the /chosen node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed creating the /chosen node.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #ifdef CONFIG_OF_HAS_UBOOT_ENV
> if (fdt_env(of_flat_tree) < 0) {
> - puts ("ERROR: Failed creating the /u-boot-env node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed creating the /u-boot-env node.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #endif
> #ifdef CONFIG_OF_HAS_BD_T
> if (fdt_bd_t(of_flat_tree) < 0) {
> - puts ("ERROR: Failed creating the /bd_t node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed creating the /bd_t node.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #endif
> @@ -989,23 +1010,34 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> }
> #endif
> #if defined(CONFIG_OF_FLAT_TREE)
> + /*
> + * Create the /chosen node and modify the blob with board specific
> + * values as needed.
> + */
> ft_setup(of_flat_tree, kbd, initrd_start, initrd_end);
> /* ft_dump_blob(of_flat_tree); */
> #endif
> #if defined(CONFIG_OF_LIBFDT)
> + /*
> + * Create the /chosen node and modify the blob with board specific
> + * values as needed.
> + */
duplicate comment; why not hoist it up?
> if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {
> - puts ("ERROR: Failed to create the /chosen node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed to create the /chosen node.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #ifdef CONFIG_OF_HAS_UBOOT_ENV
> if (fdt_env(of_flat_tree) < 0) {
> - puts ("ERROR: Failed to create the /u-boot-env node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed to create the /u-boot-env node.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #endif
> #ifdef CONFIG_OF_HAS_BD_T
> if (fdt_bd_t(of_flat_tree) < 0) {
> - puts ("ERROR: Failed to create the /bd_t node, aborting.\nMust RESET board to recover\n");
> + puts ("ERROR: Failed to create the /bd_t node, aborting.\n"
> + "Must RESET the board to recover.\n");
> do_reset (cmdtp, flag, argc, argv);
> }
> #endif
> @@ -1024,7 +1056,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
> if (of_flat_tree) { /* device tree; boot new style */
> /*
> * Linux Kernel Parameters (passing device tree):
> - * r3: ptr to flattened device tree, followed by the board info data
> + * r3: pointer to the fdt, followed by the board info data
Kim
More information about the U-Boot
mailing list