[U-Boot] [PATCH v3 3/3] common: Generic loader for file system
Simon Glass
sjg at chromium.org
Tue Jun 26 03:58:51 UTC 2018
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?
>
> 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.
> + } 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?
> + 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/
> +{
> + 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?
> +
> + ubi_volume = env_get("fw_ubi_volume");
> + if (ubi_volume)
> + set_storage_ubivol(plat, ubi_volume);
> +
> + ret = select_fs_dev(plat);
> + if (ret)
> + goto out;
> +
> + fw_priv = firmware_p->priv;
> +
> + ret = fs_read(fw_priv->name, (ulong)firmware_p->data, fw_priv->offset,
map_to_sysmem(firmware_p->data)
(so that it works on sandbox)
> + firmware_p->size, &actread);
> + if (ret) {
> + debug("Error: %d Failed to read %s from flash %lld != %d.\n",
> + ret, fw_priv->name, actread, firmware_p->size);
> + } else {
> + ret = actread;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * request_firmware_into_buf - Load firmware into a previously allocated buffer.
> + * @plat: Platform data such as storage and partition firmware loading from.
> + * @firmware_p: Pointer to firmware image.
> + * @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.
> + *
> + * The firmware is loaded directly into the buffer pointed to by @buf and
> + * the @firmware_p data member is pointed at @buf.
> + *
> + * Return: Size of total read, negative value when error.
> + */
> +int request_firmware_into_buf(struct device_platdata *plat,
> + struct firmware **firmware_p,
> + const char *name,
> + void *buf, size_t size, u32 offset)
> +{
> + int ret;
> +
> + if (!plat)
> + return -EINVAL;
> +
> + ret = _request_firmware_prepare(firmware_p, name, buf, size, offset);
> + if (ret < 0) /* error */
> + return ret;
> +
> + ret = fw_get_filesystem_firmware(plat, *firmware_p);
> +
> + return ret;
> +}
> +
> +static int fs_loader_ofdata_to_platdata(struct udevice *dev)
> +{
> + const char *fs_loader_path;
> +
> + 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;
> +
> + plat->name = (char *)ofnode_read_string(fs_loader_node,
> + "storage_device");
> +
> + plat->devpart = (char *)ofnode_read_string(
> + fs_loader_node, "devpart");
> +
> + 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 = "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..e3e5eeb
> --- /dev/null
> +++ b/include/fs_loader.h
> @@ -0,0 +1,28 @@
> +/*
> + * 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 {
> + size_t size; /* Size of a file */
> + const u8 *data; /* Buffer for file */
> + void *priv; /* Firmware loader private fields */
> +};
> +
> +struct device_platdata {
> + char *name; /* Such as mmc, usb,and sata. */
> + char *devpart; /* Use the load command dev:part conventions */
> + char *mtdpart; /* MTD partition for ubi part */
> + char *ubivol; /* UBI volume-name for ubifsmount */
> +};
> +
> +int request_firmware_into_buf(struct device_platdata *plat,
> + struct firmware **firmware_p,
> + const char *name,
> + void *buf, size_t size, u32 offset);
Please can you add a full function comment here?
> +#endif
> --
> 2.2.0
>
Regards,
Simon
More information about the U-Boot
mailing list