[U-Boot] [PATCH 3/4] tools: mkimage: kwbimage list command support

[Stefan Roese]

On Tuesday 21 July 2009 23:25:50 Prafulla Wadaskar wrote:
> Signed-off-by: Prafulla Wadaskar <prafulla at marvell.com>

Just some quick nitpicking comment below.

> ---
>  common/image.c   |    4 ++--
>  tools/kwbimage.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  tools/mkimage.c  |    7 ++++++-
>  3 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/common/image.c b/common/image.c
> index 845006f..e3ad3e5 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -159,7 +159,7 @@ static table_entry_t uimage_comp[] = {
>
>  uint32_t crc32 (uint32_t, const unsigned char *, uint);
>  uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint);
> -static void genimg_print_size (uint32_t size);
> +void genimg_print_size (uint32_t size);
>  #if defined(CONFIG_TIMESTAMP) || defined(CONFIG_CMD_DATE) ||
> defined(USE_HOSTCC) static void genimg_print_time (time_t timestamp);
>  #endif
> @@ -473,7 +473,7 @@ void memmove_wd (void *to, void *from, size_t len,
> ulong chunksz) }
>  #endif /* !USE_HOSTCC */
>
> -static void genimg_print_size (uint32_t size)
> +void genimg_print_size (uint32_t size)
>  {
>  #ifndef USE_HOSTCC
>  	printf ("%d Bytes = ", size);
> diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> index b1ad37a..13bd57f 100644
> --- a/tools/kwbimage.c
> +++ b/tools/kwbimage.c
> @@ -33,6 +33,8 @@
>  #include <unistd.h>
>  #include "kwbimage.h"
>
> +extern void genimg_print_size (uint32_t size);
> +
>  static struct kwb_header kwbimage_header;
>  static int datacmd_cnt = 0;
>
> @@ -262,3 +264,42 @@ void kwbimage_set_header (struct kwb_header *hdr,
> struct stat *sbuf, exthdr->checkSum = kwbimage_checksum8((void *)exthdr,
>  						sizeof(extbhr_t), 0);
>  }
> +
> +/* -l support functions */
> +int kwbimage_check_header (struct kwb_header *hdr)
> +{
> +	bhr_t *mhdr = &hdr->kwb_hdr;
> +	extbhr_t *exthdr = &hdr->kwb_exthdr;
> +	uint8_t calc_hdrcsum;
> +	uint8_t calc_exthdrcsum;
> +
> +	calc_hdrcsum = kwbimage_checksum8((void *)mhdr,
> +			sizeof(bhr_t) - sizeof(uint8_t), 0);
> +	if (calc_hdrcsum != mhdr->checkSum) {
> +		return -1; /* main header csum not matched */
> +	}

No braces needed on single-line statements.

> +	calc_exthdrcsum = kwbimage_checksum8((void *)exthdr,
> +			sizeof(extbhr_t) - sizeof(uint8_t), 0);
> +	if (calc_hdrcsum != mhdr->checkSum) {
> +		return -2; /* ext header csum not matched */

Perhaps use some defines for those errors (-1, -2 etc).

> +	}

Again, no braces here.

> +	return 0;
> +}
> +
> +void kwbimage_print_contents (struct kwb_header *hdr) {
> +	bhr_t *mhdr = &hdr->kwb_hdr;
> +	char * bootdev = "Unknown";
> +
> +	if (mhdr->blockid == IBR_HDR_SPI_ID)
> +		bootdev = "spi";
> +	if (mhdr->blockid == IBR_HDR_NAND_ID)
> +		bootdev = "nand";
> +	if (mhdr->blockid == IBR_HDR_SATA_ID)
> +		bootdev = "sata";
> +
> +	printf("Image Type:   Kirkwood Boot from %s Image\n", bootdev);
> +	printf("Data Size:    ");
> +	genimg_print_size (mhdr->blocksize - sizeof(uint32_t));
> +	printf("Load Address: %08x\n", mhdr->destaddr);
> +	printf("Entry Point:  %08x\n", mhdr->execaddr);

You are mixing coding styles here. func() vs. func () (space before "("). 
Please use only one style per file.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================