On Tue, 2018-02-27 at 11:04 +0100, Lothar Waßmann wrote: > Hi, > > On Tue, 27 Feb 2018 13:21:57 +0800 tien.fong.c...@intel.com wrote: > > > > From: Tien Fong Chee <tien.fong.c...@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.c...@intel.com> > > Reviewed-by: Lothar Waßmann <l...@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. Good catch!!
> 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. > Okay, i would try with simple_strtoul(). > > > > +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. > Okay. noted. > > > Lothar Waßmann _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot