[U-Boot] [PATCH] Add a CBFS driver and commands to u-boot

[Mike Frysinger]

On Saturday 03 December 2011 06:47:01 Gabe Black wrote:
> --- /dev/null
> +++ b/common/cmd_cbfs.c
>
> +int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])

static

> +	if (argc > 2) {
> +		printf("usage: cbfsls [end of rom]>\n");
> +		return 0;
> +	}

return cmd_usage(cmdtp)

> +int do_cbfs_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char *const

static

no space before that "("

> +	if (argc < 3) {
> +		printf("usage: cbfsload <addr> <filename> [bytes]\n");
> +		return 1;
> +	}

return cmd_usage(cmdtp)

> +int do_cbfs_ls (cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])

static

no space before that "("

> +		char *typeName = NULL;

const, and don't use camelCase in var names

> +int do_cbfs_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char *const

static

no space before that "("

> +	const CbfsHeader *header = file_cbfs_get_header();

camelCase ...

> +	printf("\n");
> +	printf("CBFS version: %#x\n", header->version);
> +	printf("ROM size: %#x\n", header->romSize);
> +	printf("Boot block size: %#x\n", header->bootBlockSize);
> +	printf("CBFS size: %#x\n",
> +		header->romSize - header->bootBlockSize - header->offset);
> +	printf("Alignment: %d\n", header->align);
> +	printf("Offset: %#x\n", header->offset);
> +	printf("\n");

should be easy to merge into a single printf() call

> --- a/fs/Makefile
> +++ b/fs/Makefile
> 
>  subdirs-$(CONFIG_CMD_CRAMFS) := cramfs
> +subdirs-$(CONFIG_CMD_CBFS) += cbfs
>  subdirs-$(CONFIG_CMD_EXT2) += ext2
>  subdirs-$(CONFIG_CMD_FAT) += fat
>  subdirs-$(CONFIG_CMD_FDOS) += fdos

unrelated, but that ":=" is most likely wrong as it doesn't do what it is 
actually intended

> --- /dev/null
> +++ b/fs/cbfs/cbfs.c
>
> +const char *file_cbfs_error(void)
> +{
> +	switch (file_cbfs_result) {
> +	    case CBFS_SUCCESS:
> +		return "Success";
> +	    case CBFS_NOT_INITIALIZED:
> +		return "CBFS not initialized";
> +	    case CBFS_BAD_HEADER:
> +		return "Bad CBFS header";
> +	    case CBFS_BAD_FILE:
> +		return "Bad CBFS file";
> +	    case CBFS_FILE_NOT_FOUND:
> +		return "File not found";
> +	    default:
> +		return "Unknown";
> +	}
> +}

CbfsResult starts CBFS_SUCCESS at 0, so this should be easy to rewrite as an 
array of strings which this func simply indexes

> +typedef struct CbfsFileHeader {
> ...
> +} __attribute__((packed)) CbfsFileHeader;

__packed

> +typedef struct CbfsCacheNode {
> ...
> +} __attribute__((packed)) CbfsCacheNode;

__packed

> +static void file_cbfs_fill_cache(u8 *start, u32 size, u32 align)
> +{
> ...
> +		newNode = (CbfsCacheNode *)malloc(sizeof(CbfsCacheNode));

useless cast

> --- /dev/null
> +++ b/include/cbfs.h
>
> +typedef struct CbfsHeader {
> ...
> +} __attribute__((packed)) CbfsHeader;

__packed
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120107/cf21e940/attachment.pgp>