> 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