All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] efi_driver: simplify efi_bl_bind()
@ 2021-09-13  6:01 AKASHI Takahiro
  2021-09-13  9:19 ` Heinrich Schuchardt
  2021-09-30  4:09 ` Simon Glass
  0 siblings, 2 replies; 4+ messages in thread
From: AKASHI Takahiro @ 2021-09-13  6:01 UTC (permalink / raw)
  To: xypron.glpk, agraf; +Cc: u-boot, AKASHI Takahiro

blk_create_devicef() is what blk_create_device() +
device_set_name_alloced() really does.
The resultant name will be a bit changed.

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;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] efi_driver: simplify efi_bl_bind()
  2021-09-13  6:01 [PATCH] efi_driver: simplify efi_bl_bind() AKASHI Takahiro
@ 2021-09-13  9:19 ` Heinrich Schuchardt
  2021-09-13  9:50   ` AKASHI Takahiro
  2021-09-30  4:09 ` Simon Glass
  1 sibling, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2021-09-13  9:19 UTC (permalink / raw)
  To: AKASHI Takahiro; +Cc: u-boot, agraf, Simon Glass

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.

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;
>


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] efi_driver: simplify efi_bl_bind()
  2021-09-13  9:19 ` Heinrich Schuchardt
@ 2021-09-13  9:50   ` AKASHI Takahiro
  0 siblings, 0 replies; 4+ messages in thread
From: AKASHI Takahiro @ 2021-09-13  9:50 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot, agraf, Simon Glass

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;
> > 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] efi_driver: simplify efi_bl_bind()
  2021-09-13  6:01 [PATCH] efi_driver: simplify efi_bl_bind() AKASHI Takahiro
  2021-09-13  9:19 ` Heinrich Schuchardt
@ 2021-09-30  4:09 ` Simon Glass
  1 sibling, 0 replies; 4+ messages in thread
From: Simon Glass @ 2021-09-30  4:09 UTC (permalink / raw)
  To: AKASHI Takahiro; +Cc: Heinrich Schuchardt, Alex Graf, U-Boot Mailing List

On Mon, 13 Sept 2021 at 00:02, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> blk_create_devicef() is what blk_create_device() +
> device_set_name_alloced() really does.
> The resultant name will be a bit changed.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_driver/efi_block_device.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-30  4:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13  6:01 [PATCH] efi_driver: simplify efi_bl_bind() AKASHI Takahiro
2021-09-13  9:19 ` Heinrich Schuchardt
2021-09-13  9:50   ` AKASHI Takahiro
2021-09-30  4:09 ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.