| Message ID | 1417705357-19389-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Bin, On 4 December 2014 at 08:02, Bin Meng <bmeng.cn@gmail.com> wrote: Can we have a short commit message about what HOB is and why you want to list it? > ============================8<============================ > => hob > HOB list address: 0x3f420000 > > No. | Address | Type | Length in Bytes > -----|----------|----------------------|----------------- > 0 | 3f420000 | Hand-off | 56 > 1 | 3f420038 | GUID Extension | 240 > 2 | 3f420128 | GUID Extension | 728 > 3 | 3f420400 | GUID Extension | 120 > 4 | 3f420478 | Resource Descriptor | 48 > 5 | 3f4204a8 | Resource Descriptor | 48 > 6 | 3f4204d8 | Resource Descriptor | 48 > 7 | 3f420508 | Resource Descriptor | 48 > 8 | 3f420538 | Resource Descriptor | 48 > 9 | 3f420568 | Resource Descriptor | 48 > 10 | 3f420598 | Memory Allocation | 48 > 11 | 3f4205c8 | Memory Allocation | 48 > 12 | 3f4205f8 | Memory Allocation | 48 > 13 | 3f420628 | GUID Extension | 4120 > 14 | 3f421640 | Memory Allocation | 48 > 15 | 3f421670 | GUID Extension | 432 > 16 | 3f421820 | Memory Allocation | 48 > 17 | 3f421850 | Memory Allocation | 48 > 18 | 3f421880 | Memory Allocation | 48 > 19 | 3f4218b0 | Memory Allocation | 48 > 20 | 3f4218e0 | Memory Allocation | 48 > 21 | 3f421910 | Memory Allocation | 48 > 22 | 3f421940 | Memory Pool | 24 > 23 | 3f421958 | Memory Allocation | 48 > 24 | 3f421988 | Memory Allocation | 48 > 25 | 3f4219b8 | Memory Allocation | 48 > 26 | 3f4219e8 | Memory Pool | 1032 > 27 | 3f421df0 | Memory Allocation | 48 > 28 | 3f421e20 | Memory Pool | 272 > ============================>8============================ > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > --- > arch/x86/lib/Makefile | 1 + > arch/x86/lib/cmd_hob.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+) > create mode 100644 arch/x86/lib/cmd_hob.c > > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index 55de788..73262d7 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -10,6 +10,7 @@ obj-y += bios_asm.o > obj-y += bios_interrupts.o > obj-$(CONFIG_CMD_BOOTM) += bootm.o > obj-y += cmd_boot.o > +obj-$(CONFIG_HAVE_FSP) += cmd_hob.o > obj-y += gcc.o > obj-y += init_helpers.o > obj-y += interrupts.o > diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c > new file mode 100644 > index 0000000..bf3bd83 > --- /dev/null > +++ b/arch/x86/lib/cmd_hob.c > @@ -0,0 +1,71 @@ > +/* > + * Copyright (C) 2014, Bin Meng <bmeng.cn@gmail.com> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <command.h> > +#include <linux/compiler.h> > +#include <asm/arch/fsp/fsp_support.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +#define HOB_TYPE_MIN 0 > +#define HOB_TYPE_MAX 11 ARRAY_SIZE(hob_type) ? > + > +static char *hob_type[] = { > + "reserved", > + "Hand-off", > + "Memory Allocation", > + "Resource Descriptor", > + "GUID Extension", > + "Firmware Volumn", > + "CPU", > + "Memory Pool", > + "reserved", > + "Firmware Volumn 2", > + "Load PEIM Unused", > + "UEFI Capsule", > +}; > + > +int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + EFI_PEI_HOB_POINTERS hob; > + UINT16 type; > + char *desc; > + int i = 0; > + > + hob.Raw = (UINT8 *)gd->arch.hob_list; > + > + printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.Raw); > + > + printf(" No. | Address | Type | Length in Bytes \n"); > + printf("-----|----------|----------------------|-----------------\n"); > + while (!END_OF_HOB_LIST(hob)) { > + printf(" %-3d | %08x |", i, (unsigned int)hob.Raw); > + type = hob.Header->HobType; > + if (type == EFI_HOB_TYPE_UNUSED) > + desc = "*Unused*"; > + else if (type == EFI_HOB_TYPE_END_OF_HOB_LIST) > + desc = "**END OF HOB**"; > + else if (type >= HOB_TYPE_MIN && type <= HOB_TYPE_MAX) > + desc = hob_type[type]; > + else > + desc = "!!!Invalid Type!!!"; > + printf(" %-20s |", desc); > + printf(" %-15d \n", hob.Header->HobLength); Can we combine those two. Also I see a space before \n - are you running patman to check patches? > + hob.Raw = GET_NEXT_HOB(hob); > + i++; > + } > + > + return 0; > +} > + > +/* -------------------------------------------------------------------- */ > + > +U_BOOT_CMD( > + hob, 1, 1, do_hob, > + "print FSP Hand-Off Block information", > + "" > +); > -- > 1.8.2.1 > Regards, Simon
Hi Simon, On Fri, Dec 5, 2014 at 7:42 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 4 December 2014 at 08:02, Bin Meng <bmeng.cn@gmail.com> wrote: > > Can we have a short commit message about what HOB is and why you want > to list it? Sure. [snip] >> --- >> arch/x86/lib/Makefile | 1 + >> arch/x86/lib/cmd_hob.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 72 insertions(+) >> create mode 100644 arch/x86/lib/cmd_hob.c >> >> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >> index 55de788..73262d7 100644 >> --- a/arch/x86/lib/Makefile >> +++ b/arch/x86/lib/Makefile >> @@ -10,6 +10,7 @@ obj-y += bios_asm.o >> obj-y += bios_interrupts.o >> obj-$(CONFIG_CMD_BOOTM) += bootm.o >> obj-y += cmd_boot.o >> +obj-$(CONFIG_HAVE_FSP) += cmd_hob.o >> obj-y += gcc.o >> obj-y += init_helpers.o >> obj-y += interrupts.o >> diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c >> new file mode 100644 >> index 0000000..bf3bd83 >> --- /dev/null >> +++ b/arch/x86/lib/cmd_hob.c >> @@ -0,0 +1,71 @@ >> +/* >> + * Copyright (C) 2014, Bin Meng <bmeng.cn@gmail.com> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <command.h> >> +#include <linux/compiler.h> >> +#include <asm/arch/fsp/fsp_support.h> >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +#define HOB_TYPE_MIN 0 >> +#define HOB_TYPE_MAX 11 > > ARRAY_SIZE(hob_type) ? Yes, will fix. >> + >> +static char *hob_type[] = { >> + "reserved", >> + "Hand-off", >> + "Memory Allocation", >> + "Resource Descriptor", >> + "GUID Extension", >> + "Firmware Volumn", >> + "CPU", >> + "Memory Pool", >> + "reserved", >> + "Firmware Volumn 2", >> + "Load PEIM Unused", >> + "UEFI Capsule", >> +}; >> + >> +int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> +{ >> + EFI_PEI_HOB_POINTERS hob; >> + UINT16 type; >> + char *desc; >> + int i = 0; >> + >> + hob.Raw = (UINT8 *)gd->arch.hob_list; >> + >> + printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.Raw); >> + >> + printf(" No. | Address | Type | Length in Bytes \n"); >> + printf("-----|----------|----------------------|-----------------\n"); >> + while (!END_OF_HOB_LIST(hob)) { >> + printf(" %-3d | %08x |", i, (unsigned int)hob.Raw); >> + type = hob.Header->HobType; >> + if (type == EFI_HOB_TYPE_UNUSED) >> + desc = "*Unused*"; >> + else if (type == EFI_HOB_TYPE_END_OF_HOB_LIST) >> + desc = "**END OF HOB**"; >> + else if (type >= HOB_TYPE_MIN && type <= HOB_TYPE_MAX) >> + desc = hob_type[type]; >> + else >> + desc = "!!!Invalid Type!!!"; >> + printf(" %-20s |", desc); >> + printf(" %-15d \n", hob.Header->HobLength); > > Can we combine those two. Also I see a space before \n - are you > running patman to check patches? Yes, I was intending to put an additional space there. Now I don't think it is really necessary. I was using checkscript.pl to check patches. I will remove those spaces in v2. [snip] Regards, Bin
Hi! > +static char *hob_type[] = { > + "reserved", > + "Hand-off", > + "Memory Allocation", > + "Resource Descriptor", > + "GUID Extension", > + "Firmware Volumn", "volume?" ? > +int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + EFI_PEI_HOB_POINTERS hob; > + UINT16 type; > + char *desc; > + int i = 0; > + > + hob.Raw = (UINT8 *)gd->arch.hob_list; > + printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.Raw); > + > + printf(" No. | Address | Type | Length in Bytes \n"); > + printf("-----|----------|----------------------|-----------------\n"); > + while (!END_OF_HOB_LIST(hob)) { > + printf(" %-3d | %08x |", i, (unsigned int)hob.Raw); > + type = hob.Header->HobType; Can we get rid of camelCase easily? > + if (type == EFI_HOB_TYPE_UNUSED) > + desc = "*Unused*"; > + else if (type == EFI_HOB_TYPE_END_OF_HOB_LIST) > + desc = "**END OF HOB**"; > + else if (type >= HOB_TYPE_MIN && type <= HOB_TYPE_MAX) > + desc = hob_type[type]; > + else > + desc = "!!!Invalid Type!!!"; I'd put there less stars and !s... > +U_BOOT_CMD( > + hob, 1, 1, do_hob, > + "print FSP Hand-Off Block information", You might what to spell out "FSP"... it is help text after all. Thanks, Pavel
Hi Pavel, On Tue, Dec 16, 2014 at 6:34 AM, Pavel Machek <pavel@denx.de> wrote: > Hi! > > >> +static char *hob_type[] = { >> + "reserved", >> + "Hand-off", >> + "Memory Allocation", >> + "Resource Descriptor", >> + "GUID Extension", >> + "Firmware Volumn", > > "volume?" ? > >> +int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> +{ >> + EFI_PEI_HOB_POINTERS hob; >> + UINT16 type; >> + char *desc; >> + int i = 0; >> + >> + hob.Raw = (UINT8 *)gd->arch.hob_list; > >> + printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.Raw); >> + >> + printf(" No. | Address | Type | Length in Bytes \n"); >> + printf("-----|----------|----------------------|-----------------\n"); >> + while (!END_OF_HOB_LIST(hob)) { >> + printf(" %-3d | %08x |", i, (unsigned int)hob.Raw); >> + type = hob.Header->HobType; > > Can we get rid of camelCase easily? > >> + if (type == EFI_HOB_TYPE_UNUSED) >> + desc = "*Unused*"; >> + else if (type == EFI_HOB_TYPE_END_OF_HOB_LIST) >> + desc = "**END OF HOB**"; >> + else if (type >= HOB_TYPE_MIN && type <= HOB_TYPE_MAX) >> + desc = hob_type[type]; >> + else >> + desc = "!!!Invalid Type!!!"; > > I'd put there less stars and !s... > >> +U_BOOT_CMD( >> + hob, 1, 1, do_hob, >> + "print FSP Hand-Off Block information", > > You might what to spell out "FSP"... it is help text after all. > All issues are fixed in the v4 patch @ http://patchwork.ozlabs.org/patch/422204/ Regards, Bin
============================8<============================ => hob HOB list address: 0x3f420000 No. | Address | Type | Length in Bytes -----|----------|----------------------|----------------- 0 | 3f420000 | Hand-off | 56 1 | 3f420038 | GUID Extension | 240 2 | 3f420128 | GUID Extension | 728 3 | 3f420400 | GUID Extension | 120 4 | 3f420478 | Resource Descriptor | 48 5 | 3f4204a8 | Resource Descriptor | 48 6 | 3f4204d8 | Resource Descriptor | 48 7 | 3f420508 | Resource Descriptor | 48 8 | 3f420538 | Resource Descriptor | 48 9 | 3f420568 | Resource Descriptor | 48 10 | 3f420598 | Memory Allocation | 48 11 | 3f4205c8 | Memory Allocation | 48 12 | 3f4205f8 | Memory Allocation | 48 13 | 3f420628 | GUID Extension | 4120 14 | 3f421640 | Memory Allocation | 48 15 | 3f421670 | GUID Extension | 432 16 | 3f421820 | Memory Allocation | 48 17 | 3f421850 | Memory Allocation | 48 18 | 3f421880 | Memory Allocation | 48 19 | 3f4218b0 | Memory Allocation | 48 20 | 3f4218e0 | Memory Allocation | 48 21 | 3f421910 | Memory Allocation | 48 22 | 3f421940 | Memory Pool | 24 23 | 3f421958 | Memory Allocation | 48 24 | 3f421988 | Memory Allocation | 48 25 | 3f4219b8 | Memory Allocation | 48 26 | 3f4219e8 | Memory Pool | 1032 27 | 3f421df0 | Memory Allocation | 48 28 | 3f421e20 | Memory Pool | 272 ============================>8============================ Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- arch/x86/lib/Makefile | 1 + arch/x86/lib/cmd_hob.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 arch/x86/lib/cmd_hob.c diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 55de788..73262d7 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -10,6 +10,7 @@ obj-y += bios_asm.o obj-y += bios_interrupts.o obj-$(CONFIG_CMD_BOOTM) += bootm.o obj-y += cmd_boot.o +obj-$(CONFIG_HAVE_FSP) += cmd_hob.o obj-y += gcc.o obj-y += init_helpers.o obj-y += interrupts.o diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c new file mode 100644 index 0000000..bf3bd83 --- /dev/null +++ b/arch/x86/lib/cmd_hob.c @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2014, Bin Meng <bmeng.cn@gmail.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <command.h> +#include <linux/compiler.h> +#include <asm/arch/fsp/fsp_support.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define HOB_TYPE_MIN 0 +#define HOB_TYPE_MAX 11 + +static char *hob_type[] = { + "reserved", + "Hand-off", + "Memory Allocation", + "Resource Descriptor", + "GUID Extension", + "Firmware Volumn", + "CPU", + "Memory Pool", + "reserved", + "Firmware Volumn 2", + "Load PEIM Unused", + "UEFI Capsule", +}; + +int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + EFI_PEI_HOB_POINTERS hob; + UINT16 type; + char *desc; + int i = 0; + + hob.Raw = (UINT8 *)gd->arch.hob_list; + + printf("HOB list address: 0x%08x\n\n", (unsigned int)hob.Raw); + + printf(" No. | Address | Type | Length in Bytes \n"); + printf("-----|----------|----------------------|-----------------\n"); + while (!END_OF_HOB_LIST(hob)) { + printf(" %-3d | %08x |", i, (unsigned int)hob.Raw); + type = hob.Header->HobType; + if (type == EFI_HOB_TYPE_UNUSED) + desc = "*Unused*"; + else if (type == EFI_HOB_TYPE_END_OF_HOB_LIST) + desc = "**END OF HOB**"; + else if (type >= HOB_TYPE_MIN && type <= HOB_TYPE_MAX) + desc = hob_type[type]; + else + desc = "!!!Invalid Type!!!"; + printf(" %-20s |", desc); + printf(" %-15d \n", hob.Header->HobLength); + hob.Raw = GET_NEXT_HOB(hob); + i++; + } + + return 0; +} + +/* -------------------------------------------------------------------- */ + +U_BOOT_CMD( + hob, 1, 1, do_hob, + "print FSP Hand-Off Block information", + "" +);