On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote:
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro takahiro.akashi@linaro.org wrote:
Add efi_disk_create() function.
Any UEFI handle created by efi_disk_create() can be treated as a efi_disk object, the udevice is either a UCLASS_BLK (a whole raw disk) or UCLASS_PARTITION (a disk partition).
So this function is expected to be called every time such an udevice is detected and activated through a device model's "probe" interface.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
include/efi_loader.h | 2 + lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But some nits below.
Don't worry about !CONFIG_BLK - that code should be removed.
Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk in patch#13.
diff --git a/include/efi_loader.h b/include/efi_loader.h index c440962fe522..751fde7fb153 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, void efi_carve_out_dt_rsv(void *fdt); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); +/* Called when a block devices has been probed */ +int efi_disk_create(struct udevice *dev);
Please buck the trend in this file and add a full function comment. In this case it needs to cover @dev and the return value and also explain what the function does.
OK.
/* Called by bootefi to make all disk storage accessible as EFI objects */ efi_status_t efi_disk_register(void); /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index cd5528046251..3fae40e034fb 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -10,6 +10,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <dm/device-internal.h> #include <efi_loader.h> #include <fs.h> #include <log.h> @@ -484,6 +485,7 @@ error: return ret; }
+#ifndef CONFIG_BLK /**
- efi_disk_create_partitions() - create handles and protocols for partitions
@@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
return disks;} +#endif /* CONFIG_BLK */
+/*
- Create a handle for a whole raw disk
- @dev uclass device
?? what type of device?
(Will fix: UCLASS_BLK)
- @return 0 on success, -1 otherwise
- */
+static int efi_disk_create_raw(struct udevice *dev) +{
struct efi_disk_obj *disk;struct blk_desc *desc;const char *if_typename;int diskid;efi_status_t ret;desc = dev_get_uclass_plat(dev);if_typename = blk_get_if_type_name(desc->if_type);diskid = desc->devnum;ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,diskid, NULL, 0, &disk);if (ret != EFI_SUCCESS) {if (ret)
is much shorter and easier to read
Yeah, but I don't want to assume EFI_SUCCESS is *zero*.
log_err("Adding disk %s%d failed\n", if_typename, diskid);return -1;}disk->dev = dev;dev->efi_obj = &disk->header;return 0;+}
+/*
- Create a handle for a disk partition
- @dev uclass device
??
(UCLASS_PARTITION)
- @return 0 on success, -1 otherwise
- */
+static int efi_disk_create_part(struct udevice *dev) +{
efi_handle_t parent;struct blk_desc *desc;const char *if_typename;struct disk_part *part_data;struct disk_partition *info;unsigned int part;int diskid;struct efi_device_path *dp = NULL;struct efi_disk_obj *disk;efi_status_t ret;parent = dev->parent->efi_obj;dev_get_parent(dev)
I will replace all of "dev->parent" with dev_get_parent() if I will not forget to do that in the next version :)
Thanks, -Takahiro Akashi
desc = dev_get_uclass_plat(dev->parent);if_typename = blk_get_if_type_name(desc->if_type);diskid = desc->devnum;part_data = dev_get_uclass_plat(dev);part = part_data->partnum;info = &part_data->gpt_part_info;/* TODO: should not use desc? */dp = efi_dp_from_part(desc, 0);ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,info, part, &disk);if (ret != EFI_SUCCESS) {log_err("Adding partition %s%d:%x failed\n",if_typename, diskid, part);return -1;}disk->dev = dev;dev->efi_obj = &disk->header;return 0;+}
+int efi_disk_create(struct udevice *dev) +{
enum uclass_id id;id = device_get_uclass_id(dev);if (id == UCLASS_BLK)return efi_disk_create_raw(dev);if (id == UCLASS_PARTITION)return efi_disk_create_part(dev);return -1;+}
/**
- efi_disk_register() - register block devices
-- 2.33.0
Regards, Simon