Hi Heinrich,

On Tue, Oct 04, 2022 at 04:29:25PM +0200, Heinrich Schuchardt wrote:

• Carve out function efi_bl_create_block_device() from efi_bl_bind().
• Add a check for U-Boot devices to efi_bl_bind().

What the code does is obvious here, can we slightly change the commit message and explain why this is needed? Readability?

Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

lib/efi_driver/efi_block_device.c | 32 ++++++++++++++++++++++++------- lib/efi_driver/efi_uclass.c | 9 --------- 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c index 9ccc148590..3103040870 100644 --- a/lib/efi_driver/efi_block_device.c +++ b/lib/efi_driver/efi_block_device.c @@ -114,21 +114,16 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,

• @interface: block io protocol
• Return: status code

*/ -static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) +static efi_status_t +efi_bl_create_block_device(efi_handle_t handle, void *interface) { struct udevice *bdev = NULL, *parent = dm_root(); efi_status_t ret; int devnum; char *name;

• struct efi_object *obj = efi_search_obj(handle); struct efi_block_io *io = interface; struct efi_blk_plat *plat;

• EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);

• if (!obj || !interface)

• return EFI_INVALID_PARAMETER;
  

• devnum = blk_find_max_devnum(UCLASS_EFI_LOADER); if (devnum == -ENODEV) devnum = 0;

@@ -176,6 +171,29 @@ err: return ret; }

+/**

• • efi_bl_bind() - bind to a block io protocol
• • @handle: handle
• • @interface: block io protocol
• • Return: status code
• */

+static efi_status_t efi_bl_bind(efi_handle_t handle, void *interface) +{

• efi_status_t ret;
• struct efi_object *obj = efi_search_obj(handle);
• EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, interface);
• if (!obj || !interface)

• return EFI_INVALID_PARAMETER;
  

• if (!handle->dev)

• ret = efi_bl_create_block_device(handle, interface);
  

Keeping in mind devices should usually be bound once (unless we reinit the subsystem) should we print something here? Also ret need to be initialized on the declaration.

• return ret;

+}

/* Block device driver operators */ static const struct blk_ops efi_blk_ops = { .read = efi_bl_read, -- 2.37.2

Thanks /Ilias