Re: [PATCH] efi_driver: simplify efi_bl_bind()

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Mon, Sep 13, 2021 at 11:19:22AM +0200, Heinrich Schuchardt wrote:
> On 9/13/21 8:01 AM, AKASHI Takahiro wrote:
> > blk_create_devicef() is what blk_create_device() +
> > device_set_name_alloced() really does.
> > The resultant name will be a bit changed.
> 
> without the patch after a device has been generated by the UEFI subsystem:
> 
> => dm tree
> 
> Class Index  Probed  Driver      Name
> 
> -----------------------------------------
> root      0  [ + ]   root_driver root_driver
> mmc       2  [ + ]   mmc_sandbox |-- mmc0
> 
> blk       2  [ + ]   mmc_blk     |   `-- mmc0.blk
> blk       3  [ + ]   efi_blk     `-- efiblk#0
> 
> with the patch:
> 
> => dm tree
> 
> Class Index  Probed  Driver      Name
> 
> ---------------------------------------------------------
> 
> root      0  [ + ]   root_driver root_driver
> mmc       2  [ + ]   mmc_sandbox |-- mmc0
> 
> blk       2  [ + ]   mmc_blk     |   `-- mmc0.blk
> blk       3  [ + ]   efi_blk     `-- root_driver.efiblk#0
> 
> Prepending 'root_driver.' to the name seems to not match the rest of the
> tree.

Is there anything wrong with the new "name"?

blk_create_devicef() creates a name with the parent's name + '.' +
a given name. In this case, you specifies the dm root object
as the device's parent. So "root_driver.efiblk#0" is a natural name
like mmc0.blk and others.

Another block device example:

 root          0  [ + ]   root_driver           root_driver
 pci           0  [ + ]   pci_generic_ecam      |-- pcie@10000000
 pci_generi    0  [   ]   pci_generic_drv       |   |-- pci_0:0.0
 virtio       32  [ + ]   virtio-pci.l          |   |-- virtio-pci.l#0
 ethernet      0  [ + ]   virtio-net            |   |   `-- virtio-net#32
 ahci          0  [ + ]   ahci_pci              |   `-- ahci_pci
 scsi          0  [ + ]   ahci_scsi             |       `-- ahci_scsi
 blk           0  [   ]   scsi_blk              |           |-- ahci_scsi.id0lun0
 blk           1  [   ]   scsi_blk              |           `-- ahci_scsi.id1lun0

If you really don't like "root_driver." prefix, you can create
the "efi" object as the generic parent for EFI devices, and then
you will see "efi.blk#0".
(This is also the way[1] that I took in my previous integration patch.

But in either way, it's just a name. Who cares.

-Takahiro Akashi

[1] https://lists.denx.de/pipermail/u-boot/2019-February/357929.html

> Best regards
> 
> Heinrich
> 
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   lib/efi_driver/efi_block_device.c | 10 +++-------
> >   1 file changed, 3 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> > index 0937e3595a43..b81c75868eb4 100644
> > --- a/lib/efi_driver/efi_block_device.c
> > +++ b/lib/efi_driver/efi_block_device.c
> > @@ -159,15 +159,11 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
> >   	sprintf(name, "efiblk#%d", devnum);
> > 
> >   	/* Create driver model udevice for the EFI block io device */
> > -	ret = blk_create_device(parent, "efi_blk", name, IF_TYPE_EFI, devnum,
> > -				io->media->block_size,
> > -				(lbaint_t)io->media->last_block, &bdev);
> > +	ret = blk_create_devicef(parent, "efi_blk", name, IF_TYPE_EFI, devnum,
> > +				 io->media->block_size,
> > +				 (lbaint_t)io->media->last_block, &bdev);
> >   	if (ret)
> >   		return ret;
> > -	if (!bdev)
> > -		return -ENOENT;
> > -	/* Set the DM_FLAG_NAME_ALLOCED flag to avoid a memory leak */
> > -	device_set_name_alloced(bdev);
> > 
> >   	plat = dev_get_plat(bdev);
> >   	plat->handle = handle;
> > 
>