[U-Boot] [PATCH v4 6/6] common: Generic loader for file system

[Michal Simek]

On 6.7.2018 10: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 | 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 <www.intel.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0
> + */
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <blk.h>
> +#include <fs.h>
> +#include <fs_loader.h>
> +#include <linux/string.h>
> +#include <mapmem.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 */
> +};
> +
> +#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.

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.

Also I can't see any usage of firmware_loader from chosen which you have
in 4/6.

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.

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.

Thanks,
Michal