[U-Boot] [PATCH v9 4/4] common: Generic firmware loader for file system
Lothar Waßmann
LW at KARO-electronics.de
Tue Feb 27 10:04:27 UTC 2018
Hi,
On Tue, 27 Feb 2018 13:21:57 +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>
> Reviewed-by: Lothar Waßmann <LW at KARO-electronics.de>
> ---
> common/Kconfig | 10 ++
> common/Makefile | 1 +
> common/fs_loader.c | 365 +++++++++++++++++++++++++++++++++++++++++++++
> doc/README.firmware_loader | 83 +++++++++++
> include/fs_loader.h | 28 ++++
> 5 files changed, 487 insertions(+)
> create mode 100644 common/fs_loader.c
> create mode 100644 doc/README.firmware_loader
> create mode 100644 include/fs_loader.h
>
> diff --git a/common/Kconfig b/common/Kconfig
> index d5bc37e..ff88c6e 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -560,6 +560,16 @@ config DISPLAY_BOARDINFO
> when U-Boot starts up. The board function checkboard() is called
> to do this.
>
> +config GENERIC_FIRMWARE_LOADER
> + bool "Enable generic firmware loader driver for file system"
> + depends on SPL
> + help
> + 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.
> +
> menu "Start-up hooks"
>
> config ARCH_EARLY_INIT_R
> diff --git a/common/Makefile b/common/Makefile
> index c7bde23..92d8fb3 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -134,3 +134,4 @@ obj-$(CONFIG_$(SPL_)LOG) += log.o
> obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
> obj-y += s_record.o
> obj-y += xyzModem.o
> +obj-$(CONFIG_GENERIC_FIRMWARE_LOADER) += fs_loader.o
> diff --git a/common/fs_loader.c b/common/fs_loader.c
> new file mode 100644
> index 0000000..7fc318f
> --- /dev/null
> +++ b/common/fs_loader.c
> @@ -0,0 +1,365 @@
> +/*
> + * Copyright (C) 2018 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>
> +#ifdef CONFIG_CMD_UBIFS
> +#include <ubi_uboot.h>
> +#include <ubifs_uboot.h>
> +#endif
> +#include <usb.h>
> +
> +struct firmware_priv {
> + const char *name; /* Filename */
> + u32 offset; /* Offset of reading a file */
> +};
> +
> +static struct device_location default_locations[] = {
> + {
> + .name = "mmc",
> + .devpart = "0:1",
> + },
> + {
> + .name = "usb",
> + .devpart = "0:1",
> + },
> + {
> + .name = "sata",
> + .devpart = "0:1",
> + },
> + {
> + .name = "ubi",
> + .mtdpart = "UBI",
> + .ubivol = "ubi0",
> + },
> +};
> +
> +/* USB build is not supported yet in SPL */
> +#ifndef CONFIG_SPL_BUILD
> +#ifdef CONFIG_USB_STORAGE
> +static int init_usb(void)
> +{
> + int err;
> +
> + err = usb_init();
> + if (err)
> + return err;
> +
> +#ifndef CONFIG_DM_USB
> + err = usb_stor_scan(0);
> + if (err)
> + return err;
> +#endif
> +
> + return err;
> +}
> +#else
> +static int init_usb(void)
> +{
> + debug("Error: Cannot load flash image: no USB support\n");
> + return -ENOSYS;
> +}
> +#endif
> +#endif
> +
> +#ifdef CONFIG_SATA
> +static int init_storage_sata(struct device_location *location)
> +{
> + char *dup_str = NULL;
> + const char *dev_str, *part_str;
> + int dev;
> +
> + /* Get device ID */
> + part_str = strchr(location->devpart, ':');
> + if (part_str) {
> + dup_str = strdup(location->devpart);
> + dup_str[part_str - location->devpart] = 0;
> + dev_str = dup_str;
> + part_str++;
> + } else {
> + dev_str = location->devpart;
> + }
> +
> + dev = (int)simple_strtoul(dev_str, NULL, 10);
> +
> + return sata_probe(dev);
>
You are leaking the strdup() memory.
Furthermore, there is no need to mess around with the 'devpart' string
like this. simple_strtoul() will convert the string up to the first
non-digit character and return the position of this char in the second
argument (if that's non-null). Thus you can easily check for a valid
numerical argument.
> +static void set_storage_devpart(char *name, char *devpart)
> +{
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(default_locations); i++) {
> + if (!strcmp(default_locations[i].name, name))
> + default_locations[i].devpart = devpart;
> + }
> +}
> +
> +static void set_storage_mtdpart(char *name, char *mtdpart)
> +{
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(default_locations); i++) {
> + if (!strcmp(default_locations[i].name, name))
> + default_locations[i].mtdpart = mtdpart;
> + }
> +}
> +
> +static void set_storage_ubivol(char *name, char *ubivol)
> +{
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(default_locations); i++) {
> + if (!strcmp(default_locations[i].name, name))
> + default_locations[i].ubivol = ubivol;
> + }
> +}
> +
The arguments to these functions could well be const.
Lothar Waßmann
More information about the U-Boot
mailing list