| Message ID | 20210913060147.18761-1-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_driver: simplify efi_bl_bind() | expand |
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; >
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; > > >
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>
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;
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(-) -- 2.33.0