[PATCH v2 2/6] ramdisk: add ramdisk uclass and driver
Masahisa Kojima
masahisa.kojima at linaro.org
Thu Jul 20 02:21:45 CEST 2023
Hi Simon,
On Thu, 20 Jul 2023 at 04:11, Simon Glass <sjg at chromium.org> wrote:
>
> 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
Thank you for your review.
This patch is duplicated, I can use blkmap. Please drop this series.
Thanks,
Masahisa Kojima
>
> > @@ -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