At present the result variable in the cbfs_priv is called 'result' as is the local variable in a few functions. Change the latter to 'ret' which is more common in U-Boot and avoids confusion.

Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: None

fs/cbfs/cbfs.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 1aa6f8ee84..70440aa80b 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -145,18 +145,18 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, priv->file_cache = NULL;

while (size >= align) { - int result; + int ret; u32 used;

new_node = (struct cbfs_cachenode *) malloc(sizeof(struct cbfs_cachenode)); - result = file_cbfs_next_file(priv, start, size, align, new_node, - &used); + ret = file_cbfs_next_file(priv, start, size, align, new_node, + &used);

- if (result < 0) { + if (ret < 0) { free(new_node); return; - } else if (result == 0) { + } else if (ret == 0) { free(new_node); break; } @@ -341,15 +341,15 @@ const struct cbfs_cachenode *file_cbfs_find_uncached(uintptr_t end_of_rom, align = priv->header.align;

while (size >= align) { - int result; + int ret; u32 used;

- result = file_cbfs_next_file(priv, start, size, align, &node, - &used); + ret = file_cbfs_next_file(priv, start, size, align, &node, + &used);

- if (result < 0) + if (ret < 0) return NULL; - else if (result == 0) + else if (ret == 0) break;

if (!strcmp(name, node.name))

At present this uses an int type. U-Boot now supports bool so use this instead.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: - Stick with US spelling

Changes in v3: None Changes in v2: None

fs/cbfs/cbfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 846102dce3..91d7af0493 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -12,7 +12,7 @@ static const u32 good_magic = 0x4f524243; static const u8 good_file_magic[] = "LARCHIVE";

struct cbfs_priv { - int initialized; + bool initialized; struct cbfs_header header; struct cbfs_cachenode *file_cache; enum cbfs_result result; @@ -207,7 +207,7 @@ static void cbfs_init(struct cbfs_priv *priv, ulong end_of_rom) { u8 *start_of_rom;

- priv->initialized = 0; + priv->initialized = false;

if (file_cbfs_load_header(end_of_rom, &priv->header)) return; @@ -217,7 +217,7 @@ static void cbfs_init(struct cbfs_priv *priv, ulong end_of_rom) file_cbfs_fill_cache(priv, start_of_rom, priv->header.rom_size, priv->header.align); if (priv->result == CBFS_SUCCESS) - priv->initialized = 1; + priv->initialized = true; }

void file_cbfs_init(ulong end_of_rom) @@ -244,7 +244,7 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp) if (priv->result != CBFS_SUCCESS) return -EINVAL;

- priv->initialized = 1; + priv->initialized = true; priv = malloc(sizeof(priv_s)); if (!priv) return -ENOMEM;

At present this uses a true return to indicate it found a file. Adjust it to use 0 for this, so it is consistent with other functions.

Update its callers accordingly and add a check for malloc() failure in file_cbfs_fill_cache().

Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: - Fix 'his' typo

Changes in v3: - Fix typo in commit message

Changes in v2: None

fs/cbfs/cbfs.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 91d7af0493..c17f6d6250 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -77,11 +77,12 @@ static void swap_file_header(struct cbfs_fileheader *dest, * @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. + * @return 0 if a file is found, -ENOENT if one isn't, -EBADF if a bad header + * is found. */ -static int file_cbfs_next_file(struct cbfs_priv *priv, u8 *start, u32 size, - u32 align, struct cbfs_cachenode *new_node, - u32 *used) +static int file_cbfs_next_file(struct cbfs_priv *priv, u8 *start, int size, + int align, struct cbfs_cachenode *new_node, + int *used) { struct cbfs_fileheader header;

@@ -105,7 +106,7 @@ static int file_cbfs_next_file(struct cbfs_priv *priv, u8 *start, u32 size, swap_file_header(&header, file_header); if (header.offset < sizeof(struct cbfs_fileheader)) { priv->result = CBFS_BAD_FILE; - return -1; + return -EBADF; } new_node->next = NULL; new_node->type = header.type; @@ -122,14 +123,15 @@ static int file_cbfs_next_file(struct cbfs_priv *priv, u8 *start, u32 size, step = step + align - step % align;

*used += step; - return 1; + return 0; } - return 0; + + return -ENOENT; }

/* Look through a CBFS instance and copy file metadata into regular memory. */ -static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, - u32 align) +static int file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, + u32 align) { struct cbfs_cachenode *cache_node; struct cbfs_cachenode *new_node; @@ -145,20 +147,21 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, priv->file_cache = NULL;

while (size >= align) { + int used; int ret; - u32 used;

new_node = (struct cbfs_cachenode *) malloc(sizeof(struct cbfs_cachenode)); + if (!new_node) + return -ENOMEM; ret = file_cbfs_next_file(priv, start, size, align, new_node, &used);

if (ret < 0) { free(new_node); - return; - } else if (ret == 0) { - free(new_node); - break; + if (ret == -ENOENT) + break; + return ret; } *cache_tail = new_node; cache_tail = &new_node->next; @@ -167,6 +170,8 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, start += used; } priv->result = CBFS_SUCCESS; + + return 0; }

/* Get the CBFS header out of the ROM and do endian conversion. */ @@ -341,16 +346,14 @@ const struct cbfs_cachenode *file_cbfs_find_uncached(ulong end_of_rom,

while (size >= align) { int ret; - u32 used; + int used;

ret = file_cbfs_next_file(priv, start, size, align, &node, &used); - - if (ret < 0) - return NULL; - else if (ret == 0) + if (ret == -ENOENT) break; - + else if (ret) + return NULL; if (!strcmp(name, node.name)) return &node;

This function is strange at the moment in that it takes a header pointer but then accesses the cbfs_s global. Currently clients have their own priv pointer, so update the function to take that as a parameter instead.

Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: None

fs/cbfs/cbfs.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index c08fcb14a5..f3fcdab1a7 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -8,6 +8,9 @@ #include <malloc.h> #include <asm/byteorder.h>

+/* Offset of master header from the start of a coreboot ROM */ +#define MASTER_HDR_OFFSET 0x38 + static const u32 good_magic = 0x4f524243; static const u8 good_file_magic[] = "LARCHIVE";

@@ -192,9 +195,9 @@ static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom) return 0; }

-static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base, - struct cbfs_header *header) +static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base) { + struct cbfs_header *header = &priv->header; struct cbfs_header *header_in_rom;

header_in_rom = (struct cbfs_header *)base; @@ -241,7 +244,7 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp) * valid. Assume that a master header appears at the start, at offset * 0x38. */ - ret = cbfs_load_header_ptr(priv, base + 0x38, &priv->header); + ret = cbfs_load_header_ptr(priv, base + MASTER_HDR_OFFSET); if (ret) return ret;

The start address of the CBFS is used when scanning for files. It makes sense to put this in our cbfs_priv struct and calculate it when we read the header.

Update the code accordingly.

Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: None

fs/cbfs/cbfs.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 40198ae7e7..dc9789fcb8 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -14,8 +14,18 @@ static const u32 good_magic = 0x4f524243; static const u8 good_file_magic[] = "LARCHIVE";

+/** + * struct cbfs_priv - Private data for this driver + * + * @initialised: true if this CBFS has been inited + * @start: Start position of CBFS in memory, typically memory-mapped SPI flash + * @header: Header read from the CBFS, byte-swapped so U-Boot can access it + * @file_cache: List of file headers read from CBFS + * @result: Success/error result + */ struct cbfs_priv { bool initialized; + void *start; struct cbfs_header header; struct cbfs_cachenode *file_cache; enum cbfs_result result; @@ -132,12 +142,12 @@ static int file_cbfs_next_file(struct cbfs_priv *priv, void *start, int size, }

/* Look through a CBFS instance and copy file metadata into regular memory. */ -static int file_cbfs_fill_cache(struct cbfs_priv *priv, void *start, u32 size, - u32 align) +static int file_cbfs_fill_cache(struct cbfs_priv *priv, int size, int align) { struct cbfs_cachenode *cache_node; struct cbfs_cachenode *new_node; struct cbfs_cachenode **cache_tail = &priv->file_cache; + void *start;

/* Clear out old information. */ cache_node = priv->file_cache; @@ -148,6 +158,7 @@ static int file_cbfs_fill_cache(struct cbfs_priv *priv, void *start, u32 size, } priv->file_cache = NULL;

+ start = priv->start; while (size >= align) { int used; int ret; @@ -213,8 +224,14 @@ static int load_header(struct cbfs_priv *priv, ulong addr) static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom) { int offset = *(u32 *)(end_of_rom - 3); + int ret; + + ret = load_header(priv, end_of_rom + offset + 1); + if (ret) + return ret; + priv->start = (void *)(end_of_rom + 1 - priv->header.rom_size);

- return load_header(priv, end_of_rom + offset + 1); + return 0; }

/** @@ -226,20 +243,22 @@ static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom) */ static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base) { - return load_header(priv, base + MASTER_HDR_OFFSET); + int ret; + + ret = load_header(priv, base + MASTER_HDR_OFFSET); + if (ret) + return ret; + priv->start = (void *)base; + + return 0; }

static void cbfs_init(struct cbfs_priv *priv, ulong end_of_rom) { - void *start_of_rom; - if (file_cbfs_load_header(priv, end_of_rom)) return;

- start_of_rom = (void *)(end_of_rom + 1 - priv->header.rom_size); - - file_cbfs_fill_cache(priv, start_of_rom, priv->header.rom_size, - priv->header.align); + file_cbfs_fill_cache(priv, priv->header.rom_size, priv->header.align); if (priv->result == CBFS_SUCCESS) priv->initialized = true; } @@ -262,8 +281,7 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp) if (ret) return ret;

- file_cbfs_fill_cache(priv, (void *)base, priv->header.rom_size, - priv->header.align); + file_cbfs_fill_cache(priv, priv->header.rom_size, priv->header.align); if (priv->result != CBFS_SUCCESS) return -EINVAL;

@@ -358,7 +376,7 @@ const struct cbfs_cachenode *file_cbfs_find_uncached(ulong end_of_rom, if (file_cbfs_load_header(priv, end_of_rom)) return NULL;

- start = (void *)(end_of_rom + 1 - priv->header.rom_size); + start = priv->start; size = priv->header.rom_size; align = priv->header.align;

This function currently returns a node pointer so there is no way to know the error code. Also it uses data in BSS which seems unnecessary since the caller might prefer to use a local variable.

Update the function and split its body out into a separate function so we can use it later.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: - Fix s/u8/void/ in find_uncached() - Fix setting of start in file_cbfs_find_uncached() - Add more comments on @node in file_cbfs_find_uncached()

fs/cbfs/cbfs.c | 48 +++++++++++++++++++++++++++--------------------- include/cbfs.h | 17 +++++++++-------- 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 73fe3b3624..ba903d16a0 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -371,40 +371,46 @@ const struct cbfs_cachenode *file_cbfs_find(const char *name) return cbfs_find_file(&cbfs_s, name); }

-const struct cbfs_cachenode *file_cbfs_find_uncached(ulong end_of_rom, - const char *name) +static int find_uncached(struct cbfs_priv *priv, const char *name, void *start, + struct cbfs_cachenode *node) { - struct cbfs_priv *priv = &cbfs_s; - void *start; - u32 size; - u32 align; - static struct cbfs_cachenode node; - - if (file_cbfs_load_header(priv, end_of_rom)) - return NULL; - - start = priv->start; - size = priv->header.rom_size; - align = priv->header.align; + int size = priv->header.rom_size; + int align = priv->header.align;

while (size >= align) { - int ret; int used; + int ret;

- ret = file_cbfs_next_file(priv, start, size, align, &node, + ret = file_cbfs_next_file(priv, start, size, align, node, &used); if (ret == -ENOENT) break; else if (ret) - return NULL; - if (!strcmp(name, node.name)) - return &node; + return ret; + if (!strcmp(name, node->name)) + return 0;

size -= used; start += used; } - cbfs_s.result = CBFS_FILE_NOT_FOUND; - return NULL; + priv->result = CBFS_FILE_NOT_FOUND; + + return -ENOENT; +} + +int file_cbfs_find_uncached(ulong end_of_rom, const char *name, + struct cbfs_cachenode *node) +{ + struct cbfs_priv priv; + void *start; + int ret; + + ret = file_cbfs_load_header(&priv, end_of_rom); + if (ret) + return ret; + start = priv.start; + + return find_uncached(&priv, name, start, node); }

const char *file_cbfs_name(const struct cbfs_cachenode *file) diff --git a/include/cbfs.h b/include/cbfs.h index 962b3e848b..5a248781c3 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -161,17 +161,18 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp); /***************************************************************************/

/** - * file_cbfs_find_uncached() - Find a file with a particular name in CBFS - * without using the heap. + * file_cbfs_find_uncached() - Find a file in CBFS given the end of the ROM * - * @end_of_rom: Points to the end of the ROM the CBFS should be read - * from. - * @name: The name to search for. + * Note that @node should be declared by the caller. This design is to avoid + * the need for allocation here. * - * @return A handle to the file, or NULL on error. + * @end_of_rom: Points to the end of the ROM the CBFS should be read from + * @name: The name to search for + * @node: Returns the contents of the node if found (i.e. copied into *node) + * @return 0 on success, -ENOENT if not found, -EFAULT on bad header */ -const struct cbfs_cachenode *file_cbfs_find_uncached(ulong end_of_rom, - const char *name); +int file_cbfs_find_uncached(ulong end_of_rom, const char *name, + struct cbfs_cachenode *node);

/** * file_cbfs_name() - Get the name of a file in CBFS.

Currently we support reading a file from CBFS given the address of the end of the ROM. Sometimes we only know the start of the CBFS. Add a function to find a file given that.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: - Use void * instead of u8 * in file_cbfs_find_uncached_base() - Fix and expand comments in file_cbfs_find_uncached_base()

fs/cbfs/cbfs.c | 13 +++++++++++++ include/cbfs.h | 14 ++++++++++++++ 2 files changed, 27 insertions(+)

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index ba903d16a0..d3c3722c48 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -413,6 +413,19 @@ int file_cbfs_find_uncached(ulong end_of_rom, const char *name, return find_uncached(&priv, name, start, node); }

+int file_cbfs_find_uncached_base(ulong base, const char *name, + struct cbfs_cachenode *node) +{ + struct cbfs_priv priv; + int ret; + + ret = cbfs_load_header_ptr(&priv, base); + if (ret) + return ret; + + return find_uncached(&priv, name, (void *)base, node); +} + const char *file_cbfs_name(const struct cbfs_cachenode *file) { cbfs_s.result = CBFS_SUCCESS; diff --git a/include/cbfs.h b/include/cbfs.h index 5a248781c3..1aff110acb 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -174,6 +174,20 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp); int file_cbfs_find_uncached(ulong end_of_rom, const char *name, struct cbfs_cachenode *node);

+/** + * file_cbfs_find_uncached_base() - Find a file in CBFS given the base address + * + * Note that @node should be declared by the caller. This design is to avoid + * the need for allocation here. + * + * @base: Points to the base of the CBFS + * @name: The name to search for + * @node: Returns the contents of the node if found (i.e. copied into *node) + * @return 0 on success, -ENOENT if not found, -EFAULT on bad header + */ +int file_cbfs_find_uncached_base(ulong base, const char *name, + struct cbfs_cachenode *node); + /** * file_cbfs_name() - Get the name of a file in CBFS. *

The size is not actually used since it is present in the header. Drop this parameter. Also tidy up error handling while we are here.

Reviewed-by: Bin Meng bmeng.cn@gmail.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v4: None Changes in v3: None Changes in v2: - Rebase to master (with x86/master cherry-picked in too) - Resending as some comments / reviews were missed

arch/x86/lib/fsp2/fsp_init.c | 3 +-- fs/cbfs/cbfs.c | 10 ++++++---- include/cbfs.h | 3 +-- 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/lib/fsp2/fsp_init.c b/arch/x86/lib/fsp2/fsp_init.c index 8c577902b2..85cae54a0c 100644 --- a/arch/x86/lib/fsp2/fsp_init.c +++ b/arch/x86/lib/fsp2/fsp_init.c @@ -81,11 +81,10 @@ static int get_cbfs_fsp(enum fsp_type_t type, ulong map_base, * 'COREBOOT' (CBFS, size 1814528, offset 2117632). */ ulong cbfs_base = 0x205000; - ulong cbfs_size = 0x1bb000; struct cbfs_priv *cbfs; int ret;

- ret = cbfs_init_mem(map_base + cbfs_base, cbfs_size, &cbfs); + ret = cbfs_init_mem(map_base + cbfs_base, &cbfs); if (ret) return ret; if (!ret) { diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index d3c3722c48..9007aa7d15 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -5,6 +5,7 @@

#include <common.h> #include <cbfs.h> +#include <log.h> #include <malloc.h> #include <asm/byteorder.h>

@@ -275,7 +276,7 @@ int file_cbfs_init(ulong end_of_rom) return cbfs_init(&cbfs_s, end_of_rom); }

-int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp) +int cbfs_init_mem(ulong base, struct cbfs_priv **privp) { struct cbfs_priv priv_s, *priv = &priv_s; int ret; @@ -288,9 +289,10 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp) if (ret) return ret;

- file_cbfs_fill_cache(priv, priv->header.rom_size, priv->header.align); - if (priv->result != CBFS_SUCCESS) - return -EINVAL; + ret = file_cbfs_fill_cache(priv, priv->header.rom_size, + priv->header.align); + if (ret) + return log_msg_ret("fill", ret);

priv->initialized = true; priv = malloc(sizeof(priv_s)); diff --git a/include/cbfs.h b/include/cbfs.h index 1aff110acb..5f296d6a37 100644 --- a/include/cbfs.h +++ b/include/cbfs.h @@ -149,11 +149,10 @@ const struct cbfs_cachenode *cbfs_find_file(struct cbfs_priv *cbfs, * cbfs_init_mem() - Set up a new CBFS * * @base: Base address of CBFS - * @size: Size of CBFS in bytes * @cbfsp: Returns a pointer to CBFS on success * @return 0 if OK, -ve on error */ -int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp); +int cbfs_init_mem(ulong base, struct cbfs_priv **privp);

/***************************************************************************/