All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot
@ 2011-12-03 11:47 Gabe Black
  2011-12-05 22:25 ` Wolfgang Denk
  2012-01-08  4:52 ` [U-Boot] [PATCH] " Mike Frysinger
  0 siblings, 2 replies; 13+ messages in thread
From: Gabe Black @ 2011-12-03 11:47 UTC (permalink / raw)
  To: u-boot

Coreboot uses a very simple "file system" called CBFS to keep track of and
allow access to multiple "files" in a ROM image. This change adds CBFS
support and some commands to use it to u-boot. These commands are:

cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end of
the ROM is an optional parameter which defaults to the standard 0xffffffff and
can be used to support multiple CBFSes in a system. The last one set up with
cbfsinit is the one that will be used.

cbfsinfo - Print information from the CBFS header.

cbfsls - Print out the size, type, and name of all the files in the current
CBFS. Recognized types are translated into symbolic names.

cbfsload - Load a file from CBFS into memory. Like the similar command for fat
filesystems, you can optionally provide a maximum size.

Also, if u-boot needs something out of CBFS very early before the heap is
configured, it won't be able to use the normal CBFS support which caches some
information in memory it allocates from the heap. This change adds a new
cbfs_file_find_uncached function which searchs a CBFS instance without touching
the heap.

Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is specified.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
 Makefile          |    6 +-
 README            |    1 +
 common/Makefile   |    1 +
 common/cmd_cbfs.c |  212 ++++++++++++++++++++++++++++++++
 fs/Makefile       |    1 +
 fs/cbfs/Makefile  |   44 +++++++
 fs/cbfs/cbfs.c    |  351 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/cbfs.h    |  163 +++++++++++++++++++++++++
 8 files changed, 776 insertions(+), 3 deletions(-)
 create mode 100644 common/cmd_cbfs.c
 create mode 100644 fs/cbfs/Makefile
 create mode 100644 fs/cbfs/cbfs.c
 create mode 100644 include/cbfs.h

diff --git a/Makefile b/Makefile
index d84b350..e1e2ff1 100644
--- a/Makefile
+++ b/Makefile
@@ -234,9 +234,9 @@ ifeq ($(CONFIG_OF_EMBED),y)
 LIBS += dts/libdts.o
 endif
 LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
-LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o fs/jffs2/libjffs2.o \
-	fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
-	fs/ubifs/libubifs.o
+LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o \
+	fs/jffs2/libjffs2.o fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o \
+	fs/yaffs2/libyaffs2.o fs/ubifs/libubifs.o fs/cbfs/libcbfs.o
 LIBS += net/libnet.o
 LIBS += disk/libdisk.o
 LIBS += drivers/bios_emulator/libatibiosemu.o
diff --git a/README b/README
index fda0190..b878c4b 100644
--- a/README
+++ b/README
@@ -721,6 +721,7 @@ The following options need to be configured:
 		CONFIG_CMD_BSP		* Board specific commands
 		CONFIG_CMD_BOOTD	  bootd
 		CONFIG_CMD_CACHE	* icache, dcache
+		CONFIG_CMD_CBFS		* Support for coreboot's CBFS
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...
diff --git a/common/Makefile b/common/Makefile
index 1b672ad..6cfba67 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -89,6 +89,7 @@ COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += cmd_eeprom.o
 COBJS-$(CONFIG_CMD_EEPROM) += cmd_eeprom.o
 COBJS-$(CONFIG_CMD_ELF) += cmd_elf.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_exit.o
+COBJS-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
 COBJS-$(CONFIG_CMD_EXT2) += cmd_ext2.o
 COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
 COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
diff --git a/common/cmd_cbfs.c b/common/cmd_cbfs.c
new file mode 100644
index 0000000..a357831
--- /dev/null
+++ b/common/cmd_cbfs.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * CBFS commands
+ */
+#include <common.h>
+#include <command.h>
+#include <cbfs.h>
+
+int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	uintptr_t end_of_rom = 0xffffffff;
+	char *ep;
+
+	if (argc > 2) {
+		printf("usage: cbfsls [end of rom]>\n");
+		return 0;
+	}
+	if (argc == 2) {
+		end_of_rom = (int)simple_strtoul(argv[1], &ep, 16);
+		if (*ep) {
+			puts("\n** Invalid end of ROM **\n");
+			return 1;
+		}
+	}
+	file_cbfs_init(end_of_rom);
+	if (file_cbfs_result != CBFS_SUCCESS) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinit,	2,	0,	do_cbfs_init,
+	"initialize the cbfs driver",
+	"[end of rom]\n"
+	"    - Initialize the cbfs driver. The optional 'end of rom'\n"
+	"      parameter specifies where the end of the ROM is that the\n"
+	"      CBFS is in. It defaults to 0xFFFFFFFF\n"
+);
+
+int do_cbfs_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	long size;
+	unsigned long offset;
+	unsigned long count;
+	char buf[12];
+	CbfsFile file;
+
+	if (argc < 3) {
+		printf("usage: cbfsload <addr> <filename> [bytes]\n");
+		return 1;
+	}
+
+	/* parse offset and count */
+	offset = simple_strtoul(argv[1], NULL, 16);
+	if (argc == 4)
+		count = simple_strtoul(argv[3], NULL, 16);
+	else
+		count = 0;
+
+	file = file_cbfs_find(argv[2]);
+	if (!file) {
+		if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
+			printf("%s: %s\n", file_cbfs_error(), argv[2]);
+		else
+			printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	printf("reading %s\n", file_cbfs_name(file));
+
+	size = file_cbfs_read(file, (void *)offset, count);
+
+	printf("\n%ld bytes read\n", size);
+
+	sprintf(buf, "%lX", size);
+	setenv("filesize", buf);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsload,	4,	0,	do_cbfs_fsload,
+	"load binary file from a cbfs filesystem",
+	"<addr> <filename> [bytes]\n"
+	"    - load binary file 'filename' from the cbfs to address 'addr'\n"
+);
+
+int do_cbfs_ls (cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	CbfsFile file = file_cbfs_get_first();
+	int files = 0;
+
+	if (!file) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	printf("     size              type  name\n");
+	printf("------------------------------------------\n");
+	while (file) {
+		u32 type = file_cbfs_type(file);
+		char *typeName = NULL;
+		const char *filename = file_cbfs_name(file);
+		printf(" %8d", file_cbfs_size(file));
+
+		switch (type) {
+		    case CBFS_TYPE_STAGE:
+			typeName = "stage";
+			break;
+		    case CBFS_TYPE_PAYLOAD:
+			typeName = "payload";
+			break;
+		    case CBFS_TYPE_OPTIONROM:
+			typeName = "option rom";
+			break;
+		    case CBFS_TYPE_BOOTSPLASH:
+			typeName = "boot splash";
+			break;
+		    case CBFS_TYPE_RAW:
+			typeName = "raw";
+			break;
+		    case CBFS_TYPE_VSA:
+			typeName = "vsa";
+			break;
+		    case CBFS_TYPE_MBI:
+			typeName = "mbi";
+			break;
+		    case CBFS_TYPE_MICROCODE:
+			typeName = "microcode";
+			break;
+		    case CBFS_COMPONENT_CMOS_DEFAULT:
+			typeName = "cmos default";
+			break;
+		    case CBFS_COMPONENT_CMOS_LAYOUT:
+			typeName = "cmos layout";
+			break;
+		    case -1UL:
+			typeName = "null";
+			break;
+		}
+		if (typeName)
+			printf("  %16s", typeName);
+		else
+			printf("  %16d", type);
+
+		if (filename[0])
+			printf("  %s\n", filename);
+		else
+			printf("  %s\n", "(empty)");
+		file_cbfs_get_next(&file);
+		files++;
+	}
+
+	printf("\n%d file(s)\n\n", files);
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsls,	1,	1,	do_cbfs_ls,
+	"list files",
+	"    - list the files in the cbfs\n"
+);
+
+int do_cbfs_fsinfo (cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	const CbfsHeader *header = file_cbfs_get_header();
+	if (!header) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	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");
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinfo,	1,	1,	do_cbfs_fsinfo,
+	"print information about filesystem",
+	"    - print information about the cbfs filesystem\n"
+);
diff --git a/fs/Makefile b/fs/Makefile
index 22aad12..ca21464 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -23,6 +23,7 @@
 #
 
 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
diff --git a/fs/cbfs/Makefile b/fs/cbfs/Makefile
new file mode 100644
index 0000000..864fdf9
--- /dev/null
+++ b/fs/cbfs/Makefile
@@ -0,0 +1,44 @@
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libcbfs.o
+
+AOBJS	=
+COBJS-$(CONFIG_CMD_CBFS)	:= cbfs.o
+
+SRCS	:= $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(AOBJS) $(COBJS-y))
+
+all:	$(LIB) $(AOBJS)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
new file mode 100644
index 0000000..1a6ec2c
--- /dev/null
+++ b/fs/cbfs/cbfs.c
@@ -0,0 +1,351 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <cbfs.h>
+#include <malloc.h>
+#include <asm/byteorder.h>
+
+CbfsResult file_cbfs_result;
+
+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";
+	}
+}
+
+typedef struct CbfsFileHeader {
+	u8 magic[8];
+	u32 len;
+	u32 type;
+	u32 checksum;
+	u32 offset;
+} __attribute__((packed)) CbfsFileHeader;
+
+typedef struct CbfsCacheNode {
+	struct CbfsCacheNode *next;
+	u32 type;
+	void *data;
+	u32 dataLength;
+	char *name;
+	u32 nameLength;
+	u32 checksum;
+} __attribute__((packed)) CbfsCacheNode;
+
+
+static const u32 goodMagic = 0x4f524243;
+static const u8 goodFileMagic[] = "LARCHIVE";
+
+
+static int initialized;
+static struct CbfsHeader cbfsHeader;
+static CbfsCacheNode *fileCache;
+
+/* Do endian conversion on the CBFS header structure. */
+static void swap_header(CbfsHeader *dest, CbfsHeader *src)
+{
+	dest->magic = be32_to_cpu(src->magic);
+	dest->version = be32_to_cpu(src->version);
+	dest->romSize = be32_to_cpu(src->romSize);
+	dest->bootBlockSize = be32_to_cpu(src->bootBlockSize);
+	dest->align = be32_to_cpu(src->align);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/* Do endian conversion on a CBFS file header. */
+static void swap_file_header(CbfsFileHeader *dest, CbfsFileHeader *src)
+{
+	memcpy(&dest->magic, &src->magic, sizeof(dest->magic));
+	dest->len = be32_to_cpu(src->len);
+	dest->type = be32_to_cpu(src->type);
+	dest->checksum = be32_to_cpu(src->checksum);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/*
+ * Given a starting position in memory, scan forward, bounded by a size, and
+ * find the next valid CBFS file. No memory is allocated by this function. The
+ * caller is responsible for allocating space for the new file structure.
+ *
+ * @param start		The location in memory to start from.
+ * @param size		The size of the memory region to search.
+ * @param align		The alignment boundaries to check on.
+ * @param newNode	A pointer to the file structure to load.
+ * @param used		A pointer to the count of of bytes scanned through,
+ *			including the file if one is found.
+ *
+ * @return 1 if a file is found, 0 if one isn't.
+ */
+static int file_cbfs_next_file(u8 *start, u32 size, u32 align,
+	CbfsCacheNode *newNode, u32 *used)
+{
+	CbfsFileHeader header;
+
+	*used = 0;
+
+	while (size >= align) {
+		CbfsFileHeader *fileHeader = (CbfsFileHeader *)start;
+		u32 nameLen;
+		u32 step;
+
+		/* Check if there's a file here. */
+		if (memcmp(goodFileMagic, &(fileHeader->magic),
+				sizeof(fileHeader->magic))) {
+			*used += align;
+			size -= align;
+			start += align;
+			continue;
+		}
+
+		swap_file_header(&header, fileHeader);
+		if (header.offset < sizeof(CbfsFileHeader) ||
+				header.offset > header.len) {
+			file_cbfs_result = CBFS_BAD_FILE;
+			return -1;
+		}
+		newNode->next = NULL;
+		newNode->type = header.type;
+		newNode->data = start + header.offset;
+		newNode->dataLength = header.len;
+		nameLen = header.offset - sizeof(CbfsFileHeader);
+		newNode->name = (char *)fileHeader + sizeof(CbfsFileHeader);
+		newNode->nameLength = nameLen;
+		newNode->checksum = header.checksum;
+
+		step = header.len;
+		if (step % align)
+			step = step + align - step % align;
+
+		*used += step;
+		return 1;
+	}
+	return 0;
+}
+
+/* Look through a CBFS instance and copy file metadata into regular memory. */
+static void file_cbfs_fill_cache(u8 *start, u32 size, u32 align)
+{
+	CbfsCacheNode *cacheNode;
+	CbfsCacheNode *newNode;
+	CbfsCacheNode **cacheTail = &fileCache;
+
+	/* Clear out old information. */
+	cacheNode = fileCache;
+	while (cacheNode) {
+		CbfsCacheNode *oldNode = cacheNode;
+		cacheNode = cacheNode->next;
+		free(oldNode);
+	}
+	fileCache = NULL;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		newNode = (CbfsCacheNode *)malloc(sizeof(CbfsCacheNode));
+		result = file_cbfs_next_file(start, size, align,
+			newNode, &used);
+
+		if (result < 0) {
+			free(newNode);
+			return;
+		} else if (result == 0) {
+			free(newNode);
+			break;
+		}
+		*cacheTail = newNode;
+		cacheTail = &newNode->next;
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+/* Get the CBFS header out of the ROM and do endian conversion. */
+static int file_cbfs_load_header(uintptr_t endOfRom, CbfsHeader *header)
+{
+	CbfsHeader *headerInRom;
+
+	headerInRom = (CbfsHeader *)(uintptr_t)*(u32 *)(endOfRom - 3);
+	swap_header(header, headerInRom);
+
+	if (header->magic != goodMagic || header->offset >
+			header->romSize - header->bootBlockSize) {
+		file_cbfs_result = CBFS_BAD_HEADER;
+		return 1;
+	}
+	return 0;
+}
+
+void file_cbfs_init(uintptr_t endOfRom)
+{
+	u8 *startOfRom;
+	initialized = 0;
+
+	if (file_cbfs_load_header(endOfRom, &cbfsHeader))
+		return;
+
+	startOfRom = (u8 *)(endOfRom + 1 - cbfsHeader.romSize);
+
+	file_cbfs_fill_cache(startOfRom + cbfsHeader.offset,
+			     cbfsHeader.romSize, cbfsHeader.align);
+	if (file_cbfs_result == CBFS_SUCCESS)
+		initialized = 1;
+}
+
+const CbfsHeader *file_cbfs_get_header(void)
+{
+	if (initialized) {
+		file_cbfs_result = CBFS_SUCCESS;
+		return &cbfsHeader;
+	} else {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+}
+
+CbfsFile file_cbfs_get_first(void)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	} else {
+		file_cbfs_result = CBFS_SUCCESS;
+		return fileCache;
+	}
+}
+
+void file_cbfs_get_next(CbfsFile *file)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		file = NULL;
+		return;
+	}
+
+	if (*file)
+		*file = (*file)->next;
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+CbfsFile file_cbfs_find(const char *name)
+{
+	struct CbfsCacheNode *cacheNode = fileCache;
+
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+
+	while (cacheNode) {
+		if (!strcmp(name, cacheNode->name))
+			break;
+		cacheNode = cacheNode->next;
+	}
+	if (!cacheNode) {
+		file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	} else {
+		file_cbfs_result = CBFS_SUCCESS;
+	}
+	return cacheNode;
+}
+
+CbfsFile file_cbfs_find_uncached(uintptr_t endOfRom, const char *name)
+{
+	u8 *start;
+	u32 size;
+	u32 align;
+	static CbfsCacheNode node;
+
+	if (file_cbfs_load_header(endOfRom, &cbfsHeader))
+		return NULL;
+
+	start = (u8 *)(endOfRom + 1 - cbfsHeader.romSize);
+	size = cbfsHeader.romSize;
+	align = cbfsHeader.align;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		result = file_cbfs_next_file(start, size, align, &node, &used);
+
+		if (result < 0) {
+			return NULL;
+		} else if (result == 0) {
+			break;
+		}
+
+		if (!strcmp(name, node.name)) {
+			return &node;
+		}
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	return NULL;
+}
+
+const char *file_cbfs_name(CbfsFile file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->name;
+}
+
+u32 file_cbfs_size(CbfsFile file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->dataLength;
+}
+
+u32 file_cbfs_type(CbfsFile file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->type;
+}
+
+long file_cbfs_read(CbfsFile file, void *buffer, unsigned long maxsize)
+{
+	u32 size;
+
+	size = file->dataLength;
+	if (maxsize && size > maxsize)
+		size = maxsize;
+
+	memcpy(buffer, file->data, size);
+
+	file_cbfs_result = CBFS_SUCCESS;
+	return size;
+}
diff --git a/include/cbfs.h b/include/cbfs.h
new file mode 100644
index 0000000..eaab6b6
--- /dev/null
+++ b/include/cbfs.h
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CBFS_H
+#define __CBFS_H
+
+#include <compiler.h>
+
+typedef enum CbfsResult {
+	CBFS_SUCCESS = 0,
+	CBFS_NOT_INITIALIZED,
+	CBFS_BAD_HEADER,
+	CBFS_BAD_FILE,
+	CBFS_FILE_NOT_FOUND
+} CbfsResult;
+
+typedef enum CbfsFileType {
+	CBFS_TYPE_STAGE = 0x10,
+	CBFS_TYPE_PAYLOAD = 0x20,
+	CBFS_TYPE_OPTIONROM = 0x30,
+	CBFS_TYPE_BOOTSPLASH = 0x40,
+	CBFS_TYPE_RAW = 0x50,
+	CBFS_TYPE_VSA = 0x51,
+	CBFS_TYPE_MBI = 0x52,
+	CBFS_TYPE_MICROCODE = 0x53,
+	CBFS_COMPONENT_CMOS_DEFAULT = 0xaa,
+	CBFS_COMPONENT_CMOS_LAYOUT = 0x01aa
+} CbfsFileType;
+
+typedef struct CbfsHeader {
+	u32 magic;
+	u32 version;
+	u32 romSize;
+	u32 bootBlockSize;
+	u32 align;
+	u32 offset;
+	u32 pad[2];
+} __attribute__((packed)) CbfsHeader;
+
+struct CbfsCacheNode;
+typedef const struct CbfsCacheNode *CbfsFile;
+
+extern CbfsResult file_cbfs_result;
+
+/*
+ * Return a string describing the most recent error condition.
+ *
+ * @return A pointer to the constant string.
+ */
+const char *file_cbfs_error(void);
+
+/*
+ * Initialize the CBFS driver and load metadata into RAM.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ */
+void file_cbfs_init(uintptr_t end_of_rom);
+
+/*
+ * Get the header structure for the current CBFS.
+ *
+ * @return A pointer to the constant structure, or NULL if there is none.
+ */
+const CbfsHeader *file_cbfs_get_header(void);
+
+/*
+ * Get a handle for the first file in CBFS.
+ *
+ * @return A handle for the first file in CBFS, NULL on error.
+ */
+CbfsFile file_cbfs_get_first(void);
+
+/*
+ * Get a handle to the file after this one in CBFS.
+ *
+ * @param file		A pointer to the handle to advance.
+ */
+void file_cbfs_get_next(CbfsFile *file);
+
+/*
+ * Find a file with a particular name in CBFS.
+ *
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+CbfsFile file_cbfs_find(const char *name);
+
+
+/***************************************************************************/
+/* All of the functions below can be used without first initializing CBFS. */
+/***************************************************************************/
+
+/*
+ * Find a file with a particular name in CBFS without using the heap.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+CbfsFile file_cbfs_find_uncached(uintptr_t end_of_rom, const char *name);
+
+/*
+ * Get the name of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The name of the file, NULL on error.
+ */
+const char *file_cbfs_name(CbfsFile file);
+
+/*
+ * Get the size of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The size of the file, zero on error.
+ */
+u32 file_cbfs_size(CbfsFile file);
+
+/*
+ * Get the type of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The type of the file, zero on error.
+ */
+u32 file_cbfs_type(CbfsFile file);
+
+/*
+ * Read a file from CBFS into RAM
+ *
+ * @param file		A handle to the file to read.
+ * @param buffer	Where to read it into memory.
+ *
+ * @return If positive or zero, the number of characters read. If negative, an
+ *         error occurred.
+ */
+long file_cbfs_read(CbfsFile file, void *buffer, unsigned long maxsize);
+
+#endif /* __CBFS_H */
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot
  2011-12-03 11:47 [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot Gabe Black
@ 2011-12-05 22:25 ` Wolfgang Denk
  2011-12-05 22:35   ` Gabe Black
  2011-12-06  1:25   ` [U-Boot] [PATCH v2] " Gabe Black
  2012-01-08  4:52 ` [U-Boot] [PATCH] " Mike Frysinger
  1 sibling, 2 replies; 13+ messages in thread
From: Wolfgang Denk @ 2011-12-05 22:25 UTC (permalink / raw)
  To: u-boot

Dear Gabe Black,

In message <1322912821-32677-1-git-send-email-gabeblack@chromium.org> you wrote:
> Coreboot uses a very simple "file system" called CBFS to keep track of and
> allow access to multiple "files" in a ROM image. This change adds CBFS
> support and some commands to use it to u-boot. These commands are:
> 
> cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end of
> the ROM is an optional parameter which defaults to the standard 0xffffffff and
> can be used to support multiple CBFSes in a system. The last one set up with
> cbfsinit is the one that will be used.
> 
> cbfsinfo - Print information from the CBFS header.
> 
> cbfsls - Print out the size, type, and name of all the files in the current
> CBFS. Recognized types are translated into symbolic names.
> 
> cbfsload - Load a file from CBFS into memory. Like the similar command for fat
> filesystems, you can optionally provide a maximum size.
> 
> Also, if u-boot needs something out of CBFS very early before the heap is
> configured, it won't be able to use the normal CBFS support which caches some
> information in memory it allocates from the heap. This change adds a new
> cbfs_file_find_uncached function which searchs a CBFS instance without touching
> the heap.
> 
> Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is specified.
> 
> Signed-off-by: Gabe Black <gabeblack@chromium.org>

Checkpatch reports 2 errors, 15 warnings

Please clean up and resubmit.

Also, please use puts() instead of printf() when you have constant
strings without formatting.

And fix your identifiers: CamelCaps identifiers are not allowed in
U-Boot.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The human race is faced with a cruel choice: work  or  daytime  tele-
vision.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot
  2011-12-05 22:25 ` Wolfgang Denk
@ 2011-12-05 22:35   ` Gabe Black
  2011-12-05 23:46     ` Gabe Black
  2011-12-06  1:25   ` [U-Boot] [PATCH v2] " Gabe Black
  1 sibling, 1 reply; 13+ messages in thread
From: Gabe Black @ 2011-12-05 22:35 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 5, 2011 at 2:25 PM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Gabe Black,
>
> In message <1322912821-32677-1-git-send-email-gabeblack@chromium.org> you
> wrote:
> > Coreboot uses a very simple "file system" called CBFS to keep track of
> and
> > allow access to multiple "files" in a ROM image. This change adds CBFS
> > support and some commands to use it to u-boot. These commands are:
> >
> > cbfsinit - Initialize CBFS support and pull all metadata into RAM. The
> end of
> > the ROM is an optional parameter which defaults to the standard
> 0xffffffff and
> > can be used to support multiple CBFSes in a system. The last one set up
> with
> > cbfsinit is the one that will be used.
> >
> > cbfsinfo - Print information from the CBFS header.
> >
> > cbfsls - Print out the size, type, and name of all the files in the
> current
> > CBFS. Recognized types are translated into symbolic names.
> >
> > cbfsload - Load a file from CBFS into memory. Like the similar command
> for fat
> > filesystems, you can optionally provide a maximum size.
> >
> > Also, if u-boot needs something out of CBFS very early before the heap is
> > configured, it won't be able to use the normal CBFS support which caches
> some
> > information in memory it allocates from the heap. This change adds a new
> > cbfs_file_find_uncached function which searchs a CBFS instance without
> touching
> > the heap.
> >
> > Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
> specified.
> >
> > Signed-off-by: Gabe Black <gabeblack@chromium.org>
>
> Checkpatch reports 2 errors, 15 warnings
>


A few of these are checkpatch getting confused by inline assembly, but I'll
fix up the rest and the things below.



>
> Please clean up and resubmit.
>
> Also, please use puts() instead of printf() when you have constant
> strings without formatting.
>
> And fix your identifiers: CamelCaps identifiers are not allowed in
> U-Boot.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> The human race is faced with a cruel choice: work  or  daytime  tele-
> vision.
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot
  2011-12-05 22:35   ` Gabe Black
@ 2011-12-05 23:46     ` Gabe Black
  0 siblings, 0 replies; 13+ messages in thread
From: Gabe Black @ 2011-12-05 23:46 UTC (permalink / raw)
  To: u-boot

On Mon, Dec 5, 2011 at 2:35 PM, Gabe Black <gabeblack@google.com> wrote:

>
>
> On Mon, Dec 5, 2011 at 2:25 PM, Wolfgang Denk <wd@denx.de> wrote:
>
>> Dear Gabe Black,
>>
>> In message <1322912821-32677-1-git-send-email-gabeblack@chromium.org>
>> you wrote:
>> > Coreboot uses a very simple "file system" called CBFS to keep track of
>> and
>> > allow access to multiple "files" in a ROM image. This change adds CBFS
>> > support and some commands to use it to u-boot. These commands are:
>> >
>> > cbfsinit - Initialize CBFS support and pull all metadata into RAM. The
>> end of
>> > the ROM is an optional parameter which defaults to the standard
>> 0xffffffff and
>> > can be used to support multiple CBFSes in a system. The last one set up
>> with
>> > cbfsinit is the one that will be used.
>> >
>> > cbfsinfo - Print information from the CBFS header.
>> >
>> > cbfsls - Print out the size, type, and name of all the files in the
>> current
>> > CBFS. Recognized types are translated into symbolic names.
>> >
>> > cbfsload - Load a file from CBFS into memory. Like the similar command
>> for fat
>> > filesystems, you can optionally provide a maximum size.
>> >
>> > Also, if u-boot needs something out of CBFS very early before the heap
>> is
>> > configured, it won't be able to use the normal CBFS support which
>> caches some
>> > information in memory it allocates from the heap. This change adds a new
>> > cbfs_file_find_uncached function which searchs a CBFS instance without
>> touching
>> > the heap.
>> >
>> > Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
>> specified.
>> >
>> > Signed-off-by: Gabe Black <gabeblack@chromium.org>
>>
>> Checkpatch reports 2 errors, 15 warnings
>>
>
>
> A few of these are checkpatch getting confused by inline assembly, but
> I'll fix up the rest and the things below.
>
>
Oops, wrong patch series. There's no inline assembly in this one.

Gabe

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2] Add a CBFS driver and commands to u-boot
  2011-12-05 22:25 ` Wolfgang Denk
  2011-12-05 22:35   ` Gabe Black
@ 2011-12-06  1:25   ` Gabe Black
  2011-12-06 11:01     ` Wolfgang Denk
  1 sibling, 1 reply; 13+ messages in thread
From: Gabe Black @ 2011-12-06  1:25 UTC (permalink / raw)
  To: u-boot

Coreboot uses a very simple "file system" called CBFS to keep track of and
allow access to multiple "files" in a ROM image. This change adds CBFS
support and some commands to use it to u-boot. These commands are:

cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end
of the ROM is an optional parameter which defaults to the standard
0xffffffff and can be used to support multiple CBFSes in a system. The last
one set up with cbfsinit is the one that will be used.

cbfsinfo - Print information from the CBFS header.

cbfsls - Print out the size, type, and name of all the files in the current
CBFS. Recognized types are translated into symbolic names.

cbfsload - Load a file from CBFS into memory. Like the similar command for
fat filesystems, you can optionally provide a maximum size.

Also, if u-boot needs something out of CBFS very early before the heap is
configured, it won't be able to use the normal CBFS support which caches
some information in memory it allocates from the heap. This change adds a
new cbfs_file_find_uncached function which searchs a CBFS instance without
touching the heap.

Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
specified.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
Changes in v2:
Fix checkpatch problems, change around identifiers, and change printf to
puts where possible.

 Makefile          |    6 +-
 README            |    1 +
 common/Makefile   |    1 +
 common/cmd_cbfs.c |  212 ++++++++++++++++++++++++++++++++
 fs/Makefile       |    1 +
 fs/cbfs/Makefile  |   44 +++++++
 fs/cbfs/cbfs.c    |  354 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/cbfs.h    |  164 +++++++++++++++++++++++++
 8 files changed, 780 insertions(+), 3 deletions(-)
 create mode 100644 common/cmd_cbfs.c
 create mode 100644 fs/cbfs/Makefile
 create mode 100644 fs/cbfs/cbfs.c
 create mode 100644 include/cbfs.h

diff --git a/Makefile b/Makefile
index d84b350..e1e2ff1 100644
--- a/Makefile
+++ b/Makefile
@@ -234,9 +234,9 @@ ifeq ($(CONFIG_OF_EMBED),y)
 LIBS += dts/libdts.o
 endif
 LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
-LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o fs/jffs2/libjffs2.o \
-	fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
-	fs/ubifs/libubifs.o
+LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o \
+	fs/jffs2/libjffs2.o fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o \
+	fs/yaffs2/libyaffs2.o fs/ubifs/libubifs.o fs/cbfs/libcbfs.o
 LIBS += net/libnet.o
 LIBS += disk/libdisk.o
 LIBS += drivers/bios_emulator/libatibiosemu.o
diff --git a/README b/README
index fda0190..b878c4b 100644
--- a/README
+++ b/README
@@ -721,6 +721,7 @@ The following options need to be configured:
 		CONFIG_CMD_BSP		* Board specific commands
 		CONFIG_CMD_BOOTD	  bootd
 		CONFIG_CMD_CACHE	* icache, dcache
+		CONFIG_CMD_CBFS		* Support for coreboot's CBFS
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...
diff --git a/common/Makefile b/common/Makefile
index 1b672ad..6cfba67 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -89,6 +89,7 @@ COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += cmd_eeprom.o
 COBJS-$(CONFIG_CMD_EEPROM) += cmd_eeprom.o
 COBJS-$(CONFIG_CMD_ELF) += cmd_elf.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_exit.o
+COBJS-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
 COBJS-$(CONFIG_CMD_EXT2) += cmd_ext2.o
 COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
 COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
diff --git a/common/cmd_cbfs.c b/common/cmd_cbfs.c
new file mode 100644
index 0000000..5936588
--- /dev/null
+++ b/common/cmd_cbfs.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * CBFS commands
+ */
+#include <common.h>
+#include <command.h>
+#include <cbfs.h>
+
+int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	uintptr_t end_of_rom = 0xffffffff;
+	char *ep;
+
+	if (argc > 2) {
+		puts("usage: cbfsls [end of rom]>\n");
+		return 0;
+	}
+	if (argc == 2) {
+		end_of_rom = (int)simple_strtoul(argv[1], &ep, 16);
+		if (*ep) {
+			puts("\n** Invalid end of ROM **\n");
+			return 1;
+		}
+	}
+	file_cbfs_init(end_of_rom);
+	if (file_cbfs_result != CBFS_SUCCESS) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinit,	2,	0,	do_cbfs_init,
+	"initialize the cbfs driver",
+	"[end of rom]\n"
+	"    - Initialize the cbfs driver. The optional 'end of rom'\n"
+	"      parameter specifies where the end of the ROM is that the\n"
+	"      CBFS is in. It defaults to 0xFFFFFFFF\n"
+);
+
+int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	long size;
+	unsigned long offset;
+	unsigned long count;
+	char buf[12];
+	cbfs_file file;
+
+	if (argc < 3) {
+		puts("usage: cbfsload <addr> <filename> [bytes]\n");
+		return 1;
+	}
+
+	/* parse offset and count */
+	offset = simple_strtoul(argv[1], NULL, 16);
+	if (argc == 4)
+		count = simple_strtoul(argv[3], NULL, 16);
+	else
+		count = 0;
+
+	file = file_cbfs_find(argv[2]);
+	if (!file) {
+		if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
+			printf("%s: %s\n", file_cbfs_error(), argv[2]);
+		else
+			printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	printf("reading %s\n", file_cbfs_name(file));
+
+	size = file_cbfs_read(file, (void *)offset, count);
+
+	printf("\n%ld bytes read\n", size);
+
+	sprintf(buf, "%lX", size);
+	setenv("filesize", buf);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsload,	4,	0,	do_cbfs_fsload,
+	"load binary file from a cbfs filesystem",
+	"<addr> <filename> [bytes]\n"
+	"    - load binary file 'filename' from the cbfs to address 'addr'\n"
+);
+
+int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	cbfs_file file = file_cbfs_get_first();
+	int files = 0;
+
+	if (!file) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	puts("     size              type  name\n");
+	puts("------------------------------------------\n");
+	while (file) {
+		u32 type = file_cbfs_type(file);
+		char *type_name = NULL;
+		const char *filename = file_cbfs_name(file);
+		printf(" %8d", file_cbfs_size(file));
+
+		switch (type) {
+		case CBFS_TYPE_STAGE:
+			type_name = "stage";
+			break;
+		case CBFS_TYPE_PAYLOAD:
+			type_name = "payload";
+			break;
+		case CBFS_TYPE_OPTIONROM:
+			type_name = "option rom";
+			break;
+		case CBFS_TYPE_BOOTSPLASH:
+			type_name = "boot splash";
+			break;
+		case CBFS_TYPE_RAW:
+			type_name = "raw";
+			break;
+		case CBFS_TYPE_VSA:
+			type_name = "vsa";
+			break;
+		case CBFS_TYPE_MBI:
+			type_name = "mbi";
+			break;
+		case CBFS_TYPE_MICROCODE:
+			type_name = "microcode";
+			break;
+		case CBFS_COMPONENT_CMOS_DEFAULT:
+			type_name = "cmos default";
+			break;
+		case CBFS_COMPONENT_CMOS_LAYOUT:
+			type_name = "cmos layout";
+			break;
+		case -1UL:
+			type_name = "null";
+			break;
+		}
+		if (type_name)
+			printf("  %16s", type_name);
+		else
+			printf("  %16d", type);
+
+		if (filename[0])
+			printf("  %s\n", filename);
+		else
+			printf("  %s\n", "(empty)");
+		file_cbfs_get_next(&file);
+		files++;
+	}
+
+	printf("\n%d file(s)\n\n", files);
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsls,	1,	1,	do_cbfs_ls,
+	"list files",
+	"    - list the files in the cbfs\n"
+);
+
+int do_cbfs_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	const struct cbfs_header *header = file_cbfs_get_header();
+	if (!header) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	puts("\n");
+	printf("CBFS version: %#x\n", header->version);
+	printf("ROM size: %#x\n", header->rom_size);
+	printf("Boot block size: %#x\n", header->boot_block_size);
+	printf("CBFS size: %#x\n",
+		header->rom_size - header->boot_block_size - header->offset);
+	printf("Alignment: %d\n", header->align);
+	printf("Offset: %#x\n", header->offset);
+	puts("\n");
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinfo,	1,	1,	do_cbfs_fsinfo,
+	"print information about filesystem",
+	"    - print information about the cbfs filesystem\n"
+);
diff --git a/fs/Makefile b/fs/Makefile
index 22aad12..ca21464 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -23,6 +23,7 @@
 #
 
 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
diff --git a/fs/cbfs/Makefile b/fs/cbfs/Makefile
new file mode 100644
index 0000000..864fdf9
--- /dev/null
+++ b/fs/cbfs/Makefile
@@ -0,0 +1,44 @@
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libcbfs.o
+
+AOBJS	=
+COBJS-$(CONFIG_CMD_CBFS)	:= cbfs.o
+
+SRCS	:= $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(AOBJS) $(COBJS-y))
+
+all:	$(LIB) $(AOBJS)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
new file mode 100644
index 0000000..1a7eed0
--- /dev/null
+++ b/fs/cbfs/cbfs.c
@@ -0,0 +1,354 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <cbfs.h>
+#include <malloc.h>
+#include <asm/byteorder.h>
+
+enum cbfs_result file_cbfs_result;
+
+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";
+	}
+}
+
+struct cbfs_file_header {
+	u8 magic[8];
+	u32 len;
+	u32 type;
+	u32 checksum;
+	u32 offset;
+} __packed;
+
+struct cbfs_cache_node {
+	struct cbfs_cache_node *next;
+	u32 type;
+	void *data;
+	u32 data_length;
+	char *name;
+	u32 name_length;
+	u32 checksum;
+} __packed;
+
+
+static const u32 good_magic = 0x4f524243;
+static const u8 good_file_magic[] = "LARCHIVE";
+
+
+static int initialized;
+static struct cbfs_header cbfs_header;
+static struct cbfs_cache_node *file_cache;
+
+/* Do endian conversion on the CBFS header structure. */
+static void swap_header(struct cbfs_header *dest, struct cbfs_header *src)
+{
+	dest->magic = be32_to_cpu(src->magic);
+	dest->version = be32_to_cpu(src->version);
+	dest->rom_size = be32_to_cpu(src->rom_size);
+	dest->boot_block_size = be32_to_cpu(src->boot_block_size);
+	dest->align = be32_to_cpu(src->align);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/* Do endian conversion on a CBFS file header. */
+static void swap_file_header(struct cbfs_file_header *dest,
+			     struct cbfs_file_header *src)
+{
+	memcpy(&dest->magic, &src->magic, sizeof(dest->magic));
+	dest->len = be32_to_cpu(src->len);
+	dest->type = be32_to_cpu(src->type);
+	dest->checksum = be32_to_cpu(src->checksum);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/*
+ * Given a starting position in memory, scan forward, bounded by a size, and
+ * find the next valid CBFS file. No memory is allocated by this function. The
+ * caller is responsible for allocating space for the new file structure.
+ *
+ * @param start		The location in memory to start from.
+ * @param size		The size of the memory region to search.
+ * @param align		The alignment boundaries to check on.
+ * @param newNode	A pointer to the file structure to load.
+ * @param used		A pointer to the count of of bytes scanned through,
+ *			including the file if one is found.
+ *
+ * @return 1 if a file is found, 0 if one isn't.
+ */
+static int file_cbfs_next_file(u8 *start, u32 size, u32 align,
+			       struct cbfs_cache_node *new_node, u32 *used)
+{
+	struct cbfs_file_header header;
+
+	*used = 0;
+
+	while (size >= align) {
+		struct cbfs_file_header *file_header =
+			(struct cbfs_file_header *)start;
+		u32 name_len;
+		u32 step;
+
+		/* Check if there's a file here. */
+		if (memcmp(good_file_magic, &(file_header->magic),
+				sizeof(file_header->magic))) {
+			*used += align;
+			size -= align;
+			start += align;
+			continue;
+		}
+
+		swap_file_header(&header, file_header);
+		if (header.offset < sizeof(struct cbfs_file_header) ||
+				header.offset > header.len) {
+			file_cbfs_result = CBFS_BAD_FILE;
+			return -1;
+		}
+		new_node->next = NULL;
+		new_node->type = header.type;
+		new_node->data = start + header.offset;
+		new_node->data_length = header.len;
+		name_len = header.offset - sizeof(struct cbfs_file_header);
+		new_node->name = (char *)file_header +
+			sizeof(struct cbfs_file_header);
+		new_node->name_length = name_len;
+		new_node->checksum = header.checksum;
+
+		step = header.len;
+		if (step % align)
+			step = step + align - step % align;
+
+		*used += step;
+		return 1;
+	}
+	return 0;
+}
+
+/* Look through a CBFS instance and copy file metadata into regular memory. */
+static void file_cbfs_fill_cache(u8 *start, u32 size, u32 align)
+{
+	struct cbfs_cache_node *cache_node;
+	struct cbfs_cache_node *new_node;
+	struct cbfs_cache_node **cache_tail = &file_cache;
+
+	/* Clear out old information. */
+	cache_node = file_cache;
+	while (cache_node) {
+		struct cbfs_cache_node *old_node = cache_node;
+		cache_node = cache_node->next;
+		free(old_node);
+	}
+	file_cache = NULL;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		new_node = (struct cbfs_cache_node *)
+			malloc(sizeof(struct cbfs_cache_node));
+		result = file_cbfs_next_file(start, size, align,
+			new_node, &used);
+
+		if (result < 0) {
+			free(new_node);
+			return;
+		} else if (result == 0) {
+			free(new_node);
+			break;
+		}
+		*cache_tail = new_node;
+		cache_tail = &new_node->next;
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+/* Get the CBFS header out of the ROM and do endian conversion. */
+static int file_cbfs_load_header(uintptr_t end_of_rom,
+				 struct cbfs_header *header)
+{
+	struct cbfs_header *header_in_rom;
+
+	header_in_rom = (struct cbfs_header *)(uintptr_t)
+		*(u32 *)(end_of_rom - 3);
+	swap_header(header, header_in_rom);
+
+	if (header->magic != good_magic || header->offset >
+			header->rom_size - header->boot_block_size) {
+		file_cbfs_result = CBFS_BAD_HEADER;
+		return 1;
+	}
+	return 0;
+}
+
+void file_cbfs_init(uintptr_t end_of_rom)
+{
+	u8 *start_of_rom;
+	initialized = 0;
+
+	if (file_cbfs_load_header(end_of_rom, &cbfs_header))
+		return;
+
+	start_of_rom = (u8 *)(end_of_rom + 1 - cbfs_header.rom_size);
+
+	file_cbfs_fill_cache(start_of_rom + cbfs_header.offset,
+			     cbfs_header.rom_size, cbfs_header.align);
+	if (file_cbfs_result == CBFS_SUCCESS)
+		initialized = 1;
+}
+
+const struct cbfs_header *file_cbfs_get_header(void)
+{
+	if (initialized) {
+		file_cbfs_result = CBFS_SUCCESS;
+		return &cbfs_header;
+	} else {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+}
+
+cbfs_file file_cbfs_get_first(void)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	} else {
+		file_cbfs_result = CBFS_SUCCESS;
+		return file_cache;
+	}
+}
+
+void file_cbfs_get_next(cbfs_file *file)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		file = NULL;
+		return;
+	}
+
+	if (*file)
+		*file = (*file)->next;
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+cbfs_file file_cbfs_find(const char *name)
+{
+	struct cbfs_cache_node *cache_node = file_cache;
+
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+
+	while (cache_node) {
+		if (!strcmp(name, cache_node->name))
+			break;
+		cache_node = cache_node->next;
+	}
+	if (!cache_node)
+		file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	else
+		file_cbfs_result = CBFS_SUCCESS;
+	return cache_node;
+}
+
+cbfs_file file_cbfs_find_uncached(uintptr_t end_of_rom, const char *name)
+{
+	u8 *start;
+	u32 size;
+	u32 align;
+	static struct cbfs_cache_node node;
+
+	if (file_cbfs_load_header(end_of_rom, &cbfs_header))
+		return NULL;
+
+	start = (u8 *)(end_of_rom + 1 - cbfs_header.rom_size);
+	size = cbfs_header.rom_size;
+	align = cbfs_header.align;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		result = file_cbfs_next_file(start, size, align, &node, &used);
+
+		if (result < 0)
+			return NULL;
+		else if (result == 0)
+			break;
+
+		if (!strcmp(name, node.name))
+			return &node;
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	return NULL;
+}
+
+const char *file_cbfs_name(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->name;
+}
+
+u32 file_cbfs_size(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->data_length;
+}
+
+u32 file_cbfs_type(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->type;
+}
+
+long file_cbfs_read(cbfs_file file, void *buffer, unsigned long maxsize)
+{
+	u32 size;
+
+	size = file->data_length;
+	if (maxsize && size > maxsize)
+		size = maxsize;
+
+	memcpy(buffer, file->data, size);
+
+	file_cbfs_result = CBFS_SUCCESS;
+	return size;
+}
diff --git a/include/cbfs.h b/include/cbfs.h
new file mode 100644
index 0000000..9e97c2d
--- /dev/null
+++ b/include/cbfs.h
@@ -0,0 +1,164 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CBFS_H
+#define __CBFS_H
+
+#include <compiler.h>
+#include <linux/compiler.h>
+
+enum cbfs_result {
+	CBFS_SUCCESS = 0,
+	CBFS_NOT_INITIALIZED,
+	CBFS_BAD_HEADER,
+	CBFS_BAD_FILE,
+	CBFS_FILE_NOT_FOUND
+};
+
+enum cbfs_file_type {
+	CBFS_TYPE_STAGE = 0x10,
+	CBFS_TYPE_PAYLOAD = 0x20,
+	CBFS_TYPE_OPTIONROM = 0x30,
+	CBFS_TYPE_BOOTSPLASH = 0x40,
+	CBFS_TYPE_RAW = 0x50,
+	CBFS_TYPE_VSA = 0x51,
+	CBFS_TYPE_MBI = 0x52,
+	CBFS_TYPE_MICROCODE = 0x53,
+	CBFS_COMPONENT_CMOS_DEFAULT = 0xaa,
+	CBFS_COMPONENT_CMOS_LAYOUT = 0x01aa
+};
+
+struct cbfs_header {
+	u32 magic;
+	u32 version;
+	u32 rom_size;
+	u32 boot_block_size;
+	u32 align;
+	u32 offset;
+	u32 pad[2];
+} __packed;
+
+struct cbfs_cache_node;
+typedef const struct cbfs_cache_node *cbfs_file;
+
+extern enum cbfs_result file_cbfs_result;
+
+/*
+ * Return a string describing the most recent error condition.
+ *
+ * @return A pointer to the constant string.
+ */
+const char *file_cbfs_error(void);
+
+/*
+ * Initialize the CBFS driver and load metadata into RAM.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ */
+void file_cbfs_init(uintptr_t end_of_rom);
+
+/*
+ * Get the header structure for the current CBFS.
+ *
+ * @return A pointer to the constant structure, or NULL if there is none.
+ */
+const struct cbfs_header *file_cbfs_get_header(void);
+
+/*
+ * Get a handle for the first file in CBFS.
+ *
+ * @return A handle for the first file in CBFS, NULL on error.
+ */
+cbfs_file file_cbfs_get_first(void);
+
+/*
+ * Get a handle to the file after this one in CBFS.
+ *
+ * @param file		A pointer to the handle to advance.
+ */
+void file_cbfs_get_next(cbfs_file *file);
+
+/*
+ * Find a file with a particular name in CBFS.
+ *
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+cbfs_file file_cbfs_find(const char *name);
+
+
+/***************************************************************************/
+/* All of the functions below can be used without first initializing CBFS. */
+/***************************************************************************/
+
+/*
+ * Find a file with a particular name in CBFS without using the heap.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+cbfs_file file_cbfs_find_uncached(uintptr_t end_of_rom, const char *name);
+
+/*
+ * Get the name of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The name of the file, NULL on error.
+ */
+const char *file_cbfs_name(cbfs_file file);
+
+/*
+ * Get the size of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The size of the file, zero on error.
+ */
+u32 file_cbfs_size(cbfs_file file);
+
+/*
+ * Get the type of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The type of the file, zero on error.
+ */
+u32 file_cbfs_type(cbfs_file file);
+
+/*
+ * Read a file from CBFS into RAM
+ *
+ * @param file		A handle to the file to read.
+ * @param buffer	Where to read it into memory.
+ *
+ * @return If positive or zero, the number of characters read. If negative, an
+ *         error occurred.
+ */
+long file_cbfs_read(cbfs_file file, void *buffer, unsigned long maxsize);
+
+#endif /* __CBFS_H */
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2] Add a CBFS driver and commands to u-boot
  2011-12-06  1:25   ` [U-Boot] [PATCH v2] " Gabe Black
@ 2011-12-06 11:01     ` Wolfgang Denk
  2011-12-06 23:32       ` Gabe Black
  2011-12-06 23:36       ` [U-Boot] [PATCH v3] " Gabe Black
  0 siblings, 2 replies; 13+ messages in thread
From: Wolfgang Denk @ 2011-12-06 11:01 UTC (permalink / raw)
  To: u-boot

Dear Gabe Black,

In message <1323134730-18471-1-git-send-email-gabeblack@chromium.org> you wrote:
> Coreboot uses a very simple "file system" called CBFS to keep track of and
> allow access to multiple "files" in a ROM image. This change adds CBFS
> support and some commands to use it to u-boot. These commands are:
> 
> cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end
> of the ROM is an optional parameter which defaults to the standard
> 0xffffffff and can be used to support multiple CBFSes in a system. The last
> one set up with cbfsinit is the one that will be used.
> 
> cbfsinfo - Print information from the CBFS header.
> 
> cbfsls - Print out the size, type, and name of all the files in the current
> CBFS. Recognized types are translated into symbolic names.
> 
> cbfsload - Load a file from CBFS into memory. Like the similar command for
> fat filesystems, you can optionally provide a maximum size.
> 
> Also, if u-boot needs something out of CBFS very early before the heap is
> configured, it won't be able to use the normal CBFS support which caches
> some information in memory it allocates from the heap. This change adds a
> new cbfs_file_find_uncached function which searchs a CBFS instance without
> touching the heap.
> 
> Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
> specified.
> 
> Signed-off-by: Gabe Black <gabeblack@chromium.org>
> ---
> Changes in v2:
> Fix checkpatch problems, change around identifiers, and change printf to
> puts where possible.

There is still a checkpatch warning that should be fixed:

WARNING: do not add new typedefs
#853: FILE: include/cbfs.h:61:
+typedef const struct cbfs_cache_node *cbfs_file;


> --- a/Makefile
> +++ b/Makefile
> @@ -234,9 +234,9 @@ ifeq ($(CONFIG_OF_EMBED),y)
>  LIBS += dts/libdts.o
>  endif
>  LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
> -LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o fs/jffs2/libjffs2.o \
> -	fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
> -	fs/ubifs/libubifs.o
> +LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o \
> +	fs/jffs2/libjffs2.o fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o \
> +	fs/yaffs2/libyaffs2.o fs/ubifs/libubifs.o fs/cbfs/libcbfs.o

Please keep the list sorted. [It may make sense to split it into a
list of entries with one FS per line.]

> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -89,6 +89,7 @@ COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += cmd_eeprom.o
>  COBJS-$(CONFIG_CMD_EEPROM) += cmd_eeprom.o
>  COBJS-$(CONFIG_CMD_ELF) += cmd_elf.o
>  COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_exit.o
> +COBJS-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
>  COBJS-$(CONFIG_CMD_EXT2) += cmd_ext2.o
>  COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
>  COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o

Please keep sorted (by object file names).


> +int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> +	uintptr_t end_of_rom = 0xffffffff;
> +	char *ep;
> +
> +	if (argc > 2) {
> +		puts("usage: cbfsls [end of rom]>\n");

What is the meaning / intention of that tailing '>' ?

> +	if (file_cbfs_result != CBFS_SUCCESS) {
> +		printf("%s.\n", file_cbfs_error());

Use
		puts(file_cbfs_error());
?

> +int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
...
> +	if (!file) {
> +		if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
> +			printf("%s: %s\n", file_cbfs_error(), argv[2]);
> +		else
> +			printf("%s.\n", file_cbfs_error());

See above. Please consider changing globally.

> +	printf("reading %s\n", file_cbfs_name(file));
> +
> +	size = file_cbfs_read(file, (void *)offset, count);
> +
> +	printf("\n%ld bytes read\n", size);

There should be no need for the initial newline here.  Please drop it
(fix globally?).

...
> +		if (type_name)
> +			printf("  %16s", type_name);
> +		else
> +			printf("  %16d", type);

This is probably confusing to the user.  How can he know if the file
type has the numerical value of "123" or if the file name is "123" ?

> +		if (filename[0])
> +			printf("  %s\n", filename);
> +		else
> +			printf("  %s\n", "(empty)");

Use puts().

> +	printf("\n%d file(s)\n\n", files);

Please do not print that many empty lines.

Imagine output is going to a QVGA display or similar which shows only
12 text lines or so...

> +int do_cbfs_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> +	const struct cbfs_header *header = file_cbfs_get_header();
> +	if (!header) {
> +		printf("%s.\n", file_cbfs_error());
> +		return 1;
> +	}

Please always insert a blank line between declarations and code.
Please fix globally.

> +	puts("\n");

What is this blank line good for?  Drop it!

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

How about some vertical alignment of the output?

> +	puts("\n");

Drop that, too.

> +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";

Better make this "Unknown error" or similar, otherwise the user might
not know what "Unknown" means.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A good aphorism is too hard for the tooth of time, and  is  not  worn
away  by  all  the  centuries,  although  it serves as food for every
epoch.                                  - Friedrich Wilhelm Nietzsche
                          _Miscellaneous Maxims and Opinions_ no. 168

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2] Add a CBFS driver and commands to u-boot
  2011-12-06 11:01     ` Wolfgang Denk
@ 2011-12-06 23:32       ` Gabe Black
  2011-12-07  8:01         ` Wolfgang Denk
  2011-12-06 23:36       ` [U-Boot] [PATCH v3] " Gabe Black
  1 sibling, 1 reply; 13+ messages in thread
From: Gabe Black @ 2011-12-06 23:32 UTC (permalink / raw)
  To: u-boot

On Tue, Dec 6, 2011 at 3:01 AM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Gabe Black,
>
> In message <1323134730-18471-1-git-send-email-gabeblack@chromium.org> you
> wrote:
> > Coreboot uses a very simple "file system" called CBFS to keep track of
> and
> > allow access to multiple "files" in a ROM image. This change adds CBFS
> > support and some commands to use it to u-boot. These commands are:
> >
> > cbfsinit - Initialize CBFS support and pull all metadata into RAM. The
> end
> > of the ROM is an optional parameter which defaults to the standard
> > 0xffffffff and can be used to support multiple CBFSes in a system. The
> last
> > one set up with cbfsinit is the one that will be used.
> >
> > cbfsinfo - Print information from the CBFS header.
> >
> > cbfsls - Print out the size, type, and name of all the files in the
> current
> > CBFS. Recognized types are translated into symbolic names.
> >
> > cbfsload - Load a file from CBFS into memory. Like the similar command
> for
> > fat filesystems, you can optionally provide a maximum size.
> >
> > Also, if u-boot needs something out of CBFS very early before the heap is
> > configured, it won't be able to use the normal CBFS support which caches
> > some information in memory it allocates from the heap. This change adds a
> > new cbfs_file_find_uncached function which searchs a CBFS instance
> without
> > touching the heap.
> >
> > Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
> > specified.
> >
> > Signed-off-by: Gabe Black <gabeblack@chromium.org>
> > ---
> > Changes in v2:
> > Fix checkpatch problems, change around identifiers, and change printf to
> > puts where possible.
>
> There is still a checkpatch warning that should be fixed:
>
> WARNING: do not add new typedefs
> #853: FILE: include/cbfs.h:61:
> +typedef const struct cbfs_cache_node *cbfs_file;
>


This typedef is to create an opaque handle for CBFS files. If you look here:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle

In chapter 5 in the list of acceptable uses for typedefs, creating an
opaque type is item a. This use of typedef is consistent with the Linux
coding standards as described in that document and functionally important
for this change and should stay.



> > +int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[])
> > +{
> > +     uintptr_t end_of_rom = 0xffffffff;
> > +     char *ep;
> > +
> > +     if (argc > 2) {
> > +             puts("usage: cbfsls [end of rom]>\n");
>
> What is the meaning / intention of that tailing '>' ?
>
>

A typo.



> > +     if (file_cbfs_result != CBFS_SUCCESS) {
> > +             printf("%s.\n", file_cbfs_error());
>
> Use
>                puts(file_cbfs_error());
> ?
>


That would leave out the \n. Whatever came next (like the prompt) would
continue on the same line which would be wrong.



>
> > +int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[])
> > +{
> ...
> > +     if (!file) {
> > +             if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
> > +                     printf("%s: %s\n", file_cbfs_error(), argv[2]);
> > +             else
> > +                     printf("%s.\n", file_cbfs_error());
>
> See above. Please consider changing globally.
>


The same explanation applies globally.



> ...
> > +             if (type_name)
> > +                     printf("  %16s", type_name);
> > +             else
> > +                     printf("  %16d", type);
>
> This is probably confusing to the user.  How can he know if the file
> type has the numerical value of "123" or if the file name is "123" ?
>


This isn't  file name, it's the file type. No file type is named after a
number. There are also labels on the columns indicating which is which.



>
> > +             if (filename[0])
> > +                     printf("  %s\n", filename);
> > +             else
> > +                     printf("  %s\n", "(empty)");
>
> Use puts().
>


The first one can't be a puts, it would have to be three. That's a lot of
clutter to avoid a slight performance penalty when interacting with a human
that won't notice such a tiny delay anyway. The second one could be a puts,
but that would make the two branches of the if uneven. I'll change the
second one.



>
> > +     printf("\n%d file(s)\n\n", files);
>
> Please do not print that many empty lines.
>
> Imagine output is going to a QVGA display or similar which shows only
> 12 text lines or so...
>


I put newlines there because the FAT commands do, and I have no problem
removing them. You may want to look at the other commands and remove them
there too.



>
> > +     puts("\n");
>
> What is this blank line good for?  Drop it!
>


Readability? I don't care that much one way or the other though. I'll
change it.

Gabe

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v3] Add a CBFS driver and commands to u-boot
  2011-12-06 11:01     ` Wolfgang Denk
  2011-12-06 23:32       ` Gabe Black
@ 2011-12-06 23:36       ` Gabe Black
  2011-12-07  8:03         ` Wolfgang Denk
  1 sibling, 1 reply; 13+ messages in thread
From: Gabe Black @ 2011-12-06 23:36 UTC (permalink / raw)
  To: u-boot

Coreboot uses a very simple "file system" called CBFS to keep track of and
allow access to multiple "files" in a ROM image. This change adds CBFS
support and some commands to use it to u-boot. These commands are:

cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end
of the ROM is an optional parameter which defaults to the standard
0xffffffff and can be used to support multiple CBFSes in a system. The last
one set up with cbfsinit is the one that will be used.

cbfsinfo - Print information from the CBFS header.

cbfsls - Print out the size, type, and name of all the files in the current
CBFS. Recognized types are translated into symbolic names.

cbfsload - Load a file from CBFS into memory. Like the similar command for
fat filesystems, you can optionally provide a maximum size.

Also, if u-boot needs something out of CBFS very early before the heap is
configured, it won't be able to use the normal CBFS support which caches
some information in memory it allocates from the heap. This change adds a
new cbfs_file_find_uncached function which searchs a CBFS instance without
touching the heap.

Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
specified.

Signed-off-by: Gabe Black <gabeblack@chromium.org>
---
Changes in v2:
Fix checkpatch problems, change around identifiers, and change printf to
puts where possible.

Changes in v3:
Formatting changes, "Unknown" => "Unknown error", Makefile sorting, fix a
typo.

 Makefile          |    6 +-
 README            |    1 +
 common/Makefile   |    1 +
 common/cmd_cbfs.c |  212 +++++++++++++++++++++++++++++++
 fs/Makefile       |    1 +
 fs/cbfs/Makefile  |   44 +++++++
 fs/cbfs/cbfs.c    |  355 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/cbfs.h    |  164 ++++++++++++++++++++++++
 8 files changed, 781 insertions(+), 3 deletions(-)
 create mode 100644 common/cmd_cbfs.c
 create mode 100644 fs/cbfs/Makefile
 create mode 100644 fs/cbfs/cbfs.c
 create mode 100644 include/cbfs.h

diff --git a/Makefile b/Makefile
index d84b350..3056fed 100644
--- a/Makefile
+++ b/Makefile
@@ -234,9 +234,9 @@ ifeq ($(CONFIG_OF_EMBED),y)
 LIBS += dts/libdts.o
 endif
 LIBS += arch/$(ARCH)/lib/lib$(ARCH).o
-LIBS += fs/cramfs/libcramfs.o fs/fat/libfat.o fs/fdos/libfdos.o fs/jffs2/libjffs2.o \
-	fs/reiserfs/libreiserfs.o fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o \
-	fs/ubifs/libubifs.o
+LIBS += fs/cbfs/libcbfs.o fs/cramfs/libcramfs.o fs/fat/libfat.o \
+	fs/fdos/libfdos.o fs/jffs2/libjffs2.o fs/reiserfs/libreiserfs.o \
+	fs/ext2/libext2fs.o fs/yaffs2/libyaffs2.o fs/ubifs/libubifs.o
 LIBS += net/libnet.o
 LIBS += disk/libdisk.o
 LIBS += drivers/bios_emulator/libatibiosemu.o
diff --git a/README b/README
index fda0190..b878c4b 100644
--- a/README
+++ b/README
@@ -721,6 +721,7 @@ The following options need to be configured:
 		CONFIG_CMD_BSP		* Board specific commands
 		CONFIG_CMD_BOOTD	  bootd
 		CONFIG_CMD_CACHE	* icache, dcache
+		CONFIG_CMD_CBFS		* Support for coreboot's CBFS
 		CONFIG_CMD_CONSOLE	  coninfo
 		CONFIG_CMD_CRC32	* crc32
 		CONFIG_CMD_DATE		* support for RTC, date/time...
diff --git a/common/Makefile b/common/Makefile
index 1b672ad..d3633a3 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -72,6 +72,7 @@ COBJS-$(CONFIG_CMD_BEDBUG) += bedbug.o cmd_bedbug.o
 COBJS-$(CONFIG_CMD_BMP) += cmd_bmp.o
 COBJS-$(CONFIG_CMD_BOOTLDR) += cmd_bootldr.o
 COBJS-$(CONFIG_CMD_CACHE) += cmd_cache.o
+COBJS-$(CONFIG_CMD_CBFS) += cmd_cbfs.o
 COBJS-$(CONFIG_CMD_CONSOLE) += cmd_console.o
 COBJS-$(CONFIG_CMD_CPLBINFO) += cmd_cplbinfo.o
 COBJS-$(CONFIG_DATAFLASH_MMC_SELECT) += cmd_dataflash_mmc_mux.o
diff --git a/common/cmd_cbfs.c b/common/cmd_cbfs.c
new file mode 100644
index 0000000..4748883
--- /dev/null
+++ b/common/cmd_cbfs.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * CBFS commands
+ */
+#include <common.h>
+#include <command.h>
+#include <cbfs.h>
+
+int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	uintptr_t end_of_rom = 0xffffffff;
+	char *ep;
+
+	if (argc > 2) {
+		puts("usage: cbfsls [end of rom]\n");
+		return 0;
+	}
+	if (argc == 2) {
+		end_of_rom = (int)simple_strtoul(argv[1], &ep, 16);
+		if (*ep) {
+			puts("** Invalid end of ROM **\n");
+			return 1;
+		}
+	}
+	file_cbfs_init(end_of_rom);
+	if (file_cbfs_result != CBFS_SUCCESS) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinit,	2,	0,	do_cbfs_init,
+	"initialize the cbfs driver",
+	"[end of rom]\n"
+	"    - Initialize the cbfs driver. The optional 'end of rom'\n"
+	"      parameter specifies where the end of the ROM is that the\n"
+	"      CBFS is in. It defaults to 0xFFFFFFFF\n"
+);
+
+int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	long size;
+	unsigned long offset;
+	unsigned long count;
+	char buf[12];
+	cbfs_file file;
+
+	if (argc < 3) {
+		puts("usage: cbfsload <addr> <filename> [bytes]\n");
+		return 1;
+	}
+
+	/* parse offset and count */
+	offset = simple_strtoul(argv[1], NULL, 16);
+	if (argc == 4)
+		count = simple_strtoul(argv[3], NULL, 16);
+	else
+		count = 0;
+
+	file = file_cbfs_find(argv[2]);
+	if (!file) {
+		if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
+			printf("%s: %s\n", file_cbfs_error(), argv[2]);
+		else
+			printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	printf("reading %s\n", file_cbfs_name(file));
+
+	size = file_cbfs_read(file, (void *)offset, count);
+
+	printf("%ld bytes read\n", size);
+
+	sprintf(buf, "%lX", size);
+	setenv("filesize", buf);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsload,	4,	0,	do_cbfs_fsload,
+	"load binary file from a cbfs filesystem",
+	"<addr> <filename> [bytes]\n"
+	"    - load binary file 'filename' from the cbfs to address 'addr'\n"
+);
+
+int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	cbfs_file file = file_cbfs_get_first();
+	int files = 0;
+
+	if (!file) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	puts("     size              type  name\n");
+	puts("------------------------------------------\n");
+	while (file) {
+		u32 type = file_cbfs_type(file);
+		char *type_name = NULL;
+		const char *filename = file_cbfs_name(file);
+
+		printf(" %8d", file_cbfs_size(file));
+
+		switch (type) {
+		case CBFS_TYPE_STAGE:
+			type_name = "stage";
+			break;
+		case CBFS_TYPE_PAYLOAD:
+			type_name = "payload";
+			break;
+		case CBFS_TYPE_OPTIONROM:
+			type_name = "option rom";
+			break;
+		case CBFS_TYPE_BOOTSPLASH:
+			type_name = "boot splash";
+			break;
+		case CBFS_TYPE_RAW:
+			type_name = "raw";
+			break;
+		case CBFS_TYPE_VSA:
+			type_name = "vsa";
+			break;
+		case CBFS_TYPE_MBI:
+			type_name = "mbi";
+			break;
+		case CBFS_TYPE_MICROCODE:
+			type_name = "microcode";
+			break;
+		case CBFS_COMPONENT_CMOS_DEFAULT:
+			type_name = "cmos default";
+			break;
+		case CBFS_COMPONENT_CMOS_LAYOUT:
+			type_name = "cmos layout";
+			break;
+		case -1UL:
+			type_name = "null";
+			break;
+		}
+		if (type_name)
+			printf("  %16s", type_name);
+		else
+			printf("  %16d", type);
+
+		if (filename[0])
+			printf("  %s\n", filename);
+		else
+			puts("  (empty)\n");
+		file_cbfs_get_next(&file);
+		files++;
+	}
+
+	printf("%d file(s)\n\n", files);
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsls,	1,	1,	do_cbfs_ls,
+	"list files",
+	"    - list the files in the cbfs\n"
+);
+
+int do_cbfs_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	const struct cbfs_header *header = file_cbfs_get_header();
+
+	if (!header) {
+		printf("%s.\n", file_cbfs_error());
+		return 1;
+	}
+
+	printf("CBFS version:    %#x\n", header->version);
+	printf("ROM size:        %#x\n", header->rom_size);
+	printf("Boot block size: %#x\n", header->boot_block_size);
+	printf("CBFS size:       %#x\n",
+		header->rom_size - header->boot_block_size - header->offset);
+	printf("Alignment:       %d\n", header->align);
+	printf("Offset:          %#x\n", header->offset);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	cbfsinfo,	1,	1,	do_cbfs_fsinfo,
+	"print information about filesystem",
+	"    - print information about the cbfs filesystem\n"
+);
diff --git a/fs/Makefile b/fs/Makefile
index 22aad12..ca21464 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -23,6 +23,7 @@
 #
 
 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
diff --git a/fs/cbfs/Makefile b/fs/cbfs/Makefile
new file mode 100644
index 0000000..864fdf9
--- /dev/null
+++ b/fs/cbfs/Makefile
@@ -0,0 +1,44 @@
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libcbfs.o
+
+AOBJS	=
+COBJS-$(CONFIG_CMD_CBFS)	:= cbfs.o
+
+SRCS	:= $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
+OBJS	:= $(addprefix $(obj),$(AOBJS) $(COBJS-y))
+
+all:	$(LIB) $(AOBJS)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
new file mode 100644
index 0000000..4be73e6
--- /dev/null
+++ b/fs/cbfs/cbfs.c
@@ -0,0 +1,355 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <cbfs.h>
+#include <malloc.h>
+#include <asm/byteorder.h>
+
+enum cbfs_result file_cbfs_result;
+
+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 error";
+	}
+}
+
+struct cbfs_file_header {
+	u8 magic[8];
+	u32 len;
+	u32 type;
+	u32 checksum;
+	u32 offset;
+} __packed;
+
+struct cbfs_cache_node {
+	struct cbfs_cache_node *next;
+	u32 type;
+	void *data;
+	u32 data_length;
+	char *name;
+	u32 name_length;
+	u32 checksum;
+} __packed;
+
+
+static const u32 good_magic = 0x4f524243;
+static const u8 good_file_magic[] = "LARCHIVE";
+
+
+static int initialized;
+static struct cbfs_header cbfs_header;
+static struct cbfs_cache_node *file_cache;
+
+/* Do endian conversion on the CBFS header structure. */
+static void swap_header(struct cbfs_header *dest, struct cbfs_header *src)
+{
+	dest->magic = be32_to_cpu(src->magic);
+	dest->version = be32_to_cpu(src->version);
+	dest->rom_size = be32_to_cpu(src->rom_size);
+	dest->boot_block_size = be32_to_cpu(src->boot_block_size);
+	dest->align = be32_to_cpu(src->align);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/* Do endian conversion on a CBFS file header. */
+static void swap_file_header(struct cbfs_file_header *dest,
+			     struct cbfs_file_header *src)
+{
+	memcpy(&dest->magic, &src->magic, sizeof(dest->magic));
+	dest->len = be32_to_cpu(src->len);
+	dest->type = be32_to_cpu(src->type);
+	dest->checksum = be32_to_cpu(src->checksum);
+	dest->offset = be32_to_cpu(src->offset);
+}
+
+/*
+ * Given a starting position in memory, scan forward, bounded by a size, and
+ * find the next valid CBFS file. No memory is allocated by this function. The
+ * caller is responsible for allocating space for the new file structure.
+ *
+ * @param start		The location in memory to start from.
+ * @param size		The size of the memory region to search.
+ * @param align		The alignment boundaries to check on.
+ * @param newNode	A pointer to the file structure to load.
+ * @param used		A pointer to the count of of bytes scanned through,
+ *			including the file if one is found.
+ *
+ * @return 1 if a file is found, 0 if one isn't.
+ */
+static int file_cbfs_next_file(u8 *start, u32 size, u32 align,
+			       struct cbfs_cache_node *new_node, u32 *used)
+{
+	struct cbfs_file_header header;
+
+	*used = 0;
+
+	while (size >= align) {
+		struct cbfs_file_header *file_header =
+			(struct cbfs_file_header *)start;
+		u32 name_len;
+		u32 step;
+
+		/* Check if there's a file here. */
+		if (memcmp(good_file_magic, &(file_header->magic),
+				sizeof(file_header->magic))) {
+			*used += align;
+			size -= align;
+			start += align;
+			continue;
+		}
+
+		swap_file_header(&header, file_header);
+		if (header.offset < sizeof(struct cbfs_file_header) ||
+				header.offset > header.len) {
+			file_cbfs_result = CBFS_BAD_FILE;
+			return -1;
+		}
+		new_node->next = NULL;
+		new_node->type = header.type;
+		new_node->data = start + header.offset;
+		new_node->data_length = header.len;
+		name_len = header.offset - sizeof(struct cbfs_file_header);
+		new_node->name = (char *)file_header +
+			sizeof(struct cbfs_file_header);
+		new_node->name_length = name_len;
+		new_node->checksum = header.checksum;
+
+		step = header.len;
+		if (step % align)
+			step = step + align - step % align;
+
+		*used += step;
+		return 1;
+	}
+	return 0;
+}
+
+/* Look through a CBFS instance and copy file metadata into regular memory. */
+static void file_cbfs_fill_cache(u8 *start, u32 size, u32 align)
+{
+	struct cbfs_cache_node *cache_node;
+	struct cbfs_cache_node *new_node;
+	struct cbfs_cache_node **cache_tail = &file_cache;
+
+	/* Clear out old information. */
+	cache_node = file_cache;
+	while (cache_node) {
+		struct cbfs_cache_node *old_node = cache_node;
+		cache_node = cache_node->next;
+		free(old_node);
+	}
+	file_cache = NULL;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		new_node = (struct cbfs_cache_node *)
+			malloc(sizeof(struct cbfs_cache_node));
+		result = file_cbfs_next_file(start, size, align,
+			new_node, &used);
+
+		if (result < 0) {
+			free(new_node);
+			return;
+		} else if (result == 0) {
+			free(new_node);
+			break;
+		}
+		*cache_tail = new_node;
+		cache_tail = &new_node->next;
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+/* Get the CBFS header out of the ROM and do endian conversion. */
+static int file_cbfs_load_header(uintptr_t end_of_rom,
+				 struct cbfs_header *header)
+{
+	struct cbfs_header *header_in_rom;
+
+	header_in_rom = (struct cbfs_header *)(uintptr_t)
+		*(u32 *)(end_of_rom - 3);
+	swap_header(header, header_in_rom);
+
+	if (header->magic != good_magic || header->offset >
+			header->rom_size - header->boot_block_size) {
+		file_cbfs_result = CBFS_BAD_HEADER;
+		return 1;
+	}
+	return 0;
+}
+
+void file_cbfs_init(uintptr_t end_of_rom)
+{
+	u8 *start_of_rom;
+
+	initialized = 0;
+
+	if (file_cbfs_load_header(end_of_rom, &cbfs_header))
+		return;
+
+	start_of_rom = (u8 *)(end_of_rom + 1 - cbfs_header.rom_size);
+
+	file_cbfs_fill_cache(start_of_rom + cbfs_header.offset,
+			     cbfs_header.rom_size, cbfs_header.align);
+	if (file_cbfs_result == CBFS_SUCCESS)
+		initialized = 1;
+}
+
+const struct cbfs_header *file_cbfs_get_header(void)
+{
+	if (initialized) {
+		file_cbfs_result = CBFS_SUCCESS;
+		return &cbfs_header;
+	} else {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+}
+
+cbfs_file file_cbfs_get_first(void)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	} else {
+		file_cbfs_result = CBFS_SUCCESS;
+		return file_cache;
+	}
+}
+
+void file_cbfs_get_next(cbfs_file *file)
+{
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		file = NULL;
+		return;
+	}
+
+	if (*file)
+		*file = (*file)->next;
+	file_cbfs_result = CBFS_SUCCESS;
+}
+
+cbfs_file file_cbfs_find(const char *name)
+{
+	struct cbfs_cache_node *cache_node = file_cache;
+
+	if (!initialized) {
+		file_cbfs_result = CBFS_NOT_INITIALIZED;
+		return NULL;
+	}
+
+	while (cache_node) {
+		if (!strcmp(name, cache_node->name))
+			break;
+		cache_node = cache_node->next;
+	}
+	if (!cache_node)
+		file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	else
+		file_cbfs_result = CBFS_SUCCESS;
+	return cache_node;
+}
+
+cbfs_file file_cbfs_find_uncached(uintptr_t end_of_rom, const char *name)
+{
+	u8 *start;
+	u32 size;
+	u32 align;
+	static struct cbfs_cache_node node;
+
+	if (file_cbfs_load_header(end_of_rom, &cbfs_header))
+		return NULL;
+
+	start = (u8 *)(end_of_rom + 1 - cbfs_header.rom_size);
+	size = cbfs_header.rom_size;
+	align = cbfs_header.align;
+
+	while (size >= align) {
+		int result;
+		u32 used;
+
+		result = file_cbfs_next_file(start, size, align, &node, &used);
+
+		if (result < 0)
+			return NULL;
+		else if (result == 0)
+			break;
+
+		if (!strcmp(name, node.name))
+			return &node;
+
+		size -= used;
+		start += used;
+	}
+	file_cbfs_result = CBFS_FILE_NOT_FOUND;
+	return NULL;
+}
+
+const char *file_cbfs_name(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->name;
+}
+
+u32 file_cbfs_size(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->data_length;
+}
+
+u32 file_cbfs_type(cbfs_file file)
+{
+	file_cbfs_result = CBFS_SUCCESS;
+	return file->type;
+}
+
+long file_cbfs_read(cbfs_file file, void *buffer, unsigned long maxsize)
+{
+	u32 size;
+
+	size = file->data_length;
+	if (maxsize && size > maxsize)
+		size = maxsize;
+
+	memcpy(buffer, file->data, size);
+
+	file_cbfs_result = CBFS_SUCCESS;
+	return size;
+}
diff --git a/include/cbfs.h b/include/cbfs.h
new file mode 100644
index 0000000..9e97c2d
--- /dev/null
+++ b/include/cbfs.h
@@ -0,0 +1,164 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __CBFS_H
+#define __CBFS_H
+
+#include <compiler.h>
+#include <linux/compiler.h>
+
+enum cbfs_result {
+	CBFS_SUCCESS = 0,
+	CBFS_NOT_INITIALIZED,
+	CBFS_BAD_HEADER,
+	CBFS_BAD_FILE,
+	CBFS_FILE_NOT_FOUND
+};
+
+enum cbfs_file_type {
+	CBFS_TYPE_STAGE = 0x10,
+	CBFS_TYPE_PAYLOAD = 0x20,
+	CBFS_TYPE_OPTIONROM = 0x30,
+	CBFS_TYPE_BOOTSPLASH = 0x40,
+	CBFS_TYPE_RAW = 0x50,
+	CBFS_TYPE_VSA = 0x51,
+	CBFS_TYPE_MBI = 0x52,
+	CBFS_TYPE_MICROCODE = 0x53,
+	CBFS_COMPONENT_CMOS_DEFAULT = 0xaa,
+	CBFS_COMPONENT_CMOS_LAYOUT = 0x01aa
+};
+
+struct cbfs_header {
+	u32 magic;
+	u32 version;
+	u32 rom_size;
+	u32 boot_block_size;
+	u32 align;
+	u32 offset;
+	u32 pad[2];
+} __packed;
+
+struct cbfs_cache_node;
+typedef const struct cbfs_cache_node *cbfs_file;
+
+extern enum cbfs_result file_cbfs_result;
+
+/*
+ * Return a string describing the most recent error condition.
+ *
+ * @return A pointer to the constant string.
+ */
+const char *file_cbfs_error(void);
+
+/*
+ * Initialize the CBFS driver and load metadata into RAM.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ */
+void file_cbfs_init(uintptr_t end_of_rom);
+
+/*
+ * Get the header structure for the current CBFS.
+ *
+ * @return A pointer to the constant structure, or NULL if there is none.
+ */
+const struct cbfs_header *file_cbfs_get_header(void);
+
+/*
+ * Get a handle for the first file in CBFS.
+ *
+ * @return A handle for the first file in CBFS, NULL on error.
+ */
+cbfs_file file_cbfs_get_first(void);
+
+/*
+ * Get a handle to the file after this one in CBFS.
+ *
+ * @param file		A pointer to the handle to advance.
+ */
+void file_cbfs_get_next(cbfs_file *file);
+
+/*
+ * Find a file with a particular name in CBFS.
+ *
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+cbfs_file file_cbfs_find(const char *name);
+
+
+/***************************************************************************/
+/* All of the functions below can be used without first initializing CBFS. */
+/***************************************************************************/
+
+/*
+ * Find a file with a particular name in CBFS without using the heap.
+ *
+ * @param end_of_rom	Points to the end of the ROM the CBFS should be read
+ *                      from.
+ * @param name		The name to search for.
+ *
+ * @return A handle to the file, or NULL on error.
+ */
+cbfs_file file_cbfs_find_uncached(uintptr_t end_of_rom, const char *name);
+
+/*
+ * Get the name of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The name of the file, NULL on error.
+ */
+const char *file_cbfs_name(cbfs_file file);
+
+/*
+ * Get the size of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The size of the file, zero on error.
+ */
+u32 file_cbfs_size(cbfs_file file);
+
+/*
+ * Get the type of a file in CBFS.
+ *
+ * @param file		The handle to the file.
+ *
+ * @return The type of the file, zero on error.
+ */
+u32 file_cbfs_type(cbfs_file file);
+
+/*
+ * Read a file from CBFS into RAM
+ *
+ * @param file		A handle to the file to read.
+ * @param buffer	Where to read it into memory.
+ *
+ * @return If positive or zero, the number of characters read. If negative, an
+ *         error occurred.
+ */
+long file_cbfs_read(cbfs_file file, void *buffer, unsigned long maxsize);
+
+#endif /* __CBFS_H */
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v2] Add a CBFS driver and commands to u-boot
  2011-12-06 23:32       ` Gabe Black
@ 2011-12-07  8:01         ` Wolfgang Denk
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Denk @ 2011-12-07  8:01 UTC (permalink / raw)
  To: u-boot

Dear Gabe Black,

In message <CAPPXG1nqDa_UgZsJx_g_RDCQOfJLjM=zaK5K7W9pgcsRrc9kig@mail.gmail.com> you wrote:
>
> > WARNING: do not add new typedefs
> > #853: FILE: include/cbfs.h:61:
> > +typedef const struct cbfs_cache_node *cbfs_file;
> 
> This typedef is to create an opaque handle for CBFS files. If you look here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle
> 
> In chapter 5 in the list of acceptable uses for typedefs, creating an
> opaque type is item a. This use of typedef is consistent with the Linux
> coding standards as described in that document and functionally important
> for this change and should stay.

Sorry, but you have to explain this better to me.

What would be wrong with using "struct cbfs_cache_node *" ?

And what of this exactly is "functionally important" here?

While we are quoting the CodingStyle, we should also code the next
lines:

	NOTE! Opaqueness and "accessor functions" are not good in
	themselves.

Why do you think it is needed here?

> > > +     if (file_cbfs_result != CBFS_SUCCESS) {
> > > +             printf("%s.\n", file_cbfs_error());
> >
> > Use
> >                puts(file_cbfs_error());
> > ?
> 
> That would leave out the \n. Whatever came next (like the prompt) would
> continue on the same line which would be wrong.

It appears your only use of file_cbfs_error() is in such constructs,
so just change file_cbfs_error() to include the newline.  There is
only a single call that doesn't really fit:

	+               if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
===>	+                       printf("%s: %s\n", file_cbfs_error(), argv[2]);
	+               else
	+                       printf("%s.\n", file_cbfs_error());

but that would be better written with arguments swapped anyway:

	printf("%s: %s", argv[2], file_cbfs_error());

While we are at it: for consistent style, please also omit the
trailing '.' in all such output.

Actually you can then even simplify the code:

	if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
		printf("%s: argv[2]);
	puts(file_cbfs_error());

> > > +             if (type_name)
> > > +                     printf("  %16s", type_name);
> > > +             else
> > > +                     printf("  %16d", type);
> >
> > This is probably confusing to the user.  How can he know if the file
> > type has the numerical value of "123" or if the file name is "123" ?
> 
> 
> This isn't  file name, it's the file type. No file type is named after a
> number. There are also labels on the columns indicating which is which.

Please explain the meaning of this numeric versus string value is,
then, and how the user is supposed to understand when he sees a string
and when a numeric value.

> > > +             if (filename[0])
> > > +                     printf("  %s\n", filename);
> > > +             else
> > > +                     printf("  %s\n", "(empty)");
> >
> > Use puts().
> 
> 
> The first one can't be a puts, it would have to be three. That's a lot of
> clutter to avoid a slight performance penalty when interacting with a human
> that won't notice such a tiny delay anyway. The second one could be a puts,
> but that would make the two branches of the if uneven. I'll change the
> second one.

Come on.  It this really that difficlt?  How about:

		puts("  ");
		puts(filename[0] ? filename : "(empty)");

> I put newlines there because the FAT commands do, and I have no problem
> removing them. You may want to look at the other commands and remove them
> there too.

Patches are welcome.

> Readability? I don't care that much one way or the other though. I'll
> change it.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How many hardware guys does it take to change a light bulb? "Well the
diagnostics say it's fine buddy, so it's a software problem."

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v3] Add a CBFS driver and commands to u-boot
  2011-12-06 23:36       ` [U-Boot] [PATCH v3] " Gabe Black
@ 2011-12-07  8:03         ` Wolfgang Denk
  2011-12-07  8:20           ` Gabe Black
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Denk @ 2011-12-07  8:03 UTC (permalink / raw)
  To: u-boot

Dear Gabe Black,

In message <1323214584-11635-1-git-send-email-gabeblack@chromium.org> you wrote:
> Coreboot uses a very simple "file system" called CBFS to keep track of and
> allow access to multiple "files" in a ROM image. This change adds CBFS
> support and some commands to use it to u-boot. These commands are:
> 
> cbfsinit - Initialize CBFS support and pull all metadata into RAM. The end
> of the ROM is an optional parameter which defaults to the standard
> 0xffffffff and can be used to support multiple CBFSes in a system. The last
> one set up with cbfsinit is the one that will be used.
> 
> cbfsinfo - Print information from the CBFS header.
> 
> cbfsls - Print out the size, type, and name of all the files in the current
> CBFS. Recognized types are translated into symbolic names.
> 
> cbfsload - Load a file from CBFS into memory. Like the similar command for
> fat filesystems, you can optionally provide a maximum size.
> 
> Also, if u-boot needs something out of CBFS very early before the heap is
> configured, it won't be able to use the normal CBFS support which caches
> some information in memory it allocates from the heap. This change adds a
> new cbfs_file_find_uncached function which searchs a CBFS instance without
> touching the heap.
> 
> Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
> specified.
> 
> Signed-off-by: Gabe Black <gabeblack@chromium.org>

See previous message.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
...when fits of creativity run strong, more than  one  programmer  or
writer  has  been  known to abandon the desktop for the more spacious
floor.                                             - Fred Brooks, Jr.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v3] Add a CBFS driver and commands to u-boot
  2011-12-07  8:03         ` Wolfgang Denk
@ 2011-12-07  8:20           ` Gabe Black
  2011-12-07 10:22             ` Wolfgang Denk
  0 siblings, 1 reply; 13+ messages in thread
From: Gabe Black @ 2011-12-07  8:20 UTC (permalink / raw)
  To: u-boot

On Wed, Dec 7, 2011 at 3:03 AM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Gabe Black,
>
> In message <1323214584-11635-1-git-send-email-gabeblack@chromium.org> you
> wrote:
> > Coreboot uses a very simple "file system" called CBFS to keep track of
> and
> > allow access to multiple "files" in a ROM image. This change adds CBFS
> > support and some commands to use it to u-boot. These commands are:
> >
> > cbfsinit - Initialize CBFS support and pull all metadata into RAM. The
> end
> > of the ROM is an optional parameter which defaults to the standard
> > 0xffffffff and can be used to support multiple CBFSes in a system. The
> last
> > one set up with cbfsinit is the one that will be used.
> >
> > cbfsinfo - Print information from the CBFS header.
> >
> > cbfsls - Print out the size, type, and name of all the files in the
> current
> > CBFS. Recognized types are translated into symbolic names.
> >
> > cbfsload - Load a file from CBFS into memory. Like the similar command
> for
> > fat filesystems, you can optionally provide a maximum size.
> >
> > Also, if u-boot needs something out of CBFS very early before the heap is
> > configured, it won't be able to use the normal CBFS support which caches
> > some information in memory it allocates from the heap. This change adds a
> > new cbfs_file_find_uncached function which searchs a CBFS instance
> without
> > touching the heap.
> >
> > Support for CBFS is compiled in when the CONFIG_CMD_CBFS option is
> > specified.
> >
> > Signed-off-by: Gabe Black <gabeblack@chromium.org>
>
> See previous message.
>


Could you be more specific?

Gabe

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v3] Add a CBFS driver and commands to u-boot
  2011-12-07  8:20           ` Gabe Black
@ 2011-12-07 10:22             ` Wolfgang Denk
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Denk @ 2011-12-07 10:22 UTC (permalink / raw)
  To: u-boot

Dear Gabe Black,

In message <CAPPXG1k-kGa=BnHC=qoVZaVTedEvF5zWkWx-jX0g-SrbzfFseA@mail.gmail.com> you wrote:
>
> > See previous message.
> 
> 
> Could you be more specific?

------- Forwarded Message

Dear Gabe Black,

In message <CAPPXG1nqDa_UgZsJx_g_RDCQOfJLjM=zaK5K7W9pgcsRrc9kig@mail.gmail.com> you wrote:
>
> > WARNING: do not add new typedefs
> > #853: FILE: include/cbfs.h:61:
> > +typedef const struct cbfs_cache_node *cbfs_file;
> 
> This typedef is to create an opaque handle for CBFS files. If you look here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/CodingStyle
> 
> In chapter 5 in the list of acceptable uses for typedefs, creating an
> opaque type is item a. This use of typedef is consistent with the Linux
> coding standards as described in that document and functionally important
> for this change and should stay.

Sorry, but you have to explain this better to me.

What would be wrong with using "struct cbfs_cache_node *" ?

And what of this exactly is "functionally important" here?

While we are quoting the CodingStyle, we should also code the next
lines:

	NOTE! Opaqueness and "accessor functions" are not good in
	themselves.

Why do you think it is needed here?

> > > +     if (file_cbfs_result != CBFS_SUCCESS) {
> > > +             printf("%s.\n", file_cbfs_error());
> >
> > Use
> >                puts(file_cbfs_error());
> > ?
> 
> That would leave out the \n. Whatever came next (like the prompt) would
> continue on the same line which would be wrong.

It appears your only use of file_cbfs_error() is in such constructs,
so just change file_cbfs_error() to include the newline.  There is
only a single call that doesn't really fit:

	+               if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
===>	+                       printf("%s: %s\n", file_cbfs_error(), argv[2]);
	+               else
	+                       printf("%s.\n", file_cbfs_error());

but that would be better written with arguments swapped anyway:

	printf("%s: %s", argv[2], file_cbfs_error());

While we are at it: for consistent style, please also omit the
trailing '.' in all such output.

Actually you can then even simplify the code:

	if (file_cbfs_result == CBFS_FILE_NOT_FOUND)
		printf("%s: argv[2]);
	puts(file_cbfs_error());

> > > +             if (type_name)
> > > +                     printf("  %16s", type_name);
> > > +             else
> > > +                     printf("  %16d", type);
> >
> > This is probably confusing to the user.  How can he know if the file
> > type has the numerical value of "123" or if the file name is "123" ?
> 
> 
> This isn't  file name, it's the file type. No file type is named after a
> number. There are also labels on the columns indicating which is which.

Please explain the meaning of this numeric versus string value is,
then, and how the user is supposed to understand when he sees a string
and when a numeric value.

> > > +             if (filename[0])
> > > +                     printf("  %s\n", filename);
> > > +             else
> > > +                     printf("  %s\n", "(empty)");
> >
> > Use puts().
> 
> 
> The first one can't be a puts, it would have to be three. That's a lot of
> clutter to avoid a slight performance penalty when interacting with a human
> that won't notice such a tiny delay anyway. The second one could be a puts,
> but that would make the two branches of the if uneven. I'll change the
> second one.

Come on.  It this really that difficlt?  How about:

		puts("  ");
		puts(filename[0] ? filename : "(empty)");

> I put newlines there because the FAT commands do, and I have no problem
> removing them. You may want to look at the other commands and remove them
> there too.

Patches are welcome.

> Readability? I don't care that much one way or the other though. I'll
> change it.

Thanks.

------- End of Forwarded Message


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I haven't lost my mind -- it's backed up on tape somewhere.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot
  2011-12-03 11:47 [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot Gabe Black
  2011-12-05 22:25 ` Wolfgang Denk
@ 2012-01-08  4:52 ` Mike Frysinger
  1 sibling, 0 replies; 13+ messages in thread
From: Mike Frysinger @ 2012-01-08  4:52 UTC (permalink / raw)
  To: u-boot

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>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-01-08  4:52 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-03 11:47 [U-Boot] [PATCH] Add a CBFS driver and commands to u-boot Gabe Black
2011-12-05 22:25 ` Wolfgang Denk
2011-12-05 22:35   ` Gabe Black
2011-12-05 23:46     ` Gabe Black
2011-12-06  1:25   ` [U-Boot] [PATCH v2] " Gabe Black
2011-12-06 11:01     ` Wolfgang Denk
2011-12-06 23:32       ` Gabe Black
2011-12-07  8:01         ` Wolfgang Denk
2011-12-06 23:36       ` [U-Boot] [PATCH v3] " Gabe Black
2011-12-07  8:03         ` Wolfgang Denk
2011-12-07  8:20           ` Gabe Black
2011-12-07 10:22             ` Wolfgang Denk
2012-01-08  4:52 ` [U-Boot] [PATCH] " Mike Frysinger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.