From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pavel Machek <pavel@denx.de>
Date: Mon, 15 Dec 2014 23:34:48 +0100
Subject: [U-Boot] [PATCH 15/25] x86: Add a simple command to show FSP
 HOB information
In-Reply-To: <1417705357-19389-1-git-send-email-bmeng.cn@gmail.com>
References: <1417705357-19389-1-git-send-email-bmeng.cn@gmail.com>
Message-ID: <20141215223447.GD27866@amd>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html