[PATCH v2 2/6] ramdisk: add ramdisk uclass and driver

Simon Glass sjg at chromium.org
Wed Jul 19 21:11:31 CEST 2023


Hi Masahisa,

On Thu, 13 Jul 2023 at 23:47, Masahisa Kojima
<masahisa.kojima at linaro.org> wrote:
>
> This introcudes the ramdisk uclass and driver.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> ---
> Newly introcuded in v2
>
>  disk/part.c                      |   3 +
>  drivers/block/Kconfig            |   7 +-
>  drivers/block/Makefile           |   2 +
>  drivers/block/blk-uclass.c       |   1 +
>  drivers/block/blk_ramdisk.c      | 187 +++++++++++++++++++++++++++++++
>  drivers/block/ramdisk-uclass.c   |  14 +++
>  include/dm/uclass-id.h           |   1 +
>  include/ramdisk.h                |  32 ++++++
>  lib/efi_loader/efi_device_path.c |  25 +++++
>  9 files changed, 271 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/block/blk_ramdisk.c
>  create mode 100644 drivers/block/ramdisk-uclass.c
>  create mode 100644 include/ramdisk.h

Please remember to add a sandbox driver and a test in test/dm

>
> diff --git a/disk/part.c b/disk/part.c
> index 35300df590..d0cee3cc03 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -152,6 +152,9 @@ void dev_print(struct blk_desc *dev_desc)
>         case UCLASS_EFI_MEDIA:
>                 printf("EFI media Block Device %d\n", dev_desc->devnum);
>                 break;
> +       case UCLASS_RAM_DISK:
> +               printf("RAM Disk Block Device %d\n", dev_desc->devnum);
> +               break;
>         case UCLASS_INVALID:
>                 puts("device type unknown\n");
>                 return;
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 5a1aeb3d2b..ab56ef3406 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -2,7 +2,7 @@ config BLK
>         bool # "Support block devices"
>         depends on DM
>         default y if MMC || USB || SCSI || NVME || IDE || AHCI || SATA
> -       default y if EFI_MEDIA || VIRTIO_BLK || PVBLOCK
> +       default y if EFI_MEDIA || VIRTIO_BLK || PVBLOCK || RAM_DISK
>         help
>           Enable support for block devices, such as SCSI, MMC and USB
>           flash sticks. These provide a block-level interface which permits
> @@ -255,3 +255,8 @@ config SYS_64BIT_LBA
>         help
>           Make the block subsystem use 64bit sector addresses, rather than the
>           default of 32bit.
> +
> +config RAM_DISK
> +       bool "Enable RAM disk"
> +       help
> +         This option enables to mount the RAM disk.

??

Please expand that. It tells me almost nothing. You should get a
checkpatch warning about writing such short descriptions.

> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index a161d145fd..e867c7a126 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -19,3 +19,5 @@ obj-$(CONFIG_BLKMAP) += blkmap.o
>  obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
>  obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
>  obj-$(CONFIG_EFI_MEDIA_BLK) += efi_blk.o
> +obj-$(CONFIG_RAM_DISK) += blk_ramdisk.o
> +obj-$(CONFIG_RAM_DISK) += ramdisk-uclass.o
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 614b975e25..ea411fe674 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -34,6 +34,7 @@ static struct {
>         { UCLASS_VIRTIO, "virtio" },
>         { UCLASS_PVBLOCK, "pvblock" },
>         { UCLASS_BLKMAP, "blkmap" },
> +       { UCLASS_RAM_DISK, "ramdisk" },
>  };
>
>  static enum uclass_id uclass_name_to_iftype(const char *uclass_idname)
> diff --git a/drivers/block/blk_ramdisk.c b/drivers/block/blk_ramdisk.c
> new file mode 100644
> index 0000000000..8016837a80
> --- /dev/null
> +++ b/drivers/block/blk_ramdisk.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  RAM Disk block device driver
> + *
> + *  Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <blk.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <log.h>
> +#include <part.h>
> +#include <ramdisk.h>
> +#include <dm/device-internal.h>
> +#include <dm/root.h>
> +
> +#if (IS_ENABLED(CONFIG_EFI_LOADER))

drop the #if

> +#include <efi_loader.h>
> +#endif
> +
> +#define MEM_BLOCK_SIZE_SHIFT 9 /* 512 bytes */
> +
> +/**
> + * ramdisk_read() - read from block device
> + *
> + * @dev:       device
> + * @blknr:     first block to be read
> + * @blkcnt:    number of blocks to read
> + * @buffer:    output buffer
> + * Return:     number of blocks transferred
> + */
> +static ulong ramdisk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> +                         void *buffer)
> +{
> +       u8 *start;
> +       struct ramdisk_blk_plat *plat = dev_get_plat(dev);
> +
> +       log_debug("read buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
> +                 (ulong)blkcnt);
> +
> +       if (!buffer)
> +               return 0;
> +
> +       if (blknr + blkcnt > (plat->size >> MEM_BLOCK_SIZE_SHIFT))
> +               return 0;
> +
> +       start = plat->start + (blknr << MEM_BLOCK_SIZE_SHIFT);
> +       memmove(buffer, start, blkcnt << MEM_BLOCK_SIZE_SHIFT);
> +
> +       return blkcnt;
> +}
> +
> +/**
> + * ramdisk_write() - write to block device
> + *
> + * @dev:       device
> + * @blknr:     first block to be write
> + * @blkcnt:    number of blocks to write
> + * @buffer:    input buffer
> + * Return:     number of blocks transferred
> + */
> +static ulong ramdisk_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> +                          const void *buffer)
> +{
> +       u8 *start;
> +       struct ramdisk_blk_plat *plat = dev_get_plat(dev);
> +
> +       log_debug("write buf=%p, block=%lx, count=%lx: ", buffer, (ulong)blknr,
> +                 (ulong)blkcnt);
> +
> +       if (!buffer)
> +               return 0;
> +
> +       if (blknr + blkcnt > (plat->size >> MEM_BLOCK_SIZE_SHIFT))
> +               return 0;
> +
> +       start = plat->start + (blknr << MEM_BLOCK_SIZE_SHIFT);
> +       memmove(start, buffer, blkcnt << MEM_BLOCK_SIZE_SHIFT);
> +
> +       return blkcnt;
> +}
> +
> +static const struct blk_ops ramdisk_blk_ops = {
> +       .read   = ramdisk_read,
> +       .write  = ramdisk_write,
> +};
> +
> +U_BOOT_DRIVER(ramdisk_blk) = {
> +       .name           = "ramdisk_blk",
> +       .id             = UCLASS_BLK,
> +       .ops            = &ramdisk_blk_ops,
> +       .plat_auto      = sizeof(struct ramdisk_blk_plat),
> +};
> +
> +/*
> + * ramdisk_mount - mount ramdisk
> + *
> + * @start_address:     The base address of registered RAM disk
> + * @size:              The size of registered RAM disk
> + * @guid:              The type of registered RAM disk
> + * Return:             Pointer to the udevice strucure, return NULL if failed

spelling

> + */
> +struct udevice *ramdisk_mount(u64 start_address, u64 size, void *guid)

what is guid? I want to avoid these infecting U-Boot where not nescessary

> +{
> +       int ret;
> +       char name[20];
> +       struct udevice *parent, *bdev;
> +       static struct ramdisk_blk_plat *plat;
> +
> +       if (!start_address || !size)
> +               return NULL;
> +
> +       ret = device_bind(dm_root(), DM_DRIVER_GET(ramdisk), "ramdisk", NULL,
> +                         ofnode_null(), &parent);

Can you use devicetree to bind this?

> +       if (ret) {
> +               log_err("bind ramdisk error\n");
> +               return NULL;
> +       }
> +       snprintf(name, sizeof(name), "ramdisk%d", dev_seq(parent));
> +       device_set_name(parent, name);
> +
> +       ret = blk_create_device(parent, "ramdisk_blk", "ramdisk_blk",
> +                               UCLASS_RAM_DISK, dev_seq(parent),
> +                               1 << MEM_BLOCK_SIZE_SHIFT,
> +                               (size >> MEM_BLOCK_SIZE_SHIFT) - 1, &bdev);

Can you use blk_create_devicef() ? You should not pass the devnum
parameter - use -1 instead.

> +       if (ret) {
> +               log_err("ramdisk create block device failed\n");
> +               goto err;
> +       }
> +       snprintf(name, sizeof(name), "ramdisk_blk#%d", dev_seq(parent));
> +       device_set_name(bdev, name);
> +
> +       plat = dev_get_plat(bdev);
> +       plat->start = (u8 *)start_address;
> +       plat->size = size;
> +#if (IS_ENABLED(CONFIG_EFI_LOADER))

Please use if()

But what does this have to do with EFI loader? That code lives in
lib/efi_loader, not here.

> +       if (guid)
> +               guidcpy(&plat->disk_type_guid, guid);
> +#endif
> +       ret = blk_probe_or_unbind(bdev);
> +       if (ret) {
> +               log_err("ramdisk probe error\n");
> +               goto err;
> +       }
> +
> +       return bdev;
> +
> +err:
> +       if (parent) {
> +               ret = device_remove(parent, DM_REMOVE_NORMAL);
> +               if (ret)
> +                       return NULL;
> +
> +               ret = device_unbind(parent);
> +       }
> +
> +       return NULL;
> +}
> +
> +/*
> + * ramdisk_unmount - unmount ramdisk
> + *
> + * @dev:       The device to be unmounted
> + * Return:     0 if success, negative value if error
> + */
> +int ramdisk_unmount(struct udevice *dev)
> +{
> +       int ret;
> +       struct udevice *parent;
> +
> +       if (!dev)
> +               return -EINVAL;
> +
> +       parent = dev->parent;
> +       ret = device_remove(parent, DM_REMOVE_NORMAL);
> +       if (ret)
> +               return ret;
> +
> +       ret = device_unbind(parent);
> +
> +       return ret;
> +}
> +
> +U_BOOT_DRIVER(ramdisk) = {
> +       .name           = "ramdisk",
> +       .id             = UCLASS_RAM_DISK,
> +};
> diff --git a/drivers/block/ramdisk-uclass.c b/drivers/block/ramdisk-uclass.c
> new file mode 100644
> index 0000000000..f1bf68f635
> --- /dev/null
> +++ b/drivers/block/ramdisk-uclass.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  RAM Disk block device driver
> + *
> + *  Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +
> +UCLASS_DRIVER(ramdisk) = {
> +       .name           = "ramdisk",
> +       .id             = UCLASS_RAM_DISK,
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 307ad6931c..42b4907b1f 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -109,6 +109,7 @@ enum uclass_id {
>         UCLASS_PWRSEQ,          /* Power sequence device */
>         UCLASS_QFW,             /* QEMU firmware config device */
>         UCLASS_RAM,             /* RAM controller */
> +       UCLASS_RAM_DISK,        /* RAM disk device */
>         UCLASS_REBOOT_MODE,     /* Reboot mode */
>         UCLASS_REGULATOR,       /* Regulator device */
>         UCLASS_REMOTEPROC,      /* Remote Processor device */
> diff --git a/include/ramdisk.h b/include/ramdisk.h
> new file mode 100644
> index 0000000000..71383be5b8
> --- /dev/null
> +++ b/include/ramdisk.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + *  ramdisk support
> + *
> + *  Copyright (c) 2023, Linaro Limited
> + */
> +
> +#ifndef _RAMDISK_H
> +#define _RAMDISK_H
> +
> +#ifdef CONFIG_EFI_LOADER
> +#include <efi.h>
> +#endif
> +
> +/**
> + * struct ramdisk_blk_plat - attributes of a block device
> + *
> + * @handle:    handle of the controller on which this driver is installed
> + * @io:                block io protocol proxied by this driver
> + */
> +struct ramdisk_blk_plat {
> +       u8 *start;
> +       u64 size;
> +#if (IS_ENABLED(CONFIG_EFI_LOADER))
> +       efi_guid_t disk_type_guid;
> +#endif
> +};
> +
> +struct udevice *ramdisk_mount(u64 start_address, u64 size, void *guid);
> +int ramdisk_unmount(struct udevice *dev);
> +
> +#endif
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 04ebb449ca..1f16bda6ac 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c

This should be a separate patch

> @@ -17,6 +17,7 @@
>  #include <nvme.h>
>  #include <efi_loader.h>
>  #include <part.h>
> +#include <ramdisk.h>
>  #include <uuid.h>
>  #include <asm-generic/unaligned.h>
>  #include <linux/compat.h> /* U16_MAX */
> @@ -568,6 +569,11 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
>                           */
>                         return dp_size(dev->parent)
>                                 + sizeof(struct efi_device_path_vendor) + 1;
> +#endif
> +#ifdef CONFIG_RAM_DISK
> +               case UCLASS_RAM_DISK:
> +                       return dp_size(dev->parent)
> +                               + sizeof(struct efi_device_path_ram_disk_path) + 1;
>  #endif
>                 default:
>                         return dp_size(dev->parent);
> @@ -766,6 +772,25 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
>                         dp->controller_number = desc->lun;
>                         return &dp[1];
>                 }
> +#endif
> +#if defined(CONFIG_RAM_DISK)
> +               case UCLASS_RAM_DISK: {
> +                       struct ramdisk_blk_plat *plat = dev_get_plat(dev);
> +                       struct efi_device_path_ram_disk_path *dp =
> +                               dp_fill(buf, dev->parent);
> +
> +                       dp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> +                       dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_RAM_DISK_PATH;
> +                       dp->dp.length =
> +                               sizeof(struct efi_device_path_ram_disk_path);
> +                       put_unaligned_le64((u64)plat->start,
> +                                          &dp->starting_address);
> +                       put_unaligned_le64((u64)(plat->start + plat->size - 1),
> +                                          &dp->ending_address);
> +                       guidcpy(&dp->disk_type_guid, &plat->disk_type_guid);
> +                       dp->disk_instance = 0;
> +                       return &dp[1];
> +               }
>  #endif
>                 default:
>                         debug("%s(%u) %s: unhandled parent class: %s (%u)\n",
> --
> 2.34.1
>

Regards,
Simon


More information about the U-Boot mailing list