* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
@ 2016-08-05 12:49 Alexander Graf
2016-08-08 21:44 ` Simon Glass
2016-08-10 0:46 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 19+ messages in thread
From: Alexander Graf @ 2016-08-05 12:49 UTC (permalink / raw)
To: u-boot
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 <agraf@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;
};
static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol,
@@ -78,8 +80,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);
- if (!(desc = blk_get_dev(diskobj->ifname, diskobj->dev_index)))
- return EFI_EXIT(EFI_DEVICE_ERROR);
+ desc = (struct blk_desc *) diskobj->desc;
blksz = desc->blksz;
blocks = buffer_size / blksz;
lba += diskobj->offset;
@@ -213,6 +214,7 @@ static void efi_disk_add_dev(const char *name,
diskobj->ifname = if_typename;
diskobj->dev_index = dev_index;
diskobj->offset = offset;
+ diskobj->desc = desc;
/* Fill in EFI IO Media info (for read/write callbacks) */
diskobj->media.removable_media = desc->removable;
@@ -240,7 +242,8 @@ static void efi_disk_add_dev(const char *name,
static int efi_disk_create_eltorito(struct blk_desc *desc,
const char *if_typename,
- int diskid)
+ int diskid,
+ const char *pdevname)
{
int disks = 0;
#ifdef CONFIG_ISO_PARTITION
@@ -252,8 +255,8 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
return 0;
while (!part_get_info(desc, part, &info)) {
- snprintf(devname, sizeof(devname), "%s%d:%d", if_typename,
- diskid, part);
+ snprintf(devname, sizeof(devname), "%s:%d", pdevname,
+ part);
efi_disk_add_dev(devname, if_typename, desc, diskid,
info.start);
part++;
@@ -296,7 +299,7 @@ int efi_disk_register(void)
* so let's create them here
*/
disks += efi_disk_create_eltorito(desc, if_typename,
- desc->devnum);
+ desc->devnum, dev->name);
}
#else
int i, if_type;
@@ -331,7 +334,8 @@ int efi_disk_register(void)
* 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, i);
+ disks += efi_disk_create_eltorito(desc, if_typename,
+ i, devname);
}
}
#endif
--
2.6.6
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-05 12:49 [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage Alexander Graf
@ 2016-08-08 21:44 ` Simon Glass
2016-08-10 1:43 ` Tom Rini
2016-08-10 7:47 ` Alexander Graf
2016-08-10 0:46 ` [U-Boot] " Tom Rini
1 sibling, 2 replies; 19+ messages in thread
From: Simon Glass @ 2016-08-08 21:44 UTC (permalink / raw)
To: u-boot
Hi Alexander,
On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@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?
> };
>
> static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol,
> @@ -78,8 +80,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);
> - if (!(desc = blk_get_dev(diskobj->ifname, diskobj->dev_index)))
> - return EFI_EXIT(EFI_DEVICE_ERROR);
> + desc = (struct blk_desc *) diskobj->desc;
> blksz = desc->blksz;
> blocks = buffer_size / blksz;
> lba += diskobj->offset;
> @@ -213,6 +214,7 @@ static void efi_disk_add_dev(const char *name,
> diskobj->ifname = if_typename;
> diskobj->dev_index = dev_index;
> diskobj->offset = offset;
> + diskobj->desc = desc;
>
> /* Fill in EFI IO Media info (for read/write callbacks) */
> diskobj->media.removable_media = desc->removable;
> @@ -240,7 +242,8 @@ static void efi_disk_add_dev(const char *name,
>
> static int efi_disk_create_eltorito(struct blk_desc *desc,
> const char *if_typename,
> - int diskid)
> + int diskid,
> + const char *pdevname)
> {
> int disks = 0;
> #ifdef CONFIG_ISO_PARTITION
> @@ -252,8 +255,8 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
> return 0;
>
> while (!part_get_info(desc, part, &info)) {
> - snprintf(devname, sizeof(devname), "%s%d:%d", if_typename,
> - diskid, part);
> + snprintf(devname, sizeof(devname), "%s:%d", pdevname,
> + part);
> efi_disk_add_dev(devname, if_typename, desc, diskid,
> info.start);
> part++;
> @@ -296,7 +299,7 @@ int efi_disk_register(void)
> * so let's create them here
> */
> disks += efi_disk_create_eltorito(desc, if_typename,
> - desc->devnum);
> + desc->devnum, dev->name);
> }
> #else
> int i, if_type;
> @@ -331,7 +334,8 @@ int efi_disk_register(void)
> * 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, i);
> + disks += efi_disk_create_eltorito(desc, if_typename,
> + i, devname);
> }
> }
> #endif
> --
> 2.6.6
>
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-05 12:49 [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage Alexander Graf
2016-08-08 21:44 ` Simon Glass
@ 2016-08-10 0:46 ` Tom Rini
1 sibling, 0 replies; 19+ messages in thread
From: Tom Rini @ 2016-08-10 0:46 UTC (permalink / raw)
To: u-boot
On Fri, Aug 05, 2016 at 02:49:53PM +0200, Alexander Graf 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 <agraf@suse.de>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160809/bf38f0e8/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-08 21:44 ` Simon Glass
@ 2016-08-10 1:43 ` Tom Rini
2016-08-10 7:47 ` Alexander Graf
1 sibling, 0 replies; 19+ messages in thread
From: Tom Rini @ 2016-08-10 1:43 UTC (permalink / raw)
To: u-boot
On Mon, Aug 08, 2016 at 03:44:30PM -0600, Simon Glass wrote:
> Hi Alexander,
>
> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@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?
Sorry, I had had this patch testing for a bit and missed this.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160809/5a962b39/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-08 21:44 ` Simon Glass
2016-08-10 1:43 ` Tom Rini
@ 2016-08-10 7:47 ` Alexander Graf
2016-08-10 12:56 ` Simon Glass
1 sibling, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-10 7:47 UTC (permalink / raw)
To: u-boot
> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alexander,
>
> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 7:47 ` Alexander Graf
@ 2016-08-10 12:56 ` Simon Glass
2016-08-10 13:02 ` Alexander Graf
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2016-08-10 12:56 UTC (permalink / raw)
To: u-boot
+Tom
Hi Alex,
On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>
>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alexander,
>>
>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>> ---
>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>
[...]
>>> 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.
Actually I'd like to make this feature depend on CONFIG_BLK. If we add
new features that don't use driver model, and then use the legacy data
structures such that converting to driver model becomes harder, we'll
never be done.
I did mention this at the beginning and it seems to have come to pass.
In order of preference from my side:
1. Make EFI_LOADER depend on BLK
2. Apply your messy diff (so that we can remove the old code one day
and end up with something nice)
>
>
[...messy diff...]
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 12:56 ` Simon Glass
@ 2016-08-10 13:02 ` Alexander Graf
2016-08-10 13:16 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-10 13:02 UTC (permalink / raw)
To: u-boot
On 08/10/2016 02:56 PM, Simon Glass wrote:
> +Tom
>
> Hi Alex,
>
> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Alexander,
>>>
>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>> ---
>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>
> [...]
>
>>>> 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.
> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
> new features that don't use driver model, and then use the legacy data
> structures such that converting to driver model becomes harder, we'll
> never be done.
>
> I did mention this at the beginning and it seems to have come to pass.
>
> In order of preference from my side:
>
> 1. Make EFI_LOADER depend on BLK
If we make EFI_LOADER depend on BLK, doesn't that break all systems that
need storage that isn't converted to device model today? Like the SATA
breakage on Xilinx systems, just at a much bigger scale?
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:02 ` Alexander Graf
@ 2016-08-10 13:16 ` Simon Glass
2016-08-10 13:25 ` Alexander Graf
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2016-08-10 13:16 UTC (permalink / raw)
To: u-boot
Hi Alex,
On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>
>> +Tom
>>
>> Hi Alex,
>>
>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>
>>>> Hi Alexander,
>>>>
>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>> ---
>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>>
>> [...]
>>
>>>>> 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.
>>
>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>> new features that don't use driver model, and then use the legacy data
>> structures such that converting to driver model becomes harder, we'll
>> never be done.
>>
>> I did mention this at the beginning and it seems to have come to pass.
>>
>> In order of preference from my side:
>>
>> 1. Make EFI_LOADER depend on BLK
>
>
> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
> need storage that isn't converted to device model today? Like the SATA
> breakage on Xilinx systems, just at a much bigger scale?
No it just means that these platforms need to move to BLK before they
can use the EFI loader. Given the embryonic nature of this feature,
that seems reasonable, and the impact would be small. It will also
encourage conversion and keep the code cleaner.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:16 ` Simon Glass
@ 2016-08-10 13:25 ` Alexander Graf
2016-08-10 13:33 ` Simon Glass
2016-08-10 16:25 ` Tom Rini
0 siblings, 2 replies; 19+ messages in thread
From: Alexander Graf @ 2016-08-10 13:25 UTC (permalink / raw)
To: u-boot
> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>
> Hi Alex,
>
>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>
>>> +Tom
>>>
>>> Hi Alex,
>>>
>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>
>>>>> Hi Alexander,
>>>>>
>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>> ---
>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>> [...]
>>>
>>>>>> 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.
>>>
>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>> new features that don't use driver model, and then use the legacy data
>>> structures such that converting to driver model becomes harder, we'll
>>> never be done.
>>>
>>> I did mention this at the beginning and it seems to have come to pass.
>>>
>>> In order of preference from my side:
>>>
>>> 1. Make EFI_LOADER depend on BLK
>>
>>
>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>> need storage that isn't converted to device model today? Like the SATA
>> breakage on Xilinx systems, just at a much bigger scale?
>
> No it just means that these platforms need to move to BLK before they
> can use the EFI loader. Given the embryonic nature of this feature,
> that seems reasonable, and the impact would be small. It will also
> encourage conversion and keep the code cleaner.
No, it will simply make my life harder because I would have to sit down and vonvert every single board to BLK that I need EFI enabled.
Alex
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:25 ` Alexander Graf
@ 2016-08-10 13:33 ` Simon Glass
2016-08-10 13:41 ` Alexander Graf
2016-08-10 16:25 ` Tom Rini
1 sibling, 1 reply; 19+ messages in thread
From: Simon Glass @ 2016-08-10 13:33 UTC (permalink / raw)
To: u-boot
Hi Alex,
On 10 August 2016 at 07:25, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>
>>>> +Tom
>>>>
>>>> Hi Alex,
>>>>
>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>
>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>
>>>>>> Hi Alexander,
>>>>>>
>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>> ---
>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>> [...]
>>>>
>>>>>>> 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.
>>>>
>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>> new features that don't use driver model, and then use the legacy data
>>>> structures such that converting to driver model becomes harder, we'll
>>>> never be done.
>>>>
>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>
>>>> In order of preference from my side:
>>>>
>>>> 1. Make EFI_LOADER depend on BLK
>>>
>>>
>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>> need storage that isn't converted to device model today? Like the SATA
>>> breakage on Xilinx systems, just at a much bigger scale?
>>
>> No it just means that these platforms need to move to BLK before they
>> can use the EFI loader. Given the embryonic nature of this feature,
>> that seems reasonable, and the impact would be small. It will also
>> encourage conversion and keep the code cleaner.
>
> No, it will simply make my life harder because I would have to sit down and vonvert every single board to BLK that I need EFI enabled.
Yes that's right. But it is mostly just a simple case of enabling the
option. For a few boards there might be an MMC driver that needs
converting, but we can approach those one by one.
Plus I could use the help in converting things.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:33 ` Simon Glass
@ 2016-08-10 13:41 ` Alexander Graf
2016-08-10 13:47 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-10 13:41 UTC (permalink / raw)
To: u-boot
> On 10 Aug 2016, at 15:33, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alex,
>
> On 10 August 2016 at 07:25, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>
>>> Hi Alex,
>>>
>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>
>>>>> +Tom
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>
>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>
>>>>>>> Hi Alexander,
>>>>>>>
>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>> ---
>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>> [...]
>>>>>
>>>>>>>> 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.
>>>>>
>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>> new features that don't use driver model, and then use the legacy data
>>>>> structures such that converting to driver model becomes harder, we'll
>>>>> never be done.
>>>>>
>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>
>>>>> In order of preference from my side:
>>>>>
>>>>> 1. Make EFI_LOADER depend on BLK
>>>>
>>>>
>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>> need storage that isn't converted to device model today? Like the SATA
>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>
>>> No it just means that these platforms need to move to BLK before they
>>> can use the EFI loader. Given the embryonic nature of this feature,
>>> that seems reasonable, and the impact would be small. It will also
>>> encourage conversion and keep the code cleaner.
>>
>> No, it will simply make my life harder because I would have to sit down and vonvert every single board to BLK that I need EFI enabled.
>
> Yes that's right. But it is mostly just a simple case of enabling the
> option. For a few boards there might be an MMC driver that needs
> converting, but we can approach those one by one.
That approach sounds terribly wrong tbh. If it?s so simple, why not just mark a flag day where CONFIG_BLK becomes mandatory and fix all fallout? Then you?ll have much more help at your disposal than a few percent of my overloaded days ;).
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:41 ` Alexander Graf
@ 2016-08-10 13:47 ` Simon Glass
0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2016-08-10 13:47 UTC (permalink / raw)
To: u-boot
Hi Alex,
On 10 August 2016 at 07:41, Alexander Graf <agraf@suse.de> wrote:
>
>> On 10 Aug 2016, at 15:33, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alex,
>>
>> On 10 August 2016 at 07:25, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>>
>>>>>> +Tom
>>>>>>
>>>>>> Hi Alex,
>>>>>>
>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>
>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>>
>>>>>>>> Hi Alexander,
>>>>>>>>
>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>>> ---
>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>>> [...]
>>>>>>
>>>>>>>>> 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.
>>>>>>
>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>>> new features that don't use driver model, and then use the legacy data
>>>>>> structures such that converting to driver model becomes harder, we'll
>>>>>> never be done.
>>>>>>
>>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>>
>>>>>> In order of preference from my side:
>>>>>>
>>>>>> 1. Make EFI_LOADER depend on BLK
>>>>>
>>>>>
>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>>> need storage that isn't converted to device model today? Like the SATA
>>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>>
>>>> No it just means that these platforms need to move to BLK before they
>>>> can use the EFI loader. Given the embryonic nature of this feature,
>>>> that seems reasonable, and the impact would be small. It will also
>>>> encourage conversion and keep the code cleaner.
>>>
>>> No, it will simply make my life harder because I would have to sit down and vonvert every single board to BLK that I need EFI enabled.
>>
>> Yes that's right. But it is mostly just a simple case of enabling the
>> option. For a few boards there might be an MMC driver that needs
>> converting, but we can approach those one by one.
>
> That approach sounds terribly wrong tbh. If it?s so simple, why not just mark a flag day where CONFIG_BLK becomes mandatory and fix all fallout? Then you?ll have much more help at your disposal than a few percent of my overloaded days ;).
We all have plenty of things to do. I can't see how we can do a flag
day. Things need to convert bit by bit as people want the new features
that it enables. We can't just turn off all the non-complying boards.
If we add new features based on legacy code, there is no incentive for
people to switch. I had the same discussion on SPI flash. it really
won't save you are lot of time (you are overestimating the impact),
and it's much better for U-Boot as a whole.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 13:25 ` Alexander Graf
2016-08-10 13:33 ` Simon Glass
@ 2016-08-10 16:25 ` Tom Rini
2016-08-10 19:01 ` Alexander Graf
1 sibling, 1 reply; 19+ messages in thread
From: Tom Rini @ 2016-08-10 16:25 UTC (permalink / raw)
To: u-boot
On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
>
>
> > Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
> >
> > Hi Alex,
> >
> >> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
> >>> On 08/10/2016 02:56 PM, Simon Glass wrote:
> >>>
> >>> +Tom
> >>>
> >>> Hi Alex,
> >>>
> >>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
> >>>>>
> >>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
> >>>>>
> >>>>> Hi Alexander,
> >>>>>
> >>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
> >>>>>> ---
> >>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
> >>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
> >>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
> >>> [...]
> >>>
> >>>>>> 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.
> >>>
> >>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
> >>> new features that don't use driver model, and then use the legacy data
> >>> structures such that converting to driver model becomes harder, we'll
> >>> never be done.
> >>>
> >>> I did mention this at the beginning and it seems to have come to pass.
> >>>
> >>> In order of preference from my side:
> >>>
> >>> 1. Make EFI_LOADER depend on BLK
> >>
> >>
> >> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
> >> need storage that isn't converted to device model today? Like the SATA
> >> breakage on Xilinx systems, just at a much bigger scale?
> >
> > No it just means that these platforms need to move to BLK before they
> > can use the EFI loader. Given the embryonic nature of this feature,
> > that seems reasonable, and the impact would be small. It will also
> > encourage conversion and keep the code cleaner.
>
> No, it will simply make my life harder because I would have to sit
> down and vonvert every single board to BLK that I need EFI enabled.
Seems like as good a place as any to jump in, of the boards that you
need EFI enabled on, what isn't converted today?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160810/15233a41/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 16:25 ` Tom Rini
@ 2016-08-10 19:01 ` Alexander Graf
2016-08-12 17:20 ` Simon Glass
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-10 19:01 UTC (permalink / raw)
To: u-boot
> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
>>
>>
>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>
>>> Hi Alex,
>>>
>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>
>>>>> +Tom
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>
>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>
>>>>>>> Hi Alexander,
>>>>>>>
>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>> ---
>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>> [...]
>>>>>
>>>>>>>> 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.
>>>>>
>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>> new features that don't use driver model, and then use the legacy data
>>>>> structures such that converting to driver model becomes harder, we'll
>>>>> never be done.
>>>>>
>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>
>>>>> In order of preference from my side:
>>>>>
>>>>> 1. Make EFI_LOADER depend on BLK
>>>>
>>>>
>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>> need storage that isn't converted to device model today? Like the SATA
>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>
>>> No it just means that these platforms need to move to BLK before they
>>> can use the EFI loader. Given the embryonic nature of this feature,
>>> that seems reasonable, and the impact would be small. It will also
>>> encourage conversion and keep the code cleaner.
>>
>> No, it will simply make my life harder because I would have to sit
>> down and vonvert every single board to BLK that I need EFI enabled.
>
> Seems like as good a place as any to jump in, of the boards that you
> need EFI enabled on, what isn't converted today?
I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
Again, I strongly object any dependency on BLK for EFI. If you want to push people to using CONFIG_BLK, make CONFIG_ARM depend on CONFIG_BLK.
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-10 19:01 ` Alexander Graf
@ 2016-08-12 17:20 ` Simon Glass
2016-08-12 18:19 ` Alexander Graf
0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2016-08-12 17:20 UTC (permalink / raw)
To: u-boot
Hi Alex,
On 10 August 2016 at 13:01, Alexander Graf <agraf@suse.de> wrote:
>
>> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
>>>
>>>
>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>>
>>>>>> +Tom
>>>>>>
>>>>>> Hi Alex,
>>>>>>
>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>
>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>>
>>>>>>>> Hi Alexander,
>>>>>>>>
>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>>> ---
>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>>> [...]
>>>>>>
>>>>>>>>> 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.
>>>>>>
>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>>> new features that don't use driver model, and then use the legacy data
>>>>>> structures such that converting to driver model becomes harder, we'll
>>>>>> never be done.
>>>>>>
>>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>>
>>>>>> In order of preference from my side:
>>>>>>
>>>>>> 1. Make EFI_LOADER depend on BLK
>>>>>
>>>>>
>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>>> need storage that isn't converted to device model today? Like the SATA
>>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>>
>>>> No it just means that these platforms need to move to BLK before they
>>>> can use the EFI loader. Given the embryonic nature of this feature,
>>>> that seems reasonable, and the impact would be small. It will also
>>>> encourage conversion and keep the code cleaner.
>>>
>>> No, it will simply make my life harder because I would have to sit
>>> down and vonvert every single board to BLK that I need EFI enabled.
>>
>> Seems like as good a place as any to jump in, of the boards that you
>> need EFI enabled on, what isn't converted today?
>
> I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
Of course, I fully understand that. However as mentioned in this
patch, you are creating future problems.
Can you address Tom's question? I take it that it boots on Raspberry
Pi (which I'd like to try actually - are there instructions
somewhere?). We can easily convert that over. Anything else?
>
> Again, I strongly object any dependency on BLK for EFI. If you want to push people to using CONFIG_BLK, make CONFIG_ARM depend on CONFIG_BLK.
:-) That won't work. If we could wave a magic wand and convert
everything in one day, we would.
Regards,
Simon
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-12 17:20 ` Simon Glass
@ 2016-08-12 18:19 ` Alexander Graf
2016-08-19 18:14 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-12 18:19 UTC (permalink / raw)
To: u-boot
On 12.08.16 19:20, Simon Glass wrote:
> Hi Alex,
>
> On 10 August 2016 at 13:01, Alexander Graf <agraf@suse.de> wrote:
>>
>>> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
>>>>
>>>>
>>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>>>
>>>>>>> +Tom
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>>
>>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>>>
>>>>>>>>> Hi Alexander,
>>>>>>>>>
>>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>>>> ---
>>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>>>> [...]
>>>>>>>
>>>>>>>>>> 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.
>>>>>>>
>>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>>>> new features that don't use driver model, and then use the legacy data
>>>>>>> structures such that converting to driver model becomes harder, we'll
>>>>>>> never be done.
>>>>>>>
>>>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>>>
>>>>>>> In order of preference from my side:
>>>>>>>
>>>>>>> 1. Make EFI_LOADER depend on BLK
>>>>>>
>>>>>>
>>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>>>> need storage that isn't converted to device model today? Like the SATA
>>>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>>>
>>>>> No it just means that these platforms need to move to BLK before they
>>>>> can use the EFI loader. Given the embryonic nature of this feature,
>>>>> that seems reasonable, and the impact would be small. It will also
>>>>> encourage conversion and keep the code cleaner.
>>>>
>>>> No, it will simply make my life harder because I would have to sit
>>>> down and vonvert every single board to BLK that I need EFI enabled.
>>>
>>> Seems like as good a place as any to jump in, of the boards that you
>>> need EFI enabled on, what isn't converted today?
>>
>> I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
>
> Of course, I fully understand that. However as mentioned in this
> patch, you are creating future problems.
I don't see how I am creating future problems, really. Passing a
udevice* instead of the struct blk_desc* internally doesn't improve the
code really at the end of the day.
> Can you address Tom's question? I take it that it boots on Raspberry
> Pi (which I'd like to try actually - are there instructions
> somewhere?). We can easily convert that over. Anything else?
For a list of currently available "upstream" openSUSE images, see
https://build.opensuse.org/package/view_file/openSUSE:Factory:ARM/JeOS/pre_checkin.sh?expand=1
Every one of those is on the short-term list. Any other board that
people want to run on is potentially on the mid-term to long-term list.
>
>>
>> Again, I strongly object any dependency on BLK for EFI. If you want to push people to using CONFIG_BLK, make CONFIG_ARM depend on CONFIG_BLK.
>
> :-) That won't work. If we could wave a magic wand and convert
> everything in one day, we would.
EFI isn't going to be that magic wand either though :). Most people
couldn't care less. It's hard enough to convince people that disabling
CONFIG_EFI is a bad idea. Having anyone put effort into it would
basically mean the end of the whole idea that uEFI support "just works"
with U-Boot (which is the main selling point).
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-12 18:19 ` Alexander Graf
@ 2016-08-19 18:14 ` Tom Rini
2016-08-22 4:22 ` Alexander Graf
0 siblings, 1 reply; 19+ messages in thread
From: Tom Rini @ 2016-08-19 18:14 UTC (permalink / raw)
To: u-boot
On Fri, Aug 12, 2016 at 08:19:42PM +0200, Alexander Graf wrote:
>
>
> On 12.08.16 19:20, Simon Glass wrote:
> > Hi Alex,
> >
> > On 10 August 2016 at 13:01, Alexander Graf <agraf@suse.de> wrote:
> >>
> >>> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
> >>>
> >>> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
> >>>>>
> >>>>> Hi Alex,
> >>>>>
> >>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
> >>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
> >>>>>>>
> >>>>>>> +Tom
> >>>>>>>
> >>>>>>> Hi Alex,
> >>>>>>>
> >>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
> >>>>>>>>>
> >>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Alexander,
> >>>>>>>>>
> >>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
> >>>>>>>>>> ---
> >>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
> >>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
> >>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
> >>>>>>> [...]
> >>>>>>>
> >>>>>>>>>> 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.
> >>>>>>>
> >>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
> >>>>>>> new features that don't use driver model, and then use the legacy data
> >>>>>>> structures such that converting to driver model becomes harder, we'll
> >>>>>>> never be done.
> >>>>>>>
> >>>>>>> I did mention this at the beginning and it seems to have come to pass.
> >>>>>>>
> >>>>>>> In order of preference from my side:
> >>>>>>>
> >>>>>>> 1. Make EFI_LOADER depend on BLK
> >>>>>>
> >>>>>>
> >>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
> >>>>>> need storage that isn't converted to device model today? Like the SATA
> >>>>>> breakage on Xilinx systems, just at a much bigger scale?
> >>>>>
> >>>>> No it just means that these platforms need to move to BLK before they
> >>>>> can use the EFI loader. Given the embryonic nature of this feature,
> >>>>> that seems reasonable, and the impact would be small. It will also
> >>>>> encourage conversion and keep the code cleaner.
> >>>>
> >>>> No, it will simply make my life harder because I would have to sit
> >>>> down and vonvert every single board to BLK that I need EFI enabled.
> >>>
> >>> Seems like as good a place as any to jump in, of the boards that you
> >>> need EFI enabled on, what isn't converted today?
> >>
> >> I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
> >
> > Of course, I fully understand that. However as mentioned in this
> > patch, you are creating future problems.
>
> I don't see how I am creating future problems, really. Passing a
> udevice* instead of the struct blk_desc* internally doesn't improve the
> code really at the end of the day.
>
> > Can you address Tom's question? I take it that it boots on Raspberry
> > Pi (which I'd like to try actually - are there instructions
> > somewhere?). We can easily convert that over. Anything else?
>
> For a list of currently available "upstream" openSUSE images, see
> https://build.opensuse.org/package/view_file/openSUSE:Factory:ARM/JeOS/pre_checkin.sh?expand=1
>
> Every one of those is on the short-term list. Any other board that
> people want to run on is potentially on the mid-term to long-term list.
OK, that is a lot of boards and such. And yes, I see both of these
features / projects as important to the long-term health of U-Boot.
So, Alex, when we've got storage converted fully to DM, you're willing
to do further clean-ups to make it DM-better, yes? Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160819/73196822/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-19 18:14 ` Tom Rini
@ 2016-08-22 4:22 ` Alexander Graf
2016-08-22 15:56 ` Tom Rini
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Graf @ 2016-08-22 4:22 UTC (permalink / raw)
To: u-boot
> Am 19.08.2016 um 14:14 schrieb Tom Rini <trini@konsulko.com>:
>
>> On Fri, Aug 12, 2016 at 08:19:42PM +0200, Alexander Graf wrote:
>>
>>
>>> On 12.08.16 19:20, Simon Glass wrote:
>>> Hi Alex,
>>>
>>>> On 10 August 2016 at 13:01, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>>> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
>>>>>
>>>>> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
>>>>>>
>>>>>>
>>>>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
>>>>>>>>>
>>>>>>>>> +Tom
>>>>>>>>>
>>>>>>>>> Hi Alex,
>>>>>>>>>
>>>>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Alexander,
>>>>>>>>>>>
>>>>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
>>>>>>>>>>>> ---
>>>>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
>>>>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
>>>>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
>>>>>>>>> [...]
>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
>>>>>>>>> new features that don't use driver model, and then use the legacy data
>>>>>>>>> structures such that converting to driver model becomes harder, we'll
>>>>>>>>> never be done.
>>>>>>>>>
>>>>>>>>> I did mention this at the beginning and it seems to have come to pass.
>>>>>>>>>
>>>>>>>>> In order of preference from my side:
>>>>>>>>>
>>>>>>>>> 1. Make EFI_LOADER depend on BLK
>>>>>>>>
>>>>>>>>
>>>>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
>>>>>>>> need storage that isn't converted to device model today? Like the SATA
>>>>>>>> breakage on Xilinx systems, just at a much bigger scale?
>>>>>>>
>>>>>>> No it just means that these platforms need to move to BLK before they
>>>>>>> can use the EFI loader. Given the embryonic nature of this feature,
>>>>>>> that seems reasonable, and the impact would be small. It will also
>>>>>>> encourage conversion and keep the code cleaner.
>>>>>>
>>>>>> No, it will simply make my life harder because I would have to sit
>>>>>> down and vonvert every single board to BLK that I need EFI enabled.
>>>>>
>>>>> Seems like as good a place as any to jump in, of the boards that you
>>>>> need EFI enabled on, what isn't converted today?
>>>>
>>>> I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
>>>
>>> Of course, I fully understand that. However as mentioned in this
>>> patch, you are creating future problems.
>>
>> I don't see how I am creating future problems, really. Passing a
>> udevice* instead of the struct blk_desc* internally doesn't improve the
>> code really at the end of the day.
>>
>>> Can you address Tom's question? I take it that it boots on Raspberry
>>> Pi (which I'd like to try actually - are there instructions
>>> somewhere?). We can easily convert that over. Anything else?
>>
>> For a list of currently available "upstream" openSUSE images, see
>> https://build.opensuse.org/package/view_file/openSUSE:Factory:ARM/JeOS/pre_checkin.sh?expand=1
>>
>> Every one of those is on the short-term list. Any other board that
>> people want to run on is potentially on the mid-term to long-term list.
>
> OK, that is a lot of boards and such. And yes, I see both of these
> features / projects as important to the long-term health of U-Boot.
>
> So, Alex, when we've got storage converted fully to DM, you're willing
> to do further clean-ups to make it DM-better, yes? Thanks!
I don't understand the question. Eventually I agree with Simon that we should have only a single object list and type, but with the state dm is in, I definitely will not require any of the efi code on dm.
Having said that, I am definitely making sure things work with dm and as soon as a non-dm subsystem drops, I will happily integrate the efi onject hierarchy into the dm one for that class of objects.
Alex
^ permalink raw reply [flat|nested] 19+ messages in thread
* [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage
2016-08-22 4:22 ` Alexander Graf
@ 2016-08-22 15:56 ` Tom Rini
0 siblings, 0 replies; 19+ messages in thread
From: Tom Rini @ 2016-08-22 15:56 UTC (permalink / raw)
To: u-boot
On Mon, Aug 22, 2016 at 12:22:58AM -0400, Alexander Graf wrote:
>
>
> > Am 19.08.2016 um 14:14 schrieb Tom Rini <trini@konsulko.com>:
> >
> >> On Fri, Aug 12, 2016 at 08:19:42PM +0200, Alexander Graf wrote:
> >>
> >>
> >>> On 12.08.16 19:20, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>>> On 10 August 2016 at 13:01, Alexander Graf <agraf@suse.de> wrote:
> >>>>
> >>>>> On 10 Aug 2016, at 18:25, Tom Rini <trini@konsulko.com> wrote:
> >>>>>
> >>>>> On Wed, Aug 10, 2016 at 03:25:16PM +0200, Alexander Graf wrote:
> >>>>>>
> >>>>>>
> >>>>>>> Am 10.08.2016 um 15:16 schrieb Simon Glass <sjg@chromium.org>:
> >>>>>>>
> >>>>>>> Hi Alex,
> >>>>>>>
> >>>>>>>>> On 10 August 2016 at 07:02, Alexander Graf <agraf@suse.de> wrote:
> >>>>>>>>> On 08/10/2016 02:56 PM, Simon Glass wrote:
> >>>>>>>>>
> >>>>>>>>> +Tom
> >>>>>>>>>
> >>>>>>>>> Hi Alex,
> >>>>>>>>>
> >>>>>>>>> On 10 August 2016 at 01:47, Alexander Graf <agraf@suse.de> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> On 08 Aug 2016, at 23:44, Simon Glass <sjg@chromium.org> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Alexander,
> >>>>>>>>>>>
> >>>>>>>>>>>> On 5 August 2016 at 06:49, Alexander Graf <agraf@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 <agraf@suse.de>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>> cmd/bootefi.c | 23 ++++++++++++++++++-----
> >>>>>>>>>>>> lib/efi_loader/efi_disk.c | 18 +++++++++++-------
> >>>>>>>>>>>> 2 files changed, 29 insertions(+), 12 deletions(-)
> >>>>>>>>> [...]
> >>>>>>>>>
> >>>>>>>>>>>> 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.
> >>>>>>>>>
> >>>>>>>>> Actually I'd like to make this feature depend on CONFIG_BLK. If we add
> >>>>>>>>> new features that don't use driver model, and then use the legacy data
> >>>>>>>>> structures such that converting to driver model becomes harder, we'll
> >>>>>>>>> never be done.
> >>>>>>>>>
> >>>>>>>>> I did mention this at the beginning and it seems to have come to pass.
> >>>>>>>>>
> >>>>>>>>> In order of preference from my side:
> >>>>>>>>>
> >>>>>>>>> 1. Make EFI_LOADER depend on BLK
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> If we make EFI_LOADER depend on BLK, doesn't that break all systems that
> >>>>>>>> need storage that isn't converted to device model today? Like the SATA
> >>>>>>>> breakage on Xilinx systems, just at a much bigger scale?
> >>>>>>>
> >>>>>>> No it just means that these platforms need to move to BLK before they
> >>>>>>> can use the EFI loader. Given the embryonic nature of this feature,
> >>>>>>> that seems reasonable, and the impact would be small. It will also
> >>>>>>> encourage conversion and keep the code cleaner.
> >>>>>>
> >>>>>> No, it will simply make my life harder because I would have to sit
> >>>>>> down and vonvert every single board to BLK that I need EFI enabled.
> >>>>>
> >>>>> Seems like as good a place as any to jump in, of the boards that you
> >>>>> need EFI enabled on, what isn't converted today?
> >>>>
> >>>> I want to make EFI the default boot path in openSUSE, which means any board that anyone out there wants to run openSUSE on would be on the list. Anything that keeps them from running EFI on a random board is a road block that I?d rather not have if I can avoid it.
> >>>
> >>> Of course, I fully understand that. However as mentioned in this
> >>> patch, you are creating future problems.
> >>
> >> I don't see how I am creating future problems, really. Passing a
> >> udevice* instead of the struct blk_desc* internally doesn't improve the
> >> code really at the end of the day.
> >>
> >>> Can you address Tom's question? I take it that it boots on Raspberry
> >>> Pi (which I'd like to try actually - are there instructions
> >>> somewhere?). We can easily convert that over. Anything else?
> >>
> >> For a list of currently available "upstream" openSUSE images, see
> >> https://build.opensuse.org/package/view_file/openSUSE:Factory:ARM/JeOS/pre_checkin.sh?expand=1
> >>
> >> Every one of those is on the short-term list. Any other board that
> >> people want to run on is potentially on the mid-term to long-term list.
> >
> > OK, that is a lot of boards and such. And yes, I see both of these
> > features / projects as important to the long-term health of U-Boot.
> >
> > So, Alex, when we've got storage converted fully to DM, you're willing
> > to do further clean-ups to make it DM-better, yes? Thanks!
>
> I don't understand the question. Eventually I agree with Simon that we
> should have only a single object list and type, but with the state dm
> is in, I definitely will not require any of the efi code on dm.
>
> Having said that, I am definitely making sure things work with dm and
> as soon as a non-dm subsystem drops, I will happily integrate the efi
> onject hierarchy into the dm one for that class of objects.
OK, that's what I wanted. To me the greatest problem (and why we've
pushed back on some other things that came in but didn't implement DM)
is non-DM code that won't have someone active to convert it later. That
won't be the case here, so we'll address things here more later.
Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160822/2f480823/attachment.sig>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-08-22 15:56 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-05 12:49 [U-Boot] [PATCH] efi_loader: disk: Fix CONFIG_BLK breakage Alexander Graf
2016-08-08 21:44 ` Simon Glass
2016-08-10 1:43 ` Tom Rini
2016-08-10 7:47 ` Alexander Graf
2016-08-10 12:56 ` Simon Glass
2016-08-10 13:02 ` Alexander Graf
2016-08-10 13:16 ` Simon Glass
2016-08-10 13:25 ` Alexander Graf
2016-08-10 13:33 ` Simon Glass
2016-08-10 13:41 ` Alexander Graf
2016-08-10 13:47 ` Simon Glass
2016-08-10 16:25 ` Tom Rini
2016-08-10 19:01 ` Alexander Graf
2016-08-12 17:20 ` Simon Glass
2016-08-12 18:19 ` Alexander Graf
2016-08-19 18:14 ` Tom Rini
2016-08-22 4:22 ` Alexander Graf
2016-08-22 15:56 ` Tom Rini
2016-08-10 0:46 ` [U-Boot] " Tom Rini
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.