Hi Tien, On 6 July 2018 at 02:28, <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> > --- > 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.
*a* file image? > + > + The consumer driver would then use this loader to program whatever, > + ie. the FPGA device. e.g. an 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 */ I don't understand that comment. Is it the offset within the file to read from? Please can you expand it a bit? > +}; > + > +#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"); blank line here > + 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); Can you please create device_get_global_by_ofnode() and use that instead? We should not use offsets in new code. > + 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); > + } else { > + debug("Error: unsupported storage device.\n"); > + return -ENODEV; > + } > + > + if (ret) > + debug("Error: could not access storage.\n"); > + > + return ret; > +} > + > +/** > + * _request_firmware_prepare - Prepare firmware struct. > + * > + * @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. > + * @firmwarep: Pointer to pointer to firmware image. > + * > + * Return: Negative value if fail, 0 for successful. > + */ > +static int _request_firmware_prepare(const char *name, void *dbuf, > + size_t size, u32 offset, > + struct firmware **firmwarep) > +{ struct firmware *firmware = *firmwarep; and use that instead of the clumsy *firmwarep in this function > + if (!name || name[0] == '\0') > + return -EINVAL; > + > + /* No memory allocation is required if *firmwarep is allocated */ > + if (!(*firmwarep)) { Can this not be allocated automatically by the uclass or driver? > + (*firmwarep) = calloc(1, sizeof(struct firmware)); > + if (!(*firmwarep)) > + return -ENOMEM; > + > + (*firmwarep)->priv = calloc(1, sizeof(struct firmware_priv)); > + if (!(*firmwarep)->priv) { > + free(*firmwarep); > + return -ENOMEM; > + } > + } else if (!(*firmwarep)->priv) { Can you not use the driver's priv_auto_alloc_size to allocate this? Or the uclass? > + (*firmwarep)->priv = calloc(1, sizeof(struct firmware_priv)); > + if (!(*firmwarep)->priv) { > + free(*firmwarep); > + return -ENOMEM; > + } > + } > + > + ((struct firmware_priv *)((*firmwarep)->priv))->name = name; Again this needs a local variable. > + ((struct firmware_priv *)((*firmwarep)->priv))->offset = offset; > + (*firmwarep)->data = dbuf; > + (*firmwarep)->size = size; > + > + return 0; > +} > + > +/** > + * release_firmware - Release the resource associated with a firmware image > + * @firmware: Firmware resource to release > + */ > +void release_firmware(struct firmware *firmware) > +{ > + if (firmware) { > + if (firmware->priv) { > + free(firmware->priv); > + firmware->priv = NULL; > + } > + free(firmware); > + } > +} > + > +/** > + * fw_get_filesystem_firmware - load firmware into an allocated buffer. > + * @plat: Platform data such as storage and partition firmware loading from. > + * @firmware: 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) > +{ > + struct firmware_priv *fw_priv = NULL; > + loff_t actread; > + char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume; > + int ret; > + > + storage_interface = env_get("storage_interface"); > + dev_part = env_get("fw_dev_part"); > + ubi_mtdpart = env_get("fw_ubi_mtdpart"); > + ubi_volume = env_get("fw_ubi_volume"); > + > + if (storage_interface && dev_part) { > + ret = fs_set_blk_dev(storage_interface, dev_part, > FS_TYPE_ANY); > + } else if (storage_interface && ubi_mtdpart && ubi_volume) { > + ret = mount_ubifs(ubi_mtdpart, ubi_volume); > + if (ret) > + return ret; > + > + if (!strcmp("ubi", storage_interface)) > + ret = fs_set_blk_dev(storage_interface, NULL, > + FS_TYPE_UBIFS); > + else > + ret = -ENODEV; > + } else { > + ret = select_fs_dev(plat); > + } > + > + if (ret) > + goto out; > + > + fw_priv = firmware->priv; > + > + ret = fs_read(fw_priv->name, (ulong)map_to_sysmem(firmware->data), > + fw_priv->offset, firmware->size, &actread); > + if (ret) { > + debug("Error: %d Failed to read %s from flash %lld != %d.\n", > + ret, fw_priv->name, actread, firmware->size); > + } else { > + ret = actread; > + } > + > +out: > +#ifdef CONFIG_CMD_UBIFS > + umount_ubifs(); > +#endif debug() here to report failure > + return ret; > +} > + > +/** > + * request_firmware_into_buf - Load firmware into a previously allocated > buffer. > + * @plat: Platform data such as storage and partition firmware loading from. > + * @name: Name of firmware file. > + * @buf: Address of buffer to load firmware into. > + * @size: Size of buffer. > + * @offset: Offset of a file for start reading into buffer. > + * @firmwarep: Pointer to firmware image. > + * > + * The firmware is loaded directly into the buffer pointed to by @buf and > + * the @firmwarep data member is pointed at @buf. > + * > + * Return: Size of total read, negative value when error. > + */ > +int request_firmware_into_buf(struct device_platdata *plat, > + const char *name, > + void *buf, size_t size, u32 offset, > + struct firmware **firmwarep) > +{ > + int ret; > + > + if (!plat) > + return -EINVAL; > + > + ret = _request_firmware_prepare(name, buf, size, offset, firmwarep); > + if (ret < 0) /* error */ > + return ret; > + > + ret = fw_get_filesystem_firmware(plat, *firmwarep); > + > + return ret; > +} > + > +static int fs_loader_ofdata_to_platdata(struct udevice *dev) > +{ > + const char *fs_loader_path; > + u32 phandlepart[2]; > + > + fs_loader_path = ofnode_get_chosen_prop("firmware-loader"); > + > + if (fs_loader_path) { > + ofnode fs_loader_node; > + > + fs_loader_node = ofnode_path(fs_loader_path); > + if (ofnode_valid(fs_loader_node)) { > + struct device_platdata *plat; > + plat = dev->platdata; > + > + if (!ofnode_read_u32_array(fs_loader_node, > + "phandlepart", > + phandlepart, 2)) { > + plat->phandlepart.phandle = phandlepart[0]; > + plat->phandlepart.partition = phandlepart[1]; > + } > + > + plat->mtdpart = (char *)ofnode_read_string( > + fs_loader_node, "mtdpart"); > + > + plat->ubivol = (char *)ofnode_read_string( > + fs_loader_node, "ubivol"); > + } > + } > + > + return 0; > +} > + > +static int fs_loader_probe(struct udevice *dev) > +{ > + return 0; > +}; > + > +static const struct udevice_id fs_loader_ids[] = { > + { .compatible = "u-boot,fs-loader"}, > + { } > +}; > + > +U_BOOT_DRIVER(fs_loader) = { > + .name = "fs-loader", > + .id = UCLASS_FS_FIRMWARE_LOADER, > + .of_match = fs_loader_ids, > + .probe = fs_loader_probe, > + .ofdata_to_platdata = fs_loader_ofdata_to_platdata, > + .platdata_auto_alloc_size = sizeof(struct device_platdata), > +}; > + > +UCLASS_DRIVER(fs_loader) = { > + .id = UCLASS_FS_FIRMWARE_LOADER, > + .name = "fs-loader", > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index d7f9df3..39e88ac 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -36,6 +36,7 @@ enum uclass_id { > UCLASS_DMA, /* Direct Memory Access */ > UCLASS_EFI, /* EFI managed devices */ > UCLASS_ETH, /* Ethernet device */ > + UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */ > UCLASS_GPIO, /* Bank of general-purpose I/O pins */ > UCLASS_FIRMWARE, /* Firmware */ > UCLASS_I2C, /* I2C bus */ > diff --git a/include/fs_loader.h b/include/fs_loader.h > new file mode 100644 > index 0000000..0be4f17 > --- /dev/null > +++ b/include/fs_loader.h > @@ -0,0 +1,79 @@ > +/* > + * Copyright (C) 2018 Intel Corporation <www.intel.com> > + * > + * SPDX-License-Identifier: GPL-2.0 > + */ > +#ifndef _FS_LOADER_H_ > +#define _FS_LOADER_H_ > + > +#include <dm.h> > + > +/** > + * struct firmware - A place for storing firmware and its attribute data. > + * > + * This holds information about a firmware and its content. > + * > + * @size: Size of a file > + * @data: Buffer for file > + * @priv: Firmware loader private fields > + */ > +struct firmware { > + size_t size; > + const u8 *data; > + void *priv; > +}; Let's try to get rid of priv by using driver model. > + > +/** > + * struct phandle_part - A place for storing phandle of node and its > partition > + * > + * This holds information about a phandle of the block device, and its > + * partition where the firmware would be loaded from. > + * > + * @phandle: Phandle of storage device node > + * @partition: Partition of block device > + */ > +struct phandle_part { struct device_part ? A phandle is a device-tree concept. > + u32 phandle; > + u32 partition; > +}; > + > +/** > + * struct phandle_part - A place for storing all supported storage devices > + * > + * This holds information about all supported storage devices for driver use. > + * > + * @phandlepart: Attribute data for block device. nit: please drop your periods at the end of these lines - they are not sentences. > + * @mtdpart: MTD partition for ubi partition. > + * @ubivol: UBI volume-name for ubifsmount. > + */ > +struct device_platdata { > + struct phandle_part phandlepart; > + char *mtdpart; > + char *ubivol; > +}; > + > +/** > + * release_firmware - Release the resource associated with a firmware image > + * @firmware: Firmware resource to release > + */ > +void release_firmware(struct firmware *firmware); > + > +/** > + * request_firmware_into_buf - Load firmware into a previously allocated > buffer. > + * @plat: Platform data such as storage and partition firmware loading from. > + * @name: Name of firmware file. > + * @buf: Address of buffer to load firmware into. > + * @size: Size of buffer. > + * @offset: Offset of a file for start reading into buffer. > + * @firmwarep: Pointer to firmware image. > + * > + * The firmware is loaded directly into the buffer pointed to by @buf and > + * the @firmwarep data member is pointed at @buf. > + * > + * Return: Size of total read, negative value when error. > + */ > +int request_firmware_into_buf(struct device_platdata *plat, > + const char *name, > + void *buf, size_t size, u32 offset, > + struct firmware **firmwarep); Without a test it's hard to see how this is used, but I'm not keen on the struct firmware ** here. Can you just use struct firmware * instead? Or maybe just return the fields individually since you only have start address and size, I think. > +#endif > -- > 2.2.0 > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot