Re: [U-Boot] [PATCH v7 3/3] common: Generic firmware loader for file system

2018-02-01 Thread Marek Vasut 
On 02/01/2018 08:06 AM, Chee, Tien Fong wrote:
> On Tue, 2018-01-30 at 13:12 +0100, Marek Vasut wrote:
>> On 01/30/2018 12:16 PM, tien.fong.c...@intel.com wrote:
>>>
>>> From: Tien Fong Chee 
>>>
>>> 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 
>>> Reviewed-by: Lothar Waßmann 
>>> ---
>>>  common/Kconfig |   9 ++
>>>  common/Makefile|   1 +
>>>  common/fs_loader.c | 326
>>> +
>>>  doc/README.firmware_loader |  76 +++
>>>  include/fs_loader.h|  28 
>>>  5 files changed, 440 insertions(+)
>>>  create mode 100644 common/fs_loader.c
>>>  create mode 100644 doc/README.firmware_loader
>>>  create mode 100644 include/fs_loader.h
>> [...]
>>
>>>
>>> +#ifdef CONFIG_CMD_UBIFS
>>> +static int mount_ubifs(struct device_location *location)
>>> +{
>>> +   int ret;
>>> +   char cmd[32];
>>> +
>>> +   snprintf(cmd, sizeof(location->mtdpart), "ubi part %s",
>>> +    location->mtdpart);
>>> +
>>> +   ret = run_command(cmd, 0);
>> Can you call the UBI functions directly instead of invoking the U-
>> Boot
>> command parser ?
>>
> Okay.
>>>
>>> +   if (ret)
>>> +   return ret;
>>> +
>>> +   snprintf(cmd, sizeof(location->ubivol), "ubifsmount %s",
>>> +    location->ubivol);
>>> +
>>> +   ret = run_command(cmd, 0);
>> DTTO here ...
>>
> Okay.
> 
> Could i submit the next version immediately for this minor change?

Wait a bit for some further feedback. Also make sure to go through the
patchset one more time and see if there aren't any similar mistakes.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v7 3/3] common: Generic firmware loader for file system

2018-01-31 Thread Chee, Tien Fong 
On Tue, 2018-01-30 at 13:12 +0100, Marek Vasut wrote:
> On 01/30/2018 12:16 PM, tien.fong.c...@intel.com wrote:
> > 
> > From: Tien Fong Chee 
> > 
> > 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 
> > Reviewed-by: Lothar Waßmann 
> > ---
> >  common/Kconfig |   9 ++
> >  common/Makefile|   1 +
> >  common/fs_loader.c | 326
> > +
> >  doc/README.firmware_loader |  76 +++
> >  include/fs_loader.h|  28 
> >  5 files changed, 440 insertions(+)
> >  create mode 100644 common/fs_loader.c
> >  create mode 100644 doc/README.firmware_loader
> >  create mode 100644 include/fs_loader.h
> [...]
> 
> > 
> > +#ifdef CONFIG_CMD_UBIFS
> > +static int mount_ubifs(struct device_location *location)
> > +{
> > +   int ret;
> > +   char cmd[32];
> > +
> > +   snprintf(cmd, sizeof(location->mtdpart), "ubi part %s",
> > +    location->mtdpart);
> > +
> > +   ret = run_command(cmd, 0);
> Can you call the UBI functions directly instead of invoking the U-
> Boot
> command parser ?
> 
Okay.
> > 
> > +   if (ret)
> > +   return ret;
> > +
> > +   snprintf(cmd, sizeof(location->ubivol), "ubifsmount %s",
> > +    location->ubivol);
> > +
> > +   ret = run_command(cmd, 0);
> DTTO here ...
> 
Okay.

Could i submit the next version immediately for this minor change?

> > 
> > +   return ret;
> > +}
> > +
> > +static int umount_ubifs(void)
> > +{
> > +   return cmd_ubifs_umount();
> > +}
> > +#else
> > +static int mount_ubifs(struct device_location *location)
> > +{
> > +   debug("Error: Cannot load image: no UBIFS support\n");
> > +   return -ENOSYS;
> > +}
> > +#endif
> [...]
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v7 3/3] common: Generic firmware loader for file system

2018-01-30 Thread Marek Vasut 
On 01/30/2018 12:16 PM, tien.fong.c...@intel.com wrote:
> From: Tien Fong Chee 
> 
> 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 
> Reviewed-by: Lothar Waßmann 
> ---
>  common/Kconfig |   9 ++
>  common/Makefile|   1 +
>  common/fs_loader.c | 326 
> +
>  doc/README.firmware_loader |  76 +++
>  include/fs_loader.h|  28 
>  5 files changed, 440 insertions(+)
>  create mode 100644 common/fs_loader.c
>  create mode 100644 doc/README.firmware_loader
>  create mode 100644 include/fs_loader.h

[...]

> +#ifdef CONFIG_CMD_UBIFS
> +static int mount_ubifs(struct device_location *location)
> +{
> + int ret;
> + char cmd[32];
> +
> + snprintf(cmd, sizeof(location->mtdpart), "ubi part %s",
> +  location->mtdpart);
> +
> + ret = run_command(cmd, 0);

Can you call the UBI functions directly instead of invoking the U-Boot
command parser ?

> + if (ret)
> + return ret;
> +
> + snprintf(cmd, sizeof(location->ubivol), "ubifsmount %s",
> +  location->ubivol);
> +
> + ret = run_command(cmd, 0);

DTTO here ...

> + return ret;
> +}
> +
> +static int umount_ubifs(void)
> +{
> + return cmd_ubifs_umount();
> +}
> +#else
> +static int mount_ubifs(struct device_location *location)
> +{
> + debug("Error: Cannot load image: no UBIFS support\n");
> + return -ENOSYS;
> +}
> +#endif

[...]
-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v7 3/3] common: Generic firmware loader for file system

2018-01-30 Thread tien . fong . chee 
From: Tien Fong Chee 

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 
Reviewed-by: Lothar Waßmann 
---
 common/Kconfig |   9 ++
 common/Makefile|   1 +
 common/fs_loader.c | 326 +
 doc/README.firmware_loader |  76 +++
 include/fs_loader.h|  28 
 5 files changed, 440 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 21e067c..cbb0976 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -547,6 +547,15 @@ 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"
+   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 000..c58a492
--- /dev/null
+++ b/common/fs_loader.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright (C) 2018 Intel Corporation 
+ *
+ * SPDX-License-Identifier:GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#ifdef CONFIG_SPL
+#include 
+#endif
+#include 
+#ifdef CONFIG_CMD_UBIFS
+#include 
+#endif
+#include 
+
+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",
+   },
+};
+
+/* 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(1);
+   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(void)
+{
+   return sata_probe(0);
+}
+#else
+static int init_storage_sata(void)
+{
+   debug("Error: Cannot load image: no SATA support\n");
+   return -ENOSYS;
+}
+#endif
+
+#ifdef CONFIG_CMD_UBIFS
+static int mount_ubifs(struct device_location *location)
+{
+   int ret;
+   char cmd[32];
+
+   snprintf(cmd, sizeof(location->mtdpart), "ubi part %s",
+location->mtdpart);
+
+   ret = run_command(cmd, 0);
+   if (ret)
+   return ret;
+
+   snprintf(cmd, sizeof(location->ubivol), "ubifsmount %s",
+location->ubivol);
+
+   ret = run_command(cmd, 0);
+
+   return ret;
+}
+
+static int umount_ubifs(void)
+{
+   return cmd_ubifs_umount();
+}
+#else
+static int mount_ubifs(struct device_location *location)
+{
+   debug("Error: Cannot load image: no UBIFS support\n");
+   return -ENOSYS;
+}
+#endif
+
+#if defined(CONFIG_SPL_MMC_SUPPORT) && defined(CONFIG_SPL_BUILD)
+__weak int init_mmc(void)
+{
+   /* Just for the case MMC is not yet initialized */
+   struct mmc *mmc = NULL;
+   int err;
+
+#ifdef CONFIG_SPL
+   spl_mmc_find_device(, spl_boot_device());
+#else
+   debug("#define CONFIG_SPL is required or overrriding %s\n",
+ __func__);
+   return -ENOENT;
+#endif
+
+   err = mmc_init(mmc);
+   if (err) {
+   debug("spl: mmc init failed with error: %d\n", err);
+   return err;
+   }
+
+   return err;
+}
+#else
+__weak int init_mmc(void)
+{
+   /* Expect somewhere already initialize MMC */
+   return 0;
+}
+#endif
+
+static int select_fs_dev(struct device_location *location)
+{
+   int ret;
+
+   if (!strcmp("mmc", location->name)) {
+