From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chee, Tien Fong <tien.fong.chee@intel.com>
Date: Tue, 16 Jan 2018 07:58:00 +0000
Subject: [U-Boot] [U-Boot, v6,
	2/2] common: Generic firmware loader for file system
In-Reply-To: <20180115163603.GO4660@bill-the-cat>
References: <1514351078-3487-3-git-send-email-tien.fong.chee@intel.com>
 <20180115163603.GO4660@bill-the-cat>
Message-ID: <1516089479.32057.4.camel@intel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

On Mon, 2018-01-15 at 11:36 -0500, Tom Rini wrote:
> On Wed, Dec 27, 2017 at 01:04:38PM +0800, tien.fong.chee at intel.com
> wrote:
> 
> > 
> > From: Tien Fong Chee <tien.fong.chee@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@intel.com>
> Please add Lothar's Reviewed-by for v7.  There's a number of minor
> checkpatch.pl issues that checkpatch.pl can in turn fixup itself,
> please
> correct them.
> 
I have ran the checkpatch.pl on this patch, i didn't see any error.

> [snip]
> > 
> > diff --git a/common/Makefile b/common/Makefile
> > index cec506f..2934221 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -130,3 +130,4 @@ obj-$(CONFIG_CMD_DFU) += dfu.o
> >  obj-y += command.o
> >  obj-y += s_record.o
> >  obj-y += xyzModem.o
> > +obj-y += fs_loader.o
> This needs a new Kconfig option and not to be enabled globally, only
> when needed.
> 
Okay.
> > 
> > diff --git a/common/fs_loader.c b/common/fs_loader.c
> > new file mode 100644
> > index 0000000..56d29b6
> > --- /dev/null
> > +++ b/common/fs_loader.c
> > @@ -0,0 +1,309 @@
> > +/*
> > + * Copyright (C) 2017 Intel Corporation <www.intel.com>
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0
> > + */
> > +
> > +#include <common.h>
> > +#include <errno.h>
> > +#include <fs.h>
> > +#include <fs_loader.h>
> > +#include <nand.h>
> > +#include <sata.h>
> > +#include <spi.h>
> > +#include <spi_flash.h>
> > +#include <spl.h>
> This wants <asm/spl.h> which is not globally available, so you need
> to
> come up with something here.  At least making this Kconfig-enabled
> will
> be a start and perhaps OK for now.
> 
I can enable the Kconfig, and put the caution about dependency on
<asm/spl.h> in document.
> [snip]
> > 
> > +	if (ret) {
> > +		printf("Error: %d Failed to read %s from flash
> > %lld != %d.\n",
> > +		      ret, fw_priv->name, actread, firmware_p-
> > >size);
> The last %d needs to be %zu since it's a size_t, for portability.
> 
> Thanks!
>