From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chee, Tien Fong Date: Wed, 25 Jul 2018 06:31:39 +0000 Subject: [U-Boot] [PATCH v4 6/6] common: Generic loader for file system In-Reply-To: <30e0c183-12d9-667d-3e51-caa986b399ff@xilinx.com> References: <1530865683-23207-1-git-send-email-tien.fong.chee@intel.com> <30e0c183-12d9-667d-3e51-caa986b399ff@xilinx.com> Message-ID: <1532500298.9671.24.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Wed, 2018-07-18 at 16:48 +0200, Michal Simek wrote: > On 6.7.2018 10:28, tien.fong.chee at 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 > > --- > >  drivers/misc/Kconfig     |  10 ++ > >  drivers/misc/Makefile    |   1 + > >  drivers/misc/fs_loader.c | 295 > > +++++++++++++++++++++++++++++++++++++++++++++++ > >  include/dm/uclass-id.h   |   1 + > >  include/fs_loader.h      |  79 +++++++++++++ > >  5 files changed, 386 insertions(+) > >  create mode 100644 drivers/misc/fs_loader.c > >  create mode 100644 include/fs_loader.h > > > > 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..5fe642b > > --- /dev/null > > +++ b/drivers/misc/fs_loader.c > > @@ -0,0 +1,295 @@ > > +/* > > + * Copyright (C) 2018 Intel Corporation > > + * > > + * SPDX-License-Identifier:    GPL-2.0 > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +struct firmware_priv { > > + const char *name; /* Filename */ > > + u32 offset; /* Offset of reading a file */ > > +}; > > + > > +#ifdef CONFIG_CMD_UBIFS > > +static int mount_ubifs(char *mtdpart, char *ubivol) > > +{ > > + int ret = ubi_part(mtdpart, NULL); > > + > > + if (ret) { > > + debug("Cannot find mtd partition %s\n", mtdpart); > > + return ret; > > + } > > + > > + return cmd_ubifs_mount(ubivol); > > +} > > + > > +static int umount_ubifs(void) > > +{ > > + return cmd_ubifs_umount(); > > +} > > +#else > > +static int mount_ubifs(char *mtdpart, char *ubivol) > > +{ > > + debug("Error: Cannot load image: no UBIFS support\n"); > > + return -ENOSYS; > > +} > > +#endif > > + > > +static int select_fs_dev(struct device_platdata *plat) > > +{ > > + int ret; > > + > > + if (plat->phandlepart.phandle) { > > + ofnode node; > > + > > + node = ofnode_get_by_phandle(plat- > > >phandlepart.phandle); > > + > > + int of_offset = ofnode_to_offset(node); > > + > > + struct udevice *dev; > > + > > + ret = device_get_global_by_of_offset(of_offset, > > &dev); > > + if (!ret) { > > + struct blk_desc *desc = > > blk_get_by_device(dev); > > + if (desc) { > > + ret = > > fs_set_blk_dev_with_part(desc, > > + plat- > > >phandlepart.partition); > > + } else { > > + debug("%s: No device found\n", > > __func__); > > + return -ENODEV; > > + } > > + } > > + } else if (plat->mtdpart && plat->ubivol) { > > + ret = mount_ubifs(plat->mtdpart, plat->ubivol); > > + if (ret) > > + return ret; > > + > > + ret = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS); > I am curious why it is in generic FS loader any code which target any > filesystem. It should be filesystem independent. Because it supports our use case, and our preference using file system instead of RAW. As I agree with Tom, it can be evolved to support RAW in future. > > Also that DT binding is quite weird and I don't think you will get > ACK > for this from device tree community at all. I think that calling via > platdata and avoid DT nodes would be better way to go. Why do you think DT binding is weird? The DT is designed based on Simon proposal, and i believe following the rules in DTS spec. There are some DT benefits with current design, i think someone may be maintainer need to made the final decision on the design. > > Also I can't see any usage of firmware_loader from chosen which you > have > in 4/6. https://patchwork.ozlabs.org/patch/940319/ This contains some explanation about chosen, default storage defined by user in DTS. fs_loader_ofdata_to platdata() is used to parse the configuration in chosen. > > Also based on discussion with Marek I am not quite sure if this can > be > used for his SPL FPGA A10 bitstream loading from FS by chuck from FIT > image as he mentioned. This design is currently not support RAW, but i think it can be enhanced in future. I'm still prefer to put the FPGA design in filesystem instead of RAW, thinking about the filesystem benefits. > > And last but not least I am missing user of this loader. I think it > will > be the best to also send a user of this to see how exactly this will > be > called and used. https://patchwork.ozlabs.org/patch/940319/ , did you read the doc? I have setting up a test env for this design which is using FPGA manager to call this loader for chunk by chunk transfering the FPGA design from storage into FPGA manager. It would be a bit messy and complicated to understand from the codes but it would be helpful to understand the use of this loader. Do you want it?  > > Thanks, > Michal