* [U-Boot-Users] [PATCH] of dump command
2006-10-30 11:39 [U-Boot-Users] [PATCH] of dump command Jerry Van Baren
@ 2006-10-30 17:43 ` Grant Likely
2006-10-30 22:22 ` Wolfgang Denk
0 siblings, 1 reply; 4+ messages in thread
From: Grant Likely @ 2006-10-30 17:43 UTC (permalink / raw)
To: u-boot
On 10/30/06, Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com> wrote:
> commit e57c182cdbd382e07338cfb395d8f4b98772913f
> Author: Jerry Van Baren <vanbaren@cideas.com>
> Date: Sat Oct 28 12:17:52 2006 -0400
>
> Add a "of" command to dump the flattened open firmware tree.
>
> Enhanced the ft_build tree dump to allow dumping from a variable instead
> of always dumping the whole tree. Also fixed some leading zero formatting
> and printing 8 byte variables when 64 bit vsprintf is not supported.
>
> Enhance vsprintf to handle "ll" long long specifier (it already supported
> the deprecated "q" specifier).
>
> Expanded the command configuration into another long long flag word
> since the command flag is (almost) out of bits.
These should probably be split up into separate patches. One for the
vsprintf 'll' support; one for the new command and one for the
MPC8260ADS config changes.
> diff --git a/common/Makefile b/common/Makefile
> index 07ddc95..4d883f6 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -37,7 +37,7 @@ COBJS = main.o ACEX1K.o altera.o bedbug.
> cmd_i2c.o cmd_ide.o cmd_immap.o cmd_itest.o cmd_jffs2.o \
> cmd_load.o cmd_log.o \
> cmd_mem.o cmd_mii.o cmd_misc.o cmd_mmc.o \
> - cmd_nand.o cmd_net.o cmd_nvedit.o \
> + cmd_nand.o cmd_net.o cmd_nvedit.o cmd_of.o \
> cmd_pci.o cmd_pcmcia.o cmd_portio.o \
> cmd_reginfo.o cmd_reiser.o cmd_scsi.o cmd_spi.o cmd_universe.o \
> cmd_usb.o cmd_vfd.o \
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index 3091a58..58f50bc 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -950,7 +950,7 @@ #else /* CONFIG_OF_FLAT_TREE */
> }
>
> ft_setup(of_flat_tree, kbd, initrd_start, initrd_end);
> - /* ft_dump_blob(of_flat_tree); */
> + /* ft_dump_blob(of_flat_tree, "/"); */
This is dead code; just remove it; it's been replaced by a calls to
ft_dump_blob() in ft_setup().
>
> #if defined(CFG_INIT_RAM_LOCK) && !defined(CONFIG_E500)
> unlock_ram_in_cache();
> @@ -968,7 +968,7 @@ #endif
> cmd_start, cmd_end);
> else {
> ft_setup(of_flat_tree, kbd, initrd_start, initrd_end);
> - /* ft_dump_blob(of_flat_tree); */
> + /* ft_dump_blob(of_flat_tree, "/"); */
ditto
> (*kernel) ((bd_t *)of_flat_tree, (ulong)kernel, 0, 0, 0);
> }
> #endif /* CONFIG_OF_FLAT_TREE */
> diff --git a/common/cmd_of.c b/common/cmd_of.c
> new file mode 100644
> index 0000000..0f7973f
> --- /dev/null
> +++ b/common/cmd_of.c
> @@ -0,0 +1,58 @@
> +/*
> + * (C) Copyright 2006
> + * Gerald Van Baren, Custom IDEAS, vanbaren at cideas.com.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +/*
> + * Misc functions
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <ft_build.h>
> +
> +#if (CONFIG1_COMMANDS & CFG1_CMD_OF)
This is not a lot of code; and it's pretty useful stuff; maybe just
make it automatically built when CONFIG_OF_FLAT_TREE is defined?
(Rather than adding a new CONFIG_COMMANDS macro at the moment)
If a new CFG_CMD_ macro set is needed; I wouldn't do it now. There is
still 1 bit left. :) If it needs to be changed, it might make sense
to audit the full list and see if there is anything that should be
removed.
Wolfgang needs to weigh in on this one.
> +
> +int do_of (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +{
> + void *p = (void *)simple_strtoul(argv[1], NULL, 16);
> +
> + if (argc < 2) {
> + printf ("Usage:\n%s\n", cmdtp->usage);
> + return 1;
> + } else if (argc == 2) {
> + ft_dump_blob(p, "/");
> + } else {
> + ft_dump_blob(p, argv[2]);
> + }
> +
> + return 0;
> +}
> +
> +
> +U_BOOT_CMD(
> + of, 3, 0, do_of,
> + "of - Open Firmware utility commands\n",
> + "of <addr> - Dump the address as an OF tree\n"
> + "of <addr> <prop> - Dump the given property\n"
> +);
I think 'of' is a bit terse. Maybe "ofinfo", "ofdump", or "ftinfo"?
> +
> +#endif /* CFG1_CMD_OF */
> +
> diff --git a/common/ft_build.c b/common/ft_build.c
> index 980e40f..a752b5e 100644
> --- a/common/ft_build.c
> +++ b/common/ft_build.c
> @@ -233,10 +233,19 @@ static void print_data(const void *data,
> printf(" = <%04x>", be16_to_cpu(*(u16 *) data) & 0xffff);
> break;
> case 4: /* word */
> - printf(" = <%x>", be32_to_cpu(*(u32 *) data) & 0xffffffffU);
> + printf(" = <%08x>", be32_to_cpu(*(u32 *) data) & 0xffffffffU);
> break;
> case 8: /* double-word */
> - printf(" = <%qx>", be64_to_cpu(*(uint64_t *) data));
> + /*
> + * If 64 bit printing is supported, use it.
> + */
> +#ifdef CFG_64BIT_VSPRINTF
> + printf(" = <%016llx>", be64_to_cpu(*(uint64_t *) data));
> +#else
> + printf(" = <%08x", be32_to_cpu(*(u32 *) data) & 0xffffffffU);
> + data += sizeof(u32);
> + printf(" %08x>", be32_to_cpu(*(u32 *) data) & 0xffffffffU);
> +#endif
> break;
> default: /* anything else... hexdump */
> printf(" = [");
> @@ -248,7 +257,12 @@ static void print_data(const void *data,
> }
> }
>
> -void ft_dump_blob(const void *bphp)
> +/*
> + * Used by ft_dump_blob() and ft_get_prop() to build up property names
> + */
> +static char path[256], prop[256];
Is moving these out of ft_get_prop truly necessary? (I haven't dug
into the full context, but seeing this makes me scared)
> +
> +void ft_dump_blob(const void *bphp, const char *propname)
> {
> const struct boot_param_header *bph = bphp;
> const uint64_t *p_rsvmap = (const uint64_t *)
> @@ -260,7 +274,9 @@ void ft_dump_blob(const void *bphp)
> u32 tag;
> const u32 *p;
> const char *s, *t;
> + char *ss;
> int depth, sz, shift;
> + int found;
> int i;
> uint64_t addr, size;
>
> @@ -271,6 +287,8 @@ void ft_dump_blob(const void *bphp)
>
> depth = 0;
> shift = 4;
> + found = 0;
> + strcpy(path, "/");
>
> for (i = 0;; i++) {
> addr = be64_to_cpu(p_rsvmap[i * 2]);
> @@ -290,7 +308,17 @@ void ft_dump_blob(const void *bphp)
> s = (const char *)p;
> p = (u32 *) _ALIGN((unsigned long)p + strlen(s) + 1, 4);
>
> - printf("%*s%s {\n", depth * shift, "", s);
> + strcat(path, s);
> + if (found || (strcmp(path, propname) == 0)) {
> + found++;
> + if (depth == 0)
> + printf("/ {\n");
> + else
> + printf("%*s%s {\n", depth * shift, "", s);
> + }
> + /* The root path is already there as "/" */
> + if(depth != 0)
> + strcat(path, "/");
>
> depth++;
> continue;
> @@ -299,7 +327,16 @@ void ft_dump_blob(const void *bphp)
> if (tag == OF_DT_END_NODE) {
> depth--;
>
> - printf("%*s};\n", depth * shift, "");
> + path[strlen(path) - 1] = '\0';
> + ss = strrchr(path, '/');
> + if (ss != NULL)
> + ss[1] = '\0';
> +
> + if(found) {
> + printf("%*s};\n", depth * shift, "");
> + if (found-- == 0)
> + return; /* request done */
> + }
> continue;
> }
>
> @@ -317,9 +354,15 @@ void ft_dump_blob(const void *bphp)
> s = (const char *)p_strings + be32_to_cpu(*p++);
> t = (const char *)p;
> p = (const u32 *)_ALIGN((unsigned long)p + sz, 4);
> - printf("%*s%s", depth * shift, "", s);
> - print_data(t, sz);
> - printf(";\n");
> +
> + strcpy(prop, path);
> + strcat(prop, s);
> +
> + if(found || (strcmp(prop, propname) == 0)) {
> + printf("%*s%s", depth * shift, "", s);
> + print_data(t, sz);
> + printf(";\n");
> + }
> }
> }
Hmm, I'm not all that thrilled with this approach; but I understand
that some of it is driven by the current design of the ft_dump_blob
code. I'd rather have a block at the top which 'seeks' to the correct
point in the device tree and sets the depth value to zero. Then start
printing the tree until depth gets back to zero.
>
> @@ -349,7 +392,6 @@ void *ft_get_prop(void *bphp, const char
> char *s, *t;
> char *ss;
> int sz;
> - static char path[256], prop[256];
>
> path[0] = '\0';
>
> @@ -478,7 +520,7 @@ #endif
>
> #ifdef DEBUG
> printf ("recieved oftree\n");
> - ft_dump_blob(blob);
> + ft_dump_blob(blob, "/");
> #endif
>
> ft_init_cxt(&cxt, blob);
> @@ -585,7 +627,7 @@ #endif
>
> #ifdef DEBUG
> printf("final OF-tree\n");
> - ft_dump_blob(blob);
> + ft_dump_blob(blob, "/");
> #endif
> }
> #endif
> diff --git a/include/cmd_confdefs.h b/include/cmd_confdefs.h
> index cf36583..52b873d 100644
> --- a/include/cmd_confdefs.h
> +++ b/include/cmd_confdefs.h
> @@ -94,8 +94,13 @@ #define CFG_CMD_UNIVERSE 0x0800000000000
> #define CFG_CMD_EXT2 0x1000000000000000ULL /* EXT2 Support */
> #define CFG_CMD_SNTP 0x2000000000000000ULL /* SNTP support */
> #define CFG_CMD_DISPLAY 0x4000000000000000ULL /* Display support */
> +/*
> + * Start a second level of commands...
> + */
> +#define CFG1_CMD_OF 0x0000000000000001ULL /* Open Firmware dump support */
There's still one more bit left in CFG_CMD_ALL; just use it! :)
Or eliminate CFG_CMD_OF entirely.
> diff --git a/include/configs/MPC8260ADS.h b/include/configs/MPC8260ADS.h
> index 6195bca..55c6ff9 100644
> --- a/include/configs/MPC8260ADS.h
> +++ b/include/configs/MPC8260ADS.h
> @@ -237,6 +237,7 @@ #define CONFIG_COMMANDS (CFG_CMD_ALL &
> CFG_CMD_I2C | \
> CFG_CMD_PCI | \
> CFG_EXCLUDE ) )
> +#define CONFIG1_COMMANDS (CFG1_CMD_OF)
> #else
> #define CONFIG_COMMANDS (CFG_CMD_ALL & ~( \
> CMD_CFG_PCI | \
> @@ -506,4 +507,15 @@ #if CONFIG_ADSTYPE == CFG_8272ADS
> #define CONFIG_HAS_ETH1
> #endif
>
> +/* pass open firmware flat tree */
> +#define CONFIG_OF_FLAT_TREE 1
> +
> +/* maximum size of the flat tree (8K) */
> +#define OF_FLAT_TREE_MAX_SIZE 8192
> +
> +#define OF_CPU "PowerPC,MPC8272 at 0"
> +#define OF_TBCLK (CONFIG_8260_CLKIN / 4)
> +#define CONFIG_OF_HAS_BD_T 1
> +#define CONFIG_OF_HAS_UBOOT_ENV 1
> +
> #endif /* __CONFIG_H */
All of this should be in a separate patch
> diff --git a/include/ft_build.h b/include/ft_build.h
> index 89c689c..5530122 100644
> --- a/include/ft_build.h
> +++ b/include/ft_build.h
> @@ -58,7 +58,7 @@ void ft_add_rsvmap(struct ft_cxt *cxt, u
>
> void ft_setup(void *blob, bd_t * bd, ulong initrd_start, ulong initrd_end);
>
> -void ft_dump_blob(const void *bphp);
> +void ft_dump_blob(const void *bphp, const char *propname);
> void ft_merge_blob(struct ft_cxt *cxt, void *blob);
> void *ft_get_prop(void *bphp, const char *propname, int *szp);
>
> diff --git a/lib_generic/vsprintf.c b/lib_generic/vsprintf.c
> index 2740f2e..034c619 100644
> --- a/lib_generic/vsprintf.c
> +++ b/lib_generic/vsprintf.c
> @@ -256,6 +256,13 @@ #endif
> if (*fmt == 'h' || *fmt == 'l' || *fmt == 'q') {
> qualifier = *fmt;
> ++fmt;
> +#ifdef CFG_64BIT_VSPRINTF
> + /* parse ll (64 bit) and change to 'q' */
> + if (*fmt == 'l') {
> + qualifier = 'q';
> + ++fmt;
> + }
> +#endif
ditto
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195
^ permalink raw reply [flat|nested] 4+ messages in thread