On Rab, 2017-12-06 at 12:00 +0100, Lothar Waßmann wrote:
Hi,
On Wed, 6 Dec 2017 10:06:21 +0000 Chee, Tien Fong wrote:
On Sel, 2017-12-05 at 09:53 +0100, Lothar Waßmann wrote:
Hi,
On Tue, 5 Dec 2017 15:57:57 +0800 tien.fong.chee@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
common/Makefile | 1 + common/fs_loader.c | 304 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs_loader.h | 30 ++++++ 3 files changed, 335 insertions(+) create mode 100644 common/fs_loader.c create mode 100644 include/fs_loader.h
diff --git a/common/Makefile b/common/Makefile index 801ea31..419e915 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 diff --git a/common/fs_loader.c b/common/fs_loader.c new file mode 100644 index 0000000..04f682b --- /dev/null +++ b/common/fs_loader.c @@ -0,0 +1,304 @@ +/*
- 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> +#include <linux/string.h> +#include <usb.h>
+static struct device_location default_locations[] = {
- {
.name = "mmc",.devpart = "0:1",- },
- {
.name = "usb",.devpart = "0:1",- },
- {
.name = "sata",.devpart = "0:1",- },
+};
+/* USB build is not supported yet in SPL */ +#ifndef CONFIG_SPL_BUILD +#ifdef CONFIG_USB_STORAGE +static int init_usb(void) +{
- int err = 0;
Useless initialization.
noted.
- err = usb_init();
- if (err)
Unnecessary blank line.
Sorry, i'm not catching you because there is no blank line between "if" and "return"
I meant the blank line between 'err = ...' and 'if (err)'
Okay, i can change that.
if (!strcmp(default_locations[i].name, name))default_locations[i].devpart =devpart;
- }
- return;
+}
+/*
- Prepare firmware struct;
- return -ve if fail.
- */
+static int _request_firmware_prepare(struct firmware **firmware_p,
const char *name, void*dbuf,
size_t size, u32 offset)+{
- struct firmware *firmware = NULL;
- int ret = 0;
- if (!name || name[0] == '\0')
ret = -EINVAL;Is it really useful to continue here initializing the 'firmware' struct and returning an error at the end?
I try to keep it very close to Linux firmware loader. When more API ported in from Linux in future, this can be helper function. Anyway, i have no strong opinion, i can move to caller if you guys think that is better.
The Linux firmware loader has this:
if (!name || name[0] == '\0') { ret = -EINVAL; goto out; }
Note the 'goto out' which is missing in your code. If following the Linux code closely, you would have to set *firmware_p to NULL and exit in this case.
I can set the *firmware_p to NUll in failing case. But, i checked the static int _request_firmware_prepare in Linux, there is no goto out error handling method in the function. Fyi, there is no allocated memory release in U-Boot. https://elixir.free-electrons.com/linux/v4.13.15/source/drivers/base/fi rmware_class.c#L1191
Lothar Waßmann