[PATCH v2 00/24] blk: Rationalise the block interface
Johan Jonker
jbx6244 at gmail.com
Fri Aug 12 15:51:34 CEST 2022
Hi Simon and others,
Some comments. Have a look if it's useful.
Include is an example of how currently a new Rockchip external RKNAND FTL block device must be included in U-boot.
Changes must be made all over the place. EFI has now become a somewhat "obligation" for block devices, then handling
should be made more easy I propose.
1:
The creation and removal of block devices is kind of dynamic with the blk_create_device function.
Is it possible to make the necessary functions in efi_device_path.c and part.c more flexible as well by
a new "blk_create_efi_device()" and "blk_remove_efi_device()" function? So everything combined in one function.
2:
Make the class list more dynamic/expendable. Each time a new clas is needed the source code must be changed.
Include a function that returns a generated class number that takes in account the existing know classes.
ie,: "uclass_create" and "uclass_remove".
Example block device creation:
ret = uclass_get_device(UCLASS_RK_IDB, 0, &dev);
// Be able to use a dynamic generated UCLASS for block devices.
if (ret) {
printf("no IDB device found\n");
return CMD_RET_FAILURE;
}
ret = blk_get_device(IF_TYPE_RK_IDB, 0, &bdev);
if (ret) {
ret = blk_create_device(dev, "idb_blk", "blk",
IF_TYPE_RK_IDB, 0, 512,
LBA, &bdev);
if (ret)
return ret;
ret = blk_probe_or_unbind(bdev);
if (ret)
return ret;
}
Example block device removal:
ret = blk_find_device(IF_TYPE_RK_IDB, 0, &bdev);
// Be able to find back a dynamic generated UCLASS.
if (ret) {
printf("no IDB blk device found\n");
return 0;
}
device_remove(bdev, DM_REMOVE_NORMAL);
device_unbind(bdev);
Make efi functions more flexible by replacing switch() functions by call back functions for:
dp_fill
dp_size
dev_print
print_part_header
Add them with "blk_create_efi_device()" in a structure.
If not defined fall back to standard functions.
====
Kind regards,
Johan Jonker
On 8/12/22 03:34, Simon Glass wrote:
> The block interface has two separate implementations, one using driver
> model and one not. The latter is really only needed for SPL, where
> size constraints allegedly don't allow use of driver model. Of course
> we still need space for filesystems and other code, so it isn't clear
> that driver model is anything more than the straw that breaks the
> camel's back.
>
> The driver model version uses a uclass ID for the interface time, but
> converts back and forth between that and if_type, which is the legacy
> type.
>
> The HAVE_BLOCK_DEVICE define is mostly a hangover from the old days.
> At present its main purpose is to enable the legacy block implementation
> in SPL.
>
> Finally the use of 'select' to enable BLK does not work very well. It
> causes kconfig errors when another option depends on BLK and it is
> not recommended by the kconfig style guide.
>
> This series aims to clean things up:
> - Enable BLK based on whether different media types are used, but still
> allow boards to disable it
> - Rename HAVE_BLOCK_DEVICE to indicates its real purpose
> - Drop if_type and use the uclass instead
> - Drop some obsolete if_type values
>
> An issue not resolved by this series is that the sandbox host interface
> does not actually have a device. At present it uses the root device, which
> was convenience for the driver model conversion but not really correct. It
> should be possible to clean this up, in a future series.
>
> Another minor issue is the use of UCLASS_USB for a mass-storage device.
> This has been the case for a while and is not addresed by this series,
> other than to add a comment.
>
> Note that this test relies on Tom Rini's series to drop various boards
> including warp and cm_t335
>
> Finally, a patch is included to make binman put fake files in a
> subdirectory, since repeated runs of certain boards can cause unrelated
> failues (e.g. chromebook_coral) when fake files are left around.
>
> Changes in v2:
> - Update commit message
> - Fix SPL_PARTITIONS too
> - Add SATA also
> - Refer to a suffix, not a prefix
> - Add new patch to handle UCLASS_EFI_MEDIA in dev_print()
> - Add new patch to drop ifname field from struct efi_disk_obj
> - Use conv_uclass_id() instead of the confusing uclass_id_to_uclass_id()
>
>From d0bb794b966a0134a6f321a414b48c28e8894180 Mon Sep 17 00:00:00 2001
From: Johan Jonker <jbx6244 at gmail.com>
Date: Sun, 7 Aug 2022 15:25:55 +0200
Subject: prepare rknand
diff --git a/disk/part.c b/disk/part.c
index 79955c7fb0..3f8b97a6c6 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -150,6 +150,7 @@ void dev_print (struct blk_desc *dev_desc)
case IF_TYPE_USB:
case IF_TYPE_NVME:
case IF_TYPE_PVBLOCK:
+ case IF_TYPE_RKNAND:
case IF_TYPE_HOST:
printf ("Vendor: %s Rev: %s Prod: %s\n",
dev_desc->vendor,
@@ -293,6 +294,9 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc)
case IF_TYPE_PVBLOCK:
puts("PV BLOCK");
break;
+ case IF_TYPE_RKNAND:
+ puts("RKNAND");
+ break;
case IF_TYPE_VIRTIO:
puts("VirtIO");
break;
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 0b5f219d90..28b21836c4 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -33,6 +33,7 @@ static const char *if_typename_str[IF_TYPE_COUNT] = {
[IF_TYPE_VIRTIO] = "virtio",
[IF_TYPE_PVBLOCK] = "pvblock",
[IF_TYPE_RK_IDB] = "idb",
+ [IF_TYPE_RKNAND] = "rknand",
};
static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
@@ -51,6 +52,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
[IF_TYPE_VIRTIO] = UCLASS_VIRTIO,
[IF_TYPE_PVBLOCK] = UCLASS_PVBLOCK,
[IF_TYPE_RK_IDB] = UCLASS_RK_IDB,
+ [IF_TYPE_RKNAND] = UCLASS_RKNAND,
};
static enum if_type if_typename_to_iftype(const char *if_typename)
diff --git a/include/blk.h b/include/blk.h
index a73cc577a0..56f2415e21 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -30,6 +30,7 @@ enum if_type {
IF_TYPE_USB,
IF_TYPE_DOC,
IF_TYPE_MMC,
+ IF_TYPE_RKNAND,
IF_TYPE_SD,
IF_TYPE_SATA,
IF_TYPE_HOST,
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 38a227f006..b102cdf46e 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -104,6 +104,7 @@ enum uclass_id {
UCLASS_REGULATOR, /* Regulator device */
UCLASS_REMOTEPROC, /* Remote Processor device */
UCLASS_RESET, /* Reset controller device */
+ UCLASS_RKNAND, /* Rockchip nand device with ftl */
UCLASS_RK_IDB, /* Rockchip IDB device */
UCLASS_RNG, /* Random Number Generator */
UCLASS_RTC, /* Real time clock device */
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 44d426035a..ddc7082ad6 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -145,6 +145,10 @@ static inline efi_status_t efi_launch_capsules(void)
#define U_BOOT_IDB_DEV_GUID \
EFI_GUID(0xadc021df, 0x5f24, 0x464f, \
0x9a, 0x88, 0xdb, 0xee, 0x3f, 0x1d, 0x14, 0x0f)
+/* GUID used as root for Rockchip RKNAND devices */
+#define U_BOOT_RKNAND_DEV_GUID \
+ EFI_GUID(0xe39f6cbb, 0x055a, 0x45a0, \
+ 0xb2, 0x75, 0x56, 0x0d, 0xa5, 0x22, 0x25, 0x99)
/* Use internal device tree when starting UEFI application */
#define EFI_FDT_USE_INTERNAL NULL
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index b7535373e7..9691f02b2e 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -31,6 +31,9 @@ const efi_guid_t efi_guid_virtio_dev = U_BOOT_VIRTIO_DEV_GUID;
#if CONFIG_IS_ENABLED(ROCKCHIP_IDB)
const efi_guid_t efi_guid_idb_dev = U_BOOT_IDB_DEV_GUID;
#endif
+#if CONFIG_IS_ENABLED(RKNAND)
+const efi_guid_t efi_guid_rknand_dev = U_BOOT_IDB_DEV_GUID;
+#endif
/* template END node: */
static const struct efi_device_path END = {
@@ -578,6 +581,16 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_vendor) + 1;
#endif
+#if CONFIG_IS_ENABLED(RKNAND)
+ case UCLASS_RKNAND:
+ /*
+ * Rockchip IDB device will be represented
+ * as vendor device with extra one byte for
+ * device number
+ */
+ return dp_size(dev->parent)
+ + sizeof(struct efi_device_path_vendor) + 1;
+#endif
#if CONFIG_IS_ENABLED(ROCKCHIP_IDB)
case UCLASS_RK_IDB:
/*
@@ -680,6 +693,23 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
return &dp->vendor_data[1];
}
#endif
+#if CONFIG_IS_ENABLED(RKNAND)
+ case UCLASS_RKNAND: {
+ struct efi_device_path_vendor *dp;
+ struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+ dp_fill(buf, dev->parent);
+ dp = buf;
+ ++dp;
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ dp->dp.length = sizeof(*dp) + 1;
+ memcpy(&dp->guid, &efi_guid_rknand_dev,
+ sizeof(efi_guid_t));
+ dp->vendor_data[0] = desc->devnum;
+ return &dp->vendor_data[1];
+ }
+#endif
#if CONFIG_IS_ENABLED(ROCKCHIP_IDB)
case UCLASS_RK_IDB: {
struct efi_device_path_vendor *dp;
More information about the U-Boot
mailing list