diff mbox series

[07/13] cbfs: Unify the two header loaders

Message ID 20200513142359.147589-8-sjg@chromium.org
State Superseded
Headers show
Series x86: cbfs: Various clean-ups to CBFS implementation | expand

Commit Message

Simon Glass May 13, 2020, 2:23 p.m. UTC
These two functions have mostly the same code. Pull this out into a common
function.

Also make this function zero the private data so that callers don't have
to do it. Finally, update cbfs_load_header_ptr() to take the base of the
ROM as its parameter, which makes more sense than passing the address of
the header within the ROM.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

 fs/cbfs/cbfs.c | 59 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 22 deletions(-)

Comments

Bin Meng May 21, 2020, 2:27 a.m. UTC | #1
Hi Simon,

On Wed, May 13, 2020 at 10:24 PM Simon Glass <sjg at chromium.org> wrote:
>
> These two functions have mostly the same code. Pull this out into a common
> function.
>
> Also make this function zero the private data so that callers don't have
> to do it. Finally, update cbfs_load_header_ptr() to take the base of the
> ROM as its parameter, which makes more sense than passing the address of
> the header within the ROM.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  fs/cbfs/cbfs.c | 59 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
> index 05de58cf19..b4e6b959d1 100644
> --- a/fs/cbfs/cbfs.c
> +++ b/fs/cbfs/cbfs.c
> @@ -177,47 +177,63 @@ static int file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size,
>         return 0;
>  }
>
> -/* Get the CBFS header out of the ROM and do endian conversion. */
> -static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom)
> +/**
> + * load_header() - Load the CBFS header
> + *
> + * Get the CBFS header out of the ROM and do endian conversion.
> + *
> + * @priv: Private data, which is inited by this function
> + * @addr: Address of CBFS header in memory-mapped SPI flash
> + * @return 0 if OK, -ENXIO if the header is bad
> + */
> +static int load_header(struct cbfs_priv *priv, ulong addr)
>  {
>         struct cbfs_header *header = &priv->header;
>         struct cbfs_header *header_in_rom;
> -       int32_t offset = *(u32 *)(end_of_rom - 3);
>
> -       header_in_rom = (struct cbfs_header *)(end_of_rom + offset + 1);
> +       memset(priv, '\0', sizeof(*priv));
> +       header_in_rom = (struct cbfs_header *)addr;
>         swap_header(header, header_in_rom);
>
>         if (header->magic != good_magic || header->offset >
>                         header->rom_size - header->boot_block_size) {
>                 priv->result = CBFS_BAD_HEADER;
> -               return 1;
> +               return -ENXIO;
>         }
> +
>         return 0;
>  }
>
> -static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base)
> +/**
> + * file_cbfs_load_header() - Get the CBFS header out of the ROM, given the end
> + *
> + * @priv: Private data, which is inited by this function
> + * @addr: Address of the last byte of the ROM (typically 0xffffffff)

This should be end_of_rom

> + * @return 0 if OK, -ENXIO if the header is bad
> + */
> +static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom)
>  {
> -       struct cbfs_header *header = &priv->header;
> -       struct cbfs_header *header_in_rom;
> -
> -       header_in_rom = (struct cbfs_header *)base;
> -       swap_header(header, header_in_rom);
> +       int offset = *(u32 *)(end_of_rom - 3);
>
> -       if (header->magic != good_magic || header->offset >
> -                       header->rom_size - header->boot_block_size) {
> -               priv->result = CBFS_BAD_HEADER;
> -               return -EFAULT;
> -       }
> +       return load_header(priv, end_of_rom + offset + 1);
> +}
>
> -       return 0;
> +/**
> + * cbfs_load_header_ptr() - Get the CBFS header out of the ROM, given the base
> + *
> + * @priv: Private data, which is inited by this function
> + * @addr: Address of the first byte of the ROM (e.g. 0xff000000)

This should be base

> + * @return 0 if OK, -ENXIO if the header is bad
> + */
> +static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base)
> +{
> +       return load_header(priv, base + MASTER_HDR_OFFSET);
>  }
>
>  static void cbfs_init(struct cbfs_priv *priv, ulong end_of_rom)
>  {
>         u8 *start_of_rom;
>
> -       priv->initialised = false;
> -
>         if (file_cbfs_load_header(priv, end_of_rom))
>                 return;
>
> @@ -241,10 +257,9 @@ int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp)
>
>         /*
>          * Use a local variable to start with until we know that the CBFS is
> -        * valid. Assume that a master header appears at the start, at offset
> -        * 0x38.
> +        * valid.
>          */
> -       ret = cbfs_load_header_ptr(priv, base + MASTER_HDR_OFFSET);
> +       ret = cbfs_load_header_ptr(priv, base);
>         if (ret)
>                 return ret;
>

Regards,
Bin
diff mbox series

Patch

diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c
index 05de58cf19..b4e6b959d1 100644
--- a/fs/cbfs/cbfs.c
+++ b/fs/cbfs/cbfs.c
@@ -177,47 +177,63 @@  static int file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size,
 	return 0;
 }
 
-/* Get the CBFS header out of the ROM and do endian conversion. */
-static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom)
+/**
+ * load_header() - Load the CBFS header
+ *
+ * Get the CBFS header out of the ROM and do endian conversion.
+ *
+ * @priv: Private data, which is inited by this function
+ * @addr: Address of CBFS header in memory-mapped SPI flash
+ * @return 0 if OK, -ENXIO if the header is bad
+ */
+static int load_header(struct cbfs_priv *priv, ulong addr)
 {
 	struct cbfs_header *header = &priv->header;
 	struct cbfs_header *header_in_rom;
-	int32_t offset = *(u32 *)(end_of_rom - 3);
 
-	header_in_rom = (struct cbfs_header *)(end_of_rom + offset + 1);
+	memset(priv, '\0', sizeof(*priv));
+	header_in_rom = (struct cbfs_header *)addr;
 	swap_header(header, header_in_rom);
 
 	if (header->magic != good_magic || header->offset >
 			header->rom_size - header->boot_block_size) {
 		priv->result = CBFS_BAD_HEADER;
-		return 1;
+		return -ENXIO;
 	}
+
 	return 0;
 }
 
-static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base)
+/**
+ * file_cbfs_load_header() - Get the CBFS header out of the ROM, given the end
+ *
+ * @priv: Private data, which is inited by this function
+ * @addr: Address of the last byte of the ROM (typically 0xffffffff)
+ * @return 0 if OK, -ENXIO if the header is bad
+ */
+static int file_cbfs_load_header(struct cbfs_priv *priv, ulong end_of_rom)
 {
-	struct cbfs_header *header = &priv->header;
-	struct cbfs_header *header_in_rom;
-
-	header_in_rom = (struct cbfs_header *)base;
-	swap_header(header, header_in_rom);
+	int offset = *(u32 *)(end_of_rom - 3);
 
-	if (header->magic != good_magic || header->offset >
-			header->rom_size - header->boot_block_size) {
-		priv->result = CBFS_BAD_HEADER;
-		return -EFAULT;
-	}
+	return load_header(priv, end_of_rom + offset + 1);
+}
 
-	return 0;
+/**
+ * cbfs_load_header_ptr() - Get the CBFS header out of the ROM, given the base
+ *
+ * @priv: Private data, which is inited by this function
+ * @addr: Address of the first byte of the ROM (e.g. 0xff000000)
+ * @return 0 if OK, -ENXIO if the header is bad
+ */
+static int cbfs_load_header_ptr(struct cbfs_priv *priv, ulong base)
+{
+	return load_header(priv, base + MASTER_HDR_OFFSET);
 }
 
 static void cbfs_init(struct cbfs_priv *priv, ulong end_of_rom)
 {
 	u8 *start_of_rom;
 
-	priv->initialised = false;
-
 	if (file_cbfs_load_header(priv, end_of_rom))
 		return;
 
@@ -241,10 +257,9 @@  int cbfs_init_mem(ulong base, ulong size, struct cbfs_priv **privp)
 
 	/*
 	 * Use a local variable to start with until we know that the CBFS is
-	 * valid. Assume that a master header appears at the start, at offset
-	 * 0x38.
+	 * valid.
 	 */
-	ret = cbfs_load_header_ptr(priv, base + MASTER_HDR_OFFSET);
+	ret = cbfs_load_header_ptr(priv, base);
 	if (ret)
 		return ret;