Hi Heinrich, Akashi-san,
On Wed, 20 Jul 2022 at 16:42, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 7/20/22 07:23, Takahiro Akashi wrote:
On Sun, Jul 17, 2022 at 01:23:41PM +0200, Heinrich Schuchardt wrote:
On 7/17/22 10:09, Heinrich Schuchardt wrote:
On 7/15/22 16:47, Masahisa Kojima wrote:
This is a preparation patch to provide the unified method to access udevice pointer associated with the block io device. The EFI handles of both EFI block io driver implemented in lib/efi_loader/efi_disk.c and EFI block io driver implemented as EFI payload can posess the udevice pointer in the struct efi_object.
We can use this udevice pointer to get the U-Boot friendly block device name(e.g. mmc 0:1, nvme 0:1) through efi_handle_t.
Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org
Newly created in v9
include/efi_loader.h | 8 ++++++++ lib/efi_loader/efi_disk.c | 20 +++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 3a63a1f75f..bba5ffd482 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -226,6 +226,12 @@ const char *__efi_nesting_dec(void); #define EFI_CACHELINE_SIZE 128 #endif
+/**
- efi_handle_to_udev - accessor to the DM device associated to the
EFI handle
- @handle: pointer to the EFI handle
- */
+#define efi_handle_to_udev(handle) (((struct efi_object *)handle)->dev)
This conversion will hide errors if handle is not of type efi_handle_t. We should avoid the conversion and see build time errors instead. Please, remove the macro.
For every handle of type efi_handle_t you can access the field handle->dev directly.
For struct efi_disk_obj we can use disk->header.dev.
- /* Key identifying current memory map */ extern efi_uintn_t efi_memory_map_key;
@@ -375,6 +381,7 @@ enum efi_object_type { * @protocols: linked list with the protocol interfaces installed on this * handle * @type: image type if the handle relates to an image
- @dev: pointer to the DM device which is associated with this
EFI handle * * UEFI offers a flexible and expandable object model. The objects in the UEFI * API are devices, drivers, and loaded images. struct efi_object is our storage @@ -392,6 +399,7 @@ struct efi_object { /* The list of protocols */ struct list_head protocols; enum efi_object_type type;
struct udevice *dev; };
enum efi_image_auth_status {
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 1d700b2a6b..a8e8521e3e 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -46,7 +46,6 @@ struct efi_disk_obj { struct efi_device_path *dp; unsigned int part; struct efi_simple_file_system_protocol *volume;
- struct udevice *dev; /* TODO: move it to efi_object */
ok
};
/** @@ -124,16 +123,16 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, return EFI_BAD_BUFFER_SIZE;
if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {
device_get_uclass_id(efi_handle_to_udev(diskobj)) ==UCLASS_PARTITION) {
device_get_uclass_id(diskobj->header.hdev)) == UCLASS_PARTITION) {
if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);
n = dev_read(efi_handle_to_udev(diskobj), lba, blocks,buffer);
dev_read(diskobj->header.hdev)
else
n = dev_write(diskobj->dev, lba, blocks, buffer);
n = dev_write(efi_handle_to_udev(diskobj), lba, blocks,buffer);
dev_write(diskobj->header.hdev)
} else { /* dev is a block device (UCLASS_BLK) */ struct blk_desc *desc;
desc = dev_get_uclass_plat(diskobj->dev);
desc = dev_get_uclass_plat(efi_handle_to_udev(diskobj));dev_get_uclass(diskobj->header.hdev)
if (direction == EFI_DISK_READ) n = blk_dread(desc, lba, blocks, buffer); else@@ -552,7 +551,7 @@ static int efi_disk_create_raw(struct udevice *dev)
return -1; }
- disk->dev = dev;
- efi_handle_to_udev(disk) = dev; if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) { efi_free_pool(disk->dp); efi_delete_handle(&disk->header);
@@ -609,7 +608,7 @@ static int efi_disk_create_part(struct udevice *dev) log_err("Adding partition for %s failed\n", dev->name); return -1; }
- disk->dev = dev;
- efi_handle_to_udev(disk) = dev;
disk->header.dev = dev;
if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) { efi_free_pool(disk->dp); efi_delete_handle(&disk->header);@@ -656,6 +655,13 @@ static int efi_disk_probe(void *ctx, struct event *event) ret = efi_disk_create_raw(dev); if (ret) return -1;
- } else {
efi_handle_t handle;if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))Setting handle->dev can be done more easily in efi_bl_bind():
I don't think so. "dev" field should be maintained in *one* place, i.e. efi_disk_probe(), which is uniquely called from probe event (even for efi_block_dev case).
To make this clear, we'd better put the code in efi_disk_create_raw(): efi_disk_create_raw() if (desc->if_type == IF_TYPE_EFI_LOADER) { /* handle should exist now */ dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)); efi_set_udev(handle, dev); return 0; }
/* normal block devices */ ... efi_set_udev(&disk->header, dev); ...Linking devices and handles is not block device specific.
If we would properly link all devices and handles, we would be able to generate device paths by walking the driver model device tree. This is the direction into which our future development should go.
Can we have a wrapper around dev_tag_set_ptr(dev, DM_TAG_EFI, handle) which takes care of setting handle->dev and the tag replacing all current invocations:
int efi_link_dev(efi_handle_t handle, udevice dev) { handle->dev = dev; return dev_tag_set_ptr(dev, DM_TAG_EFI, handle); }
Thank you. I create efi_link_dev().
We can further remove the field handle from struct blk_create_device as it is now available in handle->dev.
Do you mean struct efi_blk_plat?
struct efi_blk_plat {
.......efi_handle_t>...>.......handle; .......struct efi_block_io>....*io;
};
Regards, Masahisa Kojima
Best regards
Heinrich
-Takahiro Akashi
handle->dev = bdev;
We can further remove the field handle from struct blk_create_device as it is now available in handle->dev.
Best regards
Heinrich
return -1;efi_handle_to_udev(handle) = dev;handle->dev = dev;
Best regards
Heinrich
} device_foreach_child(child, dev) {