Hi Masahisa, On Thu, 13 Jul 2023 at 23:47, Masahisa Kojima <masahisa.koj...@linaro.org> wrote: > > This introcudes the ramdisk uclass and driver. > > Signed-off-by: Masahisa Kojima <masahisa.koj...@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