Re: [U-Boot] [PATCH 15/25] x86: Add a simple command to show FSP HOB information

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