[U-Boot] [PATCH v3 2/2] common: Generic firmware loader for file system

[Lothar Waßmann]

Hi,

On Tue, 12 Dec 2017 19:56:17 +0800 tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee at intel.com>
> 
> This is file system generic loader which can be used to load
> the file image from the storage into target such as memory.
> The consumer driver would then use this loader to program whatever,
> ie. the FPGA device.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> ---
>  common/Makefile     |   1 +
>  common/fs_loader.c  | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fs_loader.h |  33 ++++++
>  3 files changed, 339 insertions(+)
>  create mode 100644 common/fs_loader.c
>  create mode 100644 include/fs_loader.h
> 
[...]
> diff --git a/include/fs_loader.h b/include/fs_loader.h
> new file mode 100644
> index 0000000..35a6b5b
> --- /dev/null
> +++ b/include/fs_loader.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2017 Intel Corporation <www.intel.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0
> + */
> +#ifndef _FS_LOADER_H_
> +#define _FS_LOADER_H_
> +
> +#include <linux/types.h>
> +
> +struct firmware_priv {
> +	const char *name;	/* Filename */
> +	u32 offset;		/* Offset of reading a file */
> +};
>
This is private to the firmware loader and should be declared locally
in the source file. That's the whole point of being 'private'.
You could have different firmware loaders with different requirements
for their private data, thus there is no use in defining this globally.

> +
> +struct firmware {
> +	size_t size;		/* Size of a file */
> +	const u8 *data;		/* Buffer for file */
> +	void *priv;		/* Firmware loader private fields */
> +};
> +


Lothar Waßmann