[U-Boot] [PATCH v3 3/3] common: Generic loader for file system
Chee, Tien Fong
tien.fong.chee at intel.com
Thu Jun 28 04:45:17 UTC 2018
On Wed, 2018-06-27 at 14:03 -0700, Simon Glass wrote:
> Hi Tien Fong,
>
> On 27 June 2018 at 01:41, Chee, Tien Fong <tien.fong.chee at intel.com>
> wrote:
> >
> > On Mon, 2018-06-25 at 21:58 -0600, Simon Glass wrote:
> > >
> > > Hi,
> > >
> > > On 25 June 2018 at 07:28, <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>
> > > > ---
> > > > drivers/misc/Kconfig | 10 ++
> > > > drivers/misc/Makefile | 1 +
> > > > drivers/misc/fs_loader.c | 238
> > > > +++++++++++++++++++++++++++++++++++++++++++++++
> > > > include/dm/uclass-id.h | 1 +
> > > > include/fs_loader.h | 28 ++++++
> > > > 5 files changed, 278 insertions(+)
> > > > create mode 100644 drivers/misc/fs_loader.c
> > > > create mode 100644 include/fs_loader.h
> > > This is definitely looking good. I think we need to figure out
> > > the
> > > binding to devices, to use phandles instead of strings. Also we
> > > need
> > > a
> > > sandbox driver and test.
> > >
> > > I'm a little worried that you are blazing a trail here, but it is
> > > a
> > > valuable trail :-)
> > >
> > > Tom, do you have any ideas / thoughts on the design?
> > >
> > yeah, this part is most challenging, because this driver is
> > designed
> > based on file system implementation, which is abstract from
> > physical
> > device. So, it requires data like interface type, device number and
> > partition to work. Wonder how we can get those data if based on
> > physical storage node.
> > >
> > > >
> > > >
> > > >
> > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > > index 17b3a80..4163b4f 100644
> > > > --- a/drivers/misc/Kconfig
> > > > +++ b/drivers/misc/Kconfig
> > > > @@ -277,4 +277,14 @@ config GDSYS_RXAUI_CTRL
> > > > depends on MISC
> > > > help
> > > > Support gdsys FPGA's RXAUI control.
> > > > +
> > > > +config FS_LOADER
> > > > + bool "Enable 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.
> > > > +
> > > > endmenu
> > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > > index 4ce9d21..67a36f8 100644
> > > > --- a/drivers/misc/Makefile
> > > > +++ b/drivers/misc/Makefile
> > > > @@ -54,3 +54,4 @@ obj-$(CONFIG_STM32_RCC) += stm32_rcc.o
> > > > obj-$(CONFIG_STM32MP_FUSE) += stm32mp_fuse.o
> > > > obj-$(CONFIG_SYS_DPAA_QBMAN) += fsl_portals.o
> > > > obj-$(CONFIG_GDSYS_RXAUI_CTRL) += gdsys_rxaui_ctrl.o
> > > > +obj-$(CONFIG_FS_LOADER) += fs_loader.o
> > > > diff --git a/drivers/misc/fs_loader.c
> > > > b/drivers/misc/fs_loader.c
> > > > new file mode 100644
> > > > index 0000000..0dc385f
> > > > --- /dev/null
> > > > +++ b/drivers/misc/fs_loader.c
> > > > @@ -0,0 +1,238 @@
> > > > +/*
> > > > + * Copyright (C) 2018 Intel Corporation <www.intel.com>
> > > > + *
> > > > + * SPDX-License-Identifier: GPL-2.0
> > > > + */
> > > > +#include <common.h>
> > > > +#include <dm.h>
> > > > +#include <errno.h>
> > > > +#include <fs.h>
> > > > +#include <fs_loader.h>
> > > > +#include <linux/string.h>
> > > > +#include <malloc.h>
> > > > +#include <spl.h>
> > > > +
> > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > +
> > > > +struct firmware_priv {
> > > > + const char *name; /* Filename */
> > > > + u32 offset; /* Offset of reading a file */
> > > > +};
> > > > +
> > > > +static int select_fs_dev(struct device_platdata *plat)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + if (!strcmp("mmc", plat->name)) {
> > > > + ret = fs_set_blk_dev("mmc", plat->devpart,
> > > > FS_TYPE_ANY);
> > > > + } else if (!strcmp("usb", plat->name)) {
> > > > + ret = fs_set_blk_dev("usb", plat->devpart,
> > > > FS_TYPE_ANY);
> > > > + } else if (!strcmp("sata", plat->name)) {
> > > > + ret = fs_set_blk_dev("sata", plat->devpart,
> > > > FS_TYPE_ANY);
> > > For these I think you can look up the phandle to obtain the
> > > device,
> > > then check its uclass to find out its type.
> > >
> > How to find the interface type of the file system based on the
> > physical
> > device node? Some devices like NAND and QSPI could share the
> > similar
> > interface type like UBI.
> See my other email on this.
>
Okay.
> >
> > >
> > > >
> > > >
> > > > + } else if (!strcmp("ubi", plat->name)) {
> > > > + if (plat->ubivol)
> > > > + ret = fs_set_blk_dev("ubi", NULL,
> > > > FS_TYPE_UBIFS);
> > > > + else
> > > > + ret = -ENODEV;
> > > > + } else {
> > > > + debug("Error: unsupported storage device.\n");
> > > > + return -ENODEV;
> > > > + }
> > > > +
> > > > + if (ret)
> > > > + debug("Error: could not access storage.\n");
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void set_storage_devpart(struct device_platdata *plat,
> > > > char
> > > > *devpart)
> > > > +{
> > > > + plat->devpart = devpart;
> > > > +}
> > > > +
> > > > +static void set_storage_mtdpart(struct device_platdata *plat,
> > > > char
> > > > *mtdpart)
> > > > +{
> > > > + plat->mtdpart = mtdpart;
> > > > +}
> > > > +
> > > > +static void set_storage_ubivol(struct device_platdata *plat,
> > > > char
> > > > *ubivol)
> > > > +{
> > > > + plat->ubivol = ubivol;
> > > > +}
> > > > +
> > > > +/**
> > > > + * _request_firmware_prepare - Prepare firmware struct.
> > > > + *
> > > > + * @firmware_p: Pointer to pointer to firmware image.
> > > > + * @name: Name of firmware file.
> > > > + * @dbuf: Address of buffer to load firmware into.
> > > > + * @size: Size of buffer.
> > > > + * @offset: Offset of a file for start reading into buffer.
> > > > + *
> > > > + * Return: Negative value if fail, 0 for successful.
> > > > + */
> > > > +static int _request_firmware_prepare(struct firmware
> > > > **firmware_p,
> > > Can you please move this to be the last arg and rename to
> > > firmwarep?
> > >
> > Okay.
> > >
> > > >
> > > >
> > > > + const char *name, void
> > > > *dbuf,device to ubi
> > > > + size_t size, u32 offset)
> > > > +{
> > > > + struct firmware *firmware;
> > > > + struct firmware_priv *fw_priv;
> > > > +
> > > > + *firmware_p = NULL;
> > > > +
> > > > + if (!name || name[0] == '\0')
> > > > + return -EINVAL;
> > > > +
> > > > + firmware = calloc(1, sizeof(*firmware));
> > > > + if (!firmware)
> > > > + return -ENOMEM;
> > > > +
> > > > + fw_priv = calloc(1, sizeof(*fw_priv));
> > > > + if (!fw_priv) {
> > > > + free(firmware);
> > > > + return -ENOMEM;
> > > > + }
> > > > +
> > > > + fw_priv->name = name;
> > > > + fw_priv->offset = offset;
> > > > + firmware->data = dbuf;
> > > > + firmware->size = size;
> > > > + firmware->priv = fw_priv;
> > > > + *firmware_p = firmware;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/**
> > > > + * fw_get_filesystem_firmware - load firmware into an
> > > > allocated
> > > > buffer.
> > > > + * @plat: Platform data such as storage and partition firmware
> > > > loading from.
> > > > + * @firmware_p: pointer to firmware image.
> > > > + *
> > > > + * Return: Size of total read, negative value when error.
> > > > + */
> > > > +static int fw_get_filesystem_firmware(struct device_platdata
> > > > *plat,
> > > > + struct firmware
> > > > *firmware_p)
> > > s/firmware_p/firmware/
> > >
> > Okay.
> > >
> > > >
> > > >
> > > > +{
> > > > + struct firmware_priv *fw_priv = NULL;
> > > > + loff_t actread;
> > > > + char *dev_part, *ubi_mtdpart, *ubi_volume;
> > > > + int ret;
> > > > +
> > > > + dev_part = env_get("fw_dev_part");
> > > > + if (dev_part)
> > > > + set_storage_devpart(plat, dev_part);
> > > > +
> > > > + ubi_mtdpart = env_get("fw_ubi_mtdpart");
> > > > + if (ubi_mtdpart)
> > > > + set_storage_mtdpart(plat, ubi_mtdpart);
> > > Do you mean that the storage partition comes from the
> > > environment? I
> > > thought it came from the FDT?
> > >
> > This driver is designed not only supporting the FDT, it also work
> > with
> > U-boot env, U-boot script and without FDT.
> >
> > For example, fpga loadfs commands from U-boot console can call this
> > driver to load bitstream file from the storage.
> OK, but in that case why use environment? Can't you just put the
> parameters in the command?
>
User can save the value like partition into environment and saving in
the storage as new default env value. So, this env value would be used
for every power on.
> [..]
>
> Regards,
> Simon
More information about the U-Boot
mailing list