> On 08 Aug 2016, at 23:44, Simon Glass <s...@chromium.org> wrote:
> 
> Hi Alexander,
> 
> On 5 August 2016 at 06:49, Alexander Graf <ag...@suse.de> wrote:
>> When using CONFIG_BLK, there were 2 issues:
>> 
>>  1) The name we generate the device with has to match the
>>     name we set in efi_set_bootdev()
>> 
>>  2) The device we pass into our block functions was wrong,
>>     we should not rediscover it but just use the already known
>>     pointer.
>> 
>> This patch fixes both issues.
>> 
>> Signed-off-by: Alexander Graf <ag...@suse.de>
>> ---
>> cmd/bootefi.c             | 23 ++++++++++++++++++-----
>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>> 2 files changed, 29 insertions(+), 12 deletions(-)
>> 
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index b8ecf4c..53a6ee3 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -8,6 +8,7 @@
>> 
>> #include <common.h>
>> #include <command.h>
>> +#include <dm/device.h>
>> #include <efi_loader.h>
>> #include <errno.h>
>> #include <libfdt.h>
>> @@ -269,18 +270,30 @@ void efi_set_bootdev(const char *dev, const char 
>> *devnr, const char *path)
>>        char devname[32] = { 0 }; /* dp->str is u16[32] long */
>>        char *colon;
>> 
>> -       /* Assemble the condensed device name we use in efi_disk.c */
>> -       snprintf(devname, sizeof(devname), "%s%s", dev, devnr);
>> +#if defined(CONFIG_BLK) || defined(CONFIG_ISO_PARTITION)
>> +       desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));
>> +#endif
>> +
>> +#ifdef CONFIG_BLK
>> +       if (desc) {
>> +               snprintf(devname, sizeof(devname), "%s", desc->bdev->name);
>> +       } else
>> +#endif
>> +
>> +       {
>> +               /* Assemble the condensed device name we use in efi_disk.c */
>> +               snprintf(devname, sizeof(devname), "%s%s", dev, devnr);
>> +       }
>> +
>>        colon = strchr(devname, ':');
>> 
>> #ifdef CONFIG_ISO_PARTITION
>>        /* For ISOs we create partition block devices */
>> -       desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));
>>        if (desc && (desc->type != DEV_TYPE_UNKNOWN) &&
>>            (desc->part_type == PART_TYPE_ISO)) {
>>                if (!colon)
>> -                       snprintf(devname, sizeof(devname), "%s%s:1", dev,
>> -                                devnr);
>> +                       snprintf(devname, sizeof(devname), "%s:1", devname);
>> +
>>                colon = NULL;
>>        }
>> #endif
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index c434c92..e00a747 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -31,6 +31,8 @@ struct efi_disk_obj {
>>        struct efi_device_path_file_path *dp;
>>        /* Offset into disk for simple partitions */
>>        lbaint_t offset;
>> +       /* Internal block device */
>> +       const struct blk_desc *desc;
> 
> Rather than storing this, can you store the udevice?

I could, but then I would diverge between the CONFIG_BLK and non-CONFIG_BLK 
path again, which would turn the code into an #ifdef mess (read: hard to 
maintain), because the whole device creation path relies on struct blk_desc * 
today and doesn’t pass the udevice anywhere.

Do you feel strongly about this? To give you an idea how messy it gets, the 
diff is below.


Alex


diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d8ddcc9..79d313b 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -32,7 +32,11 @@ struct efi_disk_obj {
        /* Offset into disk for simple partitions */
        lbaint_t offset;
        /* Internal block device */
+#ifdef CONFIG_BLK
+       struct udevice *dev;
+#else
        const struct blk_desc *desc;
+#endif
 };

 static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol,
@@ -69,6 +73,15 @@ enum efi_disk_direction {
        EFI_DISK_WRITE,
 };

+static struct blk_desc *diskobj2desc(struct efi_disk_obj *diskobj)
+{
+#ifdef CONFIG_BLK
+       return dev_get_uclass_platdata(diskobj->dev);
+#else
+       return (struct blk_desc *)diskobj->desc;
+#endif
+}
+
 static efi_status_t EFIAPI efi_disk_rw_blocks(struct efi_block_io *this,
                        u32 media_id, u64 lba, unsigned long buffer_size,
                        void *buffer, enum efi_disk_direction direction)
@@ -80,7 +93,7 @@ static efi_status_t EFIAPI efi_disk_rw_blocks(struct 
efi_block_io *this,
        unsigned long n;

        diskobj = container_of(this, struct efi_disk_obj, ops);
-       desc = (struct blk_desc *) diskobj->desc;
+       desc = diskobj2desc(diskobj);
        blksz = desc->blksz;
        blocks = buffer_size / blksz;
        lba += diskobj->offset;
@@ -194,10 +207,17 @@ static const struct efi_block_io block_io_disk_template = 
{

 static void efi_disk_add_dev(const char *name,
                             const char *if_typename,
+#ifdef CONFIG_BLK
+                            struct udevice *dev,
+#else
                             const struct blk_desc *desc,
+#endif
                             int dev_index,
                             lbaint_t offset)
 {
+#ifdef CONFIG_BLK
+       struct blk_desc *desc = dev_get_uclass_platdata(dev);
+#endif
        struct efi_disk_obj *diskobj;
        struct efi_device_path_file_path *dp;
        int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
@@ -218,7 +238,11 @@ static void efi_disk_add_dev(const char *name,
        diskobj->ifname = if_typename;
        diskobj->dev_index = dev_index;
        diskobj->offset = offset;
+#ifdef CONFIG_BLK
+       diskobj->dev = dev;
+#else
        diskobj->desc = desc;
+#endif

        /* Fill in EFI IO Media info (for read/write callbacks) */
        diskobj->media.removable_media = desc->removable;
@@ -244,13 +268,19 @@ static void efi_disk_add_dev(const char *name,
        list_add_tail(&diskobj->parent.link, &efi_obj_list);
 }

-static int efi_disk_create_eltorito(struct blk_desc *desc,
-                                   const char *if_typename,
-                                   int diskid,
-                                   const char *pdevname)
+static int efi_disk_create_eltorito(
+#ifdef CONFIG_BLK
+               struct udevice *dev,
+#else
+               struct blk_desc *desc,
+#endif
+               const char *if_typename, int diskid, const char *pdevname)
 {
        int disks = 0;
 #ifdef CONFIG_ISO_PARTITION
+#ifdef CONFIG_BLK
+       struct blk_desc *desc = dev_get_uclass_platdata(dev);
+#endif
        char devname[32] = { 0 }; /* dp->str is u16[32] long */
        disk_partition_t info;
        int part = 1;
@@ -261,8 +291,13 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
        while (!part_get_info(desc, part, &info)) {
                snprintf(devname, sizeof(devname), "%s:%d", pdevname,
                         part);
+#ifdef CONFIG_BLK
+               efi_disk_add_dev(devname, if_typename, dev, diskid,
+                                info.start);
+#else
                efi_disk_add_dev(devname, if_typename, desc, diskid,
                                 info.start);
+#endif
                part++;
                disks++;
        }
@@ -295,14 +330,14 @@ int efi_disk_register(void)
                const char *if_typename = dev->driver->name;

                printf("Scanning disk %s...\n", dev->name);
-               efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
+               efi_disk_add_dev(dev->name, if_typename, dev, desc->devnum, 0);
                disks++;

                /*
                * El Torito images show up as block devices in an EFI world,
                * so let's create them here
                */
-               disks += efi_disk_create_eltorito(desc, if_typename,
+               disks += efi_disk_create_eltorito(dev, if_typename,
                                                  desc->devnum, dev->name);
        }
 #else
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to