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

[Chee, Tien Fong]

On Sel, 2017-12-12 at 15:12 +0100, Lothar Waßmann wrote:
> 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.
> 
Okay, i will move this declaration to source file. Any comments to the
source file?
> > 
> > +
> > +struct firmware {
> > +	size_t size;		/* Size of a file */
> > +	const u8 *data;		/* Buffer for file */
> > +	void *priv;		/* Firmware loader private
> > fields */
> > +};
> > +
> 
> Lothar Waßmann