[U-Boot] [RFC 3/3] drivers: align block device drivers with DM-efi integration
AKASHI Takahiro
takahiro.akashi at linaro.org
Wed Jan 30 00:40:12 UTC 2019
Alex,
On Tue, Jan 29, 2019 at 05:19:38PM +0100, Alexander Graf wrote:
> On 01/29/2019 03:59 AM, AKASHI Takahiro wrote:
> >Efi_disk_create() should be hook up at every creation of block device
> >at each driver. Associated blk_desc must be properly set up before
> >calling this function.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >---
> > common/usb_storage.c | 27 +++++++++++++++++++++++++--
> > drivers/scsi/scsi.c | 22 ++++++++++++++++++++++
> > lib/efi_driver/efi_block_device.c | 30 +++++++++---------------------
> > 3 files changed, 56 insertions(+), 23 deletions(-)
> >
> >diff --git a/common/usb_storage.c b/common/usb_storage.c
> >index 8c889bb1a648..ff895c0e4557 100644
> >--- a/common/usb_storage.c
> >+++ b/common/usb_storage.c
> >@@ -46,6 +46,10 @@
> > #include <part.h>
> > #include <usb.h>
> >+/* FIXME */
> >+extern int efi_disk_create(struct udevice *dev);
> >+extern int blk_create_partitions(struct udevice *parent);
> >+
> > #undef BBB_COMDAT_TRACE
> > #undef BBB_XPORT_TRACE
> >@@ -227,8 +231,27 @@ static int usb_stor_probe_device(struct usb_device *udev)
> > ret = usb_stor_get_info(udev, data, blkdev);
> > if (ret == 1) {
> >- usb_max_devs++;
> >- debug("%s: Found device %p\n", __func__, udev);
> >+ ret = efi_disk_create(dev);
> >+ if (ret) {
> >+ debug("Cannot create efi_disk device\n");
> >+ ret = device_unbind(dev);
> >+ if (ret)
> >+ return ret;
> >+ } else {
> >+ usb_max_devs++;
> >+ ret = blk_create_partitions(dev);
> >+ if (ret < 0) {
> >+ debug("Cannot create disk partition device\n");
> >+ /* TODO: undo create */
> >+
> >+ ret = device_unbind(dev);
> >+ if (ret)
> >+ return ret;
> >+ }
> >+ usb_max_devs += ret;
> >+ debug("%s: Found device %p, partitions:%d\n",
> >+ __func__, udev, ret);
> >+ }
>
> Why do we need to do this in device specific code?
Good point.
* efi_disk_create()
As I said in the past, efi_disk_create() will expect that blk_desc has
been initialized before it is called.
(There may be some possibility of removing this assumption.)
Blk_desc is often initialized after blk_create_device() in a driver.
So it won't be able to be placed in blk_create_device().
If, however, we have a "delayed" execution mechanism (like timer event as
you suggested before), we may put this function in a single point.
* blk_create_partions()
I initially thought of putting this function in part_init() which is
to be called at "probe," but was concerned that there might be a chance
that efi API be called before probing.
If this is not the case, we may also place this function in a single point.
(Unfortunately, "scsi rescan" won't kick off a probe hook though.)
Thanks,
-Takahiro Akashi
> Alex
>
>
> > } else {
> > debug("usb_stor_get_info: Invalid device\n");
> > ret = device_unbind(dev);
> >diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> >index df47e2fc78bd..f0f8cbc0bf26 100644
> >--- a/drivers/scsi/scsi.c
> >+++ b/drivers/scsi/scsi.c
> >@@ -11,6 +11,10 @@
> > #include <dm/device-internal.h>
> > #include <dm/uclass-internal.h>
> >+/* FIXME */
> >+int efi_disk_create(struct udevice *dev);
> >+int blk_create_partitions(struct udevice *parent);
> >+
> > #if !defined(CONFIG_DM_SCSI)
> > # ifdef CONFIG_SCSI_DEV_LIST
> > # define SCSI_DEV_LIST CONFIG_SCSI_DEV_LIST
> >@@ -593,9 +597,27 @@ static int do_scsi_scan_one(struct udevice *dev, int id, int lun, bool verbose)
> > memcpy(&bdesc->product, &bd.product, sizeof(bd.product));
> > memcpy(&bdesc->revision, &bd.revision, sizeof(bd.revision));
> >+ ret = efi_disk_create(bdev);
> >+ if (ret) {
> >+ debug("Can't create efi_disk device\n");
> >+ ret = device_unbind(bdev);
> >+
> >+ return ret;
> >+ }
> >+ ret = blk_create_partitions(bdev);
> >+ if (ret < 0) {
> >+ debug("Can't create efi_disk partitions\n");
> >+ /* TODO: undo create */
> >+
> >+ ret = device_unbind(bdev);
> >+
> >+ return ret;
> >+ }
> >+
> > if (verbose) {
> > printf(" Device %d: ", 0);
> > dev_print(bdesc);
> >+ debug("Found %d partitions\n", ret);
> > }
> > return 0;
> > }
> >diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> >index 3f147cf60879..4ab3402d6768 100644
> >--- a/lib/efi_driver/efi_block_device.c
> >+++ b/lib/efi_driver/efi_block_device.c
> >@@ -32,6 +32,10 @@
> > #include <dm/device-internal.h>
> > #include <dm/root.h>
> >+/* FIXME */
> >+extern int efi_disk_create(struct udevice *dev);
> >+extern int blk_create_partitions(struct udevice *parent);
> >+
> > /*
> > * EFI attributes of the udevice handled by this driver.
> > *
> >@@ -102,24 +106,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> > return blkcnt;
> > }
> >-/*
> >- * Create partions for the block device.
> >- *
> >- * @handle EFI handle of the block device
> >- * @dev udevice of the block device
> >- */
> >-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
> >-{
> >- struct blk_desc *desc;
> >- const char *if_typename;
> >-
> >- desc = dev_get_uclass_platdata(dev);
> >- if_typename = blk_get_if_type_name(desc->if_type);
> >-
> >- return efi_disk_create_partitions(handle, desc, if_typename,
> >- desc->devnum, dev->name);
> >-}
> >-
> > /*
> > * Create a block device for a handle
> > *
> >@@ -168,15 +154,17 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
> > platdata->handle = handle;
> > platdata->io = interface;
> >+ ret = efi_disk_create(bdev);
> >+ if (ret)
> >+ return ret;
> >+
> > ret = device_probe(bdev);
> > if (ret)
> > return ret;
> > EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
> >-
> > /* Create handles for the partions of the block device */
> >- disks = efi_bl_bind_partitions(handle, bdev);
> >+ disks = blk_create_partitions(bdev);
> > EFI_PRINT("Found %d partitions\n", disks);
> >-
> > return 0;
> > }
>
>
More information about the U-Boot
mailing list