| Message ID | 20211001050228.55183-29-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Add efi_disk_create() function. > > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk > object, the udevice is either a UCLASS_BLK (a whole raw disk) or > UCLASS_PARTITION (a disk partition). > > So this function is expected to be called every time such an udevice > is detected and activated through a device model's "probe" interface. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > include/efi_loader.h | 2 + > lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> But some nits below. Don't worry about !CONFIG_BLK - that code should be removed. > diff --git a/include/efi_loader.h b/include/efi_loader.h > index c440962fe522..751fde7fb153 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > void efi_carve_out_dt_rsv(void *fdt); > /* Called by bootefi to make console interface available */ > efi_status_t efi_console_register(void); > +/* Called when a block devices has been probed */ > +int efi_disk_create(struct udevice *dev); Please buck the trend in this file and add a full function comment. In this case it needs to cover @dev and the return value and also explain what the function does. > /* Called by bootefi to make all disk storage accessible as EFI objects */ > efi_status_t efi_disk_register(void); > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > index cd5528046251..3fae40e034fb 100644 > --- a/lib/efi_loader/efi_disk.c > +++ b/lib/efi_loader/efi_disk.c > @@ -10,6 +10,7 @@ > #include <common.h> > #include <blk.h> > #include <dm.h> > +#include <dm/device-internal.h> > #include <efi_loader.h> > #include <fs.h> > #include <log.h> > @@ -484,6 +485,7 @@ error: > return ret; > } > > +#ifndef CONFIG_BLK > /** > * efi_disk_create_partitions() - create handles and protocols for partitions > * > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > return disks; > } > +#endif /* CONFIG_BLK */ > + > +/* > + * Create a handle for a whole raw disk > + * > + * @dev uclass device ?? what type of device? > + * @return 0 on success, -1 otherwise > + */ > +static int efi_disk_create_raw(struct udevice *dev) > +{ > + struct efi_disk_obj *disk; > + struct blk_desc *desc; > + const char *if_typename; > + int diskid; > + efi_status_t ret; > + > + desc = dev_get_uclass_plat(dev); > + if_typename = blk_get_if_type_name(desc->if_type); > + diskid = desc->devnum; > + > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, > + diskid, NULL, 0, &disk); > + if (ret != EFI_SUCCESS) { if (ret) is much shorter and easier to read > + log_err("Adding disk %s%d failed\n", if_typename, diskid); > + return -1; > + } > + disk->dev = dev; > + dev->efi_obj = &disk->header; > + > + return 0; > +} > + > +/* > + * Create a handle for a disk partition > + * > + * @dev uclass device ?? > + * @return 0 on success, -1 otherwise > + */ > +static int efi_disk_create_part(struct udevice *dev) > +{ > + efi_handle_t parent; > + struct blk_desc *desc; > + const char *if_typename; > + struct disk_part *part_data; > + struct disk_partition *info; > + unsigned int part; > + int diskid; > + struct efi_device_path *dp = NULL; > + struct efi_disk_obj *disk; > + efi_status_t ret; > + > + parent = dev->parent->efi_obj; dev_get_parent(dev) > + desc = dev_get_uclass_plat(dev->parent); > + if_typename = blk_get_if_type_name(desc->if_type); > + diskid = desc->devnum; > + > + part_data = dev_get_uclass_plat(dev); > + part = part_data->partnum; > + info = &part_data->gpt_part_info; > + > + /* TODO: should not use desc? */ > + dp = efi_dp_from_part(desc, 0); > + > + ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid, > + info, part, &disk); > + if (ret != EFI_SUCCESS) { > + log_err("Adding partition %s%d:%x failed\n", > + if_typename, diskid, part); > + return -1; > + } > + disk->dev = dev; > + dev->efi_obj = &disk->header; > + > + return 0; > +} > + > +int efi_disk_create(struct udevice *dev) > +{ > + enum uclass_id id; > + > + id = device_get_uclass_id(dev); > + > + if (id == UCLASS_BLK) > + return efi_disk_create_raw(dev); > + > + if (id == UCLASS_PARTITION) > + return efi_disk_create_part(dev); > + > + return -1; > +} > > /** > * efi_disk_register() - register block devices > -- > 2.33.0 > Regards, Simon
On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote: > On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > Add efi_disk_create() function. > > > > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk > > object, the udevice is either a UCLASS_BLK (a whole raw disk) or > > UCLASS_PARTITION (a disk partition). > > > > So this function is expected to be called every time such an udevice > > is detected and activated through a device model's "probe" interface. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > include/efi_loader.h | 2 + > > lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 94 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > But some nits below. > > Don't worry about !CONFIG_BLK - that code should be removed. Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk in patch#13. > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index c440962fe522..751fde7fb153 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > > void efi_carve_out_dt_rsv(void *fdt); > > /* Called by bootefi to make console interface available */ > > efi_status_t efi_console_register(void); > > +/* Called when a block devices has been probed */ > > +int efi_disk_create(struct udevice *dev); > > Please buck the trend in this file and add a full function comment. In > this case it needs to cover @dev and the return value and also explain > what the function does. OK. > > /* Called by bootefi to make all disk storage accessible as EFI objects */ > > efi_status_t efi_disk_register(void); > > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > index cd5528046251..3fae40e034fb 100644 > > --- a/lib/efi_loader/efi_disk.c > > +++ b/lib/efi_loader/efi_disk.c > > @@ -10,6 +10,7 @@ > > #include <common.h> > > #include <blk.h> > > #include <dm.h> > > +#include <dm/device-internal.h> > > #include <efi_loader.h> > > #include <fs.h> > > #include <log.h> > > @@ -484,6 +485,7 @@ error: > > return ret; > > } > > > > +#ifndef CONFIG_BLK > > /** > > * efi_disk_create_partitions() - create handles and protocols for partitions > > * > > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > > > return disks; > > } > > +#endif /* CONFIG_BLK */ > > + > > +/* > > + * Create a handle for a whole raw disk > > + * > > + * @dev uclass device > > ?? what type of device? (Will fix: UCLASS_BLK) > > + * @return 0 on success, -1 otherwise > > + */ > > +static int efi_disk_create_raw(struct udevice *dev) > > +{ > > + struct efi_disk_obj *disk; > > + struct blk_desc *desc; > > + const char *if_typename; > > + int diskid; > > + efi_status_t ret; > > + > > + desc = dev_get_uclass_plat(dev); > > + if_typename = blk_get_if_type_name(desc->if_type); > > + diskid = desc->devnum; > > + > > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, > > + diskid, NULL, 0, &disk); > > + if (ret != EFI_SUCCESS) { > > if (ret) > > is much shorter and easier to read Yeah, but I don't want to assume EFI_SUCCESS is *zero*. > > + log_err("Adding disk %s%d failed\n", if_typename, diskid); > > + return -1; > > + } > > + disk->dev = dev; > > + dev->efi_obj = &disk->header; > > + > > + return 0; > > +} > > + > > +/* > > + * Create a handle for a disk partition > > + * > > + * @dev uclass device > > ?? (UCLASS_PARTITION) > > + * @return 0 on success, -1 otherwise > > + */ > > +static int efi_disk_create_part(struct udevice *dev) > > +{ > > + efi_handle_t parent; > > + struct blk_desc *desc; > > + const char *if_typename; > > + struct disk_part *part_data; > > + struct disk_partition *info; > > + unsigned int part; > > + int diskid; > > + struct efi_device_path *dp = NULL; > > + struct efi_disk_obj *disk; > > + efi_status_t ret; > > + > > + parent = dev->parent->efi_obj; > > dev_get_parent(dev) I will replace all of "dev->parent" with dev_get_parent() if I will not forget to do that in the next version :) Thanks, -Takahiro Akashi > > + desc = dev_get_uclass_plat(dev->parent); > > + if_typename = blk_get_if_type_name(desc->if_type); > > + diskid = desc->devnum; > > + > > + part_data = dev_get_uclass_plat(dev); > > + part = part_data->partnum; > > + info = &part_data->gpt_part_info; > > + > > + /* TODO: should not use desc? */ > > + dp = efi_dp_from_part(desc, 0); > > + > > + ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid, > > + info, part, &disk); > > + if (ret != EFI_SUCCESS) { > > + log_err("Adding partition %s%d:%x failed\n", > > + if_typename, diskid, part); > > + return -1; > > + } > > + disk->dev = dev; > > + dev->efi_obj = &disk->header; > > + > > + return 0; > > +} > > + > > +int efi_disk_create(struct udevice *dev) > > +{ > > + enum uclass_id id; > > + > > + id = device_get_uclass_id(dev); > > + > > + if (id == UCLASS_BLK) > > + return efi_disk_create_raw(dev); > > + > > + if (id == UCLASS_PARTITION) > > + return efi_disk_create_part(dev); > > + > > + return -1; > > +} > > > > /** > > * efi_disk_register() - register block devices > > -- > > 2.33.0 > > > > Regards, > Simon
Hi Takahiro, On Mon, 11 Oct 2021 at 00:52, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote: > > On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > > > > > > Add efi_disk_create() function. > > > > > > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk > > > object, the udevice is either a UCLASS_BLK (a whole raw disk) or > > > UCLASS_PARTITION (a disk partition). > > > > > > So this function is expected to be called every time such an udevice > > > is detected and activated through a device model's "probe" interface. > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > --- > > > include/efi_loader.h | 2 + > > > lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 94 insertions(+) > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > But some nits below. > > > > Don't worry about !CONFIG_BLK - that code should be removed. > > Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk > in patch#13. > > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > > index c440962fe522..751fde7fb153 100644 > > > --- a/include/efi_loader.h > > > +++ b/include/efi_loader.h > > > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > > > void efi_carve_out_dt_rsv(void *fdt); > > > /* Called by bootefi to make console interface available */ > > > efi_status_t efi_console_register(void); > > > +/* Called when a block devices has been probed */ > > > +int efi_disk_create(struct udevice *dev); > > > > Please buck the trend in this file and add a full function comment. In > > this case it needs to cover @dev and the return value and also explain > > what the function does. > > OK. > > > > /* Called by bootefi to make all disk storage accessible as EFI objects */ > > > efi_status_t efi_disk_register(void); > > > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > > index cd5528046251..3fae40e034fb 100644 > > > --- a/lib/efi_loader/efi_disk.c > > > +++ b/lib/efi_loader/efi_disk.c > > > @@ -10,6 +10,7 @@ > > > #include <common.h> > > > #include <blk.h> > > > #include <dm.h> > > > +#include <dm/device-internal.h> > > > #include <efi_loader.h> > > > #include <fs.h> > > > #include <log.h> > > > @@ -484,6 +485,7 @@ error: > > > return ret; > > > } > > > > > > +#ifndef CONFIG_BLK > > > /** > > > * efi_disk_create_partitions() - create handles and protocols for partitions > > > * > > > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > > > > > return disks; > > > } > > > +#endif /* CONFIG_BLK */ > > > + > > > +/* > > > + * Create a handle for a whole raw disk > > > + * > > > + * @dev uclass device > > > > ?? what type of device? > > (Will fix: UCLASS_BLK) > > > > > + * @return 0 on success, -1 otherwise > > > + */ > > > +static int efi_disk_create_raw(struct udevice *dev) > > > +{ > > > + struct efi_disk_obj *disk; > > > + struct blk_desc *desc; > > > + const char *if_typename; > > > + int diskid; > > > + efi_status_t ret; > > > + > > > + desc = dev_get_uclass_plat(dev); > > > + if_typename = blk_get_if_type_name(desc->if_type); > > > + diskid = desc->devnum; > > > + > > > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, > > > + diskid, NULL, 0, &disk); > > > + if (ret != EFI_SUCCESS) { > > > > if (ret) > > > > is much shorter and easier to read > > Yeah, but I don't want to assume EFI_SUCCESS is *zero*. It is defined as 0 in 'Appendix D - Status Code' and cannot change, as I understand it. This is one of the things I don't like about the EFI code in U-Boot. Presumably the people who wrote the spec defined it as 0 to make use of C constructs. [..] Regards, Simon
Simon, On Mon, Oct 11, 2021 at 08:54:06AM -0600, Simon Glass wrote: > Hi Takahiro, > > On Mon, 11 Oct 2021 at 00:52, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote: > > > On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro > > > <takahiro.akashi@linaro.org> wrote: > > > > > > > > Add efi_disk_create() function. > > > > > > > > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk > > > > object, the udevice is either a UCLASS_BLK (a whole raw disk) or > > > > UCLASS_PARTITION (a disk partition). > > > > > > > > So this function is expected to be called every time such an udevice > > > > is detected and activated through a device model's "probe" interface. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > --- > > > > include/efi_loader.h | 2 + > > > > lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 94 insertions(+) > > > > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > But some nits below. > > > > > > Don't worry about !CONFIG_BLK - that code should be removed. > > > > Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk > > in patch#13. > > > > > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > > > index c440962fe522..751fde7fb153 100644 > > > > --- a/include/efi_loader.h > > > > +++ b/include/efi_loader.h > > > > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > > > > void efi_carve_out_dt_rsv(void *fdt); > > > > /* Called by bootefi to make console interface available */ > > > > efi_status_t efi_console_register(void); > > > > +/* Called when a block devices has been probed */ > > > > +int efi_disk_create(struct udevice *dev); > > > > > > Please buck the trend in this file and add a full function comment. In > > > this case it needs to cover @dev and the return value and also explain > > > what the function does. > > > > OK. > > > > > > /* Called by bootefi to make all disk storage accessible as EFI objects */ > > > > efi_status_t efi_disk_register(void); > > > > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ > > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > > > index cd5528046251..3fae40e034fb 100644 > > > > --- a/lib/efi_loader/efi_disk.c > > > > +++ b/lib/efi_loader/efi_disk.c > > > > @@ -10,6 +10,7 @@ > > > > #include <common.h> > > > > #include <blk.h> > > > > #include <dm.h> > > > > +#include <dm/device-internal.h> > > > > #include <efi_loader.h> > > > > #include <fs.h> > > > > #include <log.h> > > > > @@ -484,6 +485,7 @@ error: > > > > return ret; > > > > } > > > > > > > > +#ifndef CONFIG_BLK > > > > /** > > > > * efi_disk_create_partitions() - create handles and protocols for partitions > > > > * > > > > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > > > > > > > return disks; > > > > } > > > > +#endif /* CONFIG_BLK */ > > > > + > > > > +/* > > > > + * Create a handle for a whole raw disk > > > > + * > > > > + * @dev uclass device > > > > > > ?? what type of device? > > > > (Will fix: UCLASS_BLK) > > > > > > > > + * @return 0 on success, -1 otherwise > > > > + */ > > > > +static int efi_disk_create_raw(struct udevice *dev) > > > > +{ > > > > + struct efi_disk_obj *disk; > > > > + struct blk_desc *desc; > > > > + const char *if_typename; > > > > + int diskid; > > > > + efi_status_t ret; > > > > + > > > > + desc = dev_get_uclass_plat(dev); > > > > + if_typename = blk_get_if_type_name(desc->if_type); > > > > + diskid = desc->devnum; > > > > + > > > > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, > > > > + diskid, NULL, 0, &disk); > > > > + if (ret != EFI_SUCCESS) { > > > > > > if (ret) > > > > > > is much shorter and easier to read > > > > Yeah, but I don't want to assume EFI_SUCCESS is *zero*. > > It is defined as 0 in 'Appendix D - Status Code' and cannot change, as > I understand it. This is one of the things I don't like about the EFI > code in U-Boot. Presumably the people who wrote the spec defined it as > 0 to make use of C constructs. Yeah, I confirmed that, but still want to keep the code as "ret != EFI_SUCCESS" is used everywhere in UEFI code :) -Takahiro Akashi > [..] > > Regards, > Simon
Hi Akashi, On Mon, 11 Oct 2021 at 19:09, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Simon, > > On Mon, Oct 11, 2021 at 08:54:06AM -0600, Simon Glass wrote: > > Hi Takahiro, > > > > On Mon, 11 Oct 2021 at 00:52, AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > > > > > > On Sun, Oct 10, 2021 at 08:14:21AM -0600, Simon Glass wrote: > > > > On Thu, 30 Sept 2021 at 23:04, AKASHI Takahiro > > > > <takahiro.akashi@linaro.org> wrote: > > > > > > > > > > Add efi_disk_create() function. > > > > > > > > > > Any UEFI handle created by efi_disk_create() can be treated as a efi_disk > > > > > object, the udevice is either a UCLASS_BLK (a whole raw disk) or > > > > > UCLASS_PARTITION (a disk partition). > > > > > > > > > > So this function is expected to be called every time such an udevice > > > > > is detected and activated through a device model's "probe" interface. > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > --- > > > > > include/efi_loader.h | 2 + > > > > > lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ > > > > > 2 files changed, 94 insertions(+) > > > > > > > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > > > But some nits below. > > > > > > > > Don't worry about !CONFIG_BLK - that code should be removed. > > > > > > Yes. I added a tentative patch to remove !CONFIG_BLK code in efi_disk > > > in patch#13. > > > > > > > > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > > > > index c440962fe522..751fde7fb153 100644 > > > > > --- a/include/efi_loader.h > > > > > +++ b/include/efi_loader.h > > > > > @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > > > > > void efi_carve_out_dt_rsv(void *fdt); > > > > > /* Called by bootefi to make console interface available */ > > > > > efi_status_t efi_console_register(void); > > > > > +/* Called when a block devices has been probed */ > > > > > +int efi_disk_create(struct udevice *dev); > > > > > > > > Please buck the trend in this file and add a full function comment. In > > > > this case it needs to cover @dev and the return value and also explain > > > > what the function does. > > > > > > OK. > > > > > > > > /* Called by bootefi to make all disk storage accessible as EFI objects */ > > > > > efi_status_t efi_disk_register(void); > > > > > /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ > > > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > > > > index cd5528046251..3fae40e034fb 100644 > > > > > --- a/lib/efi_loader/efi_disk.c > > > > > +++ b/lib/efi_loader/efi_disk.c > > > > > @@ -10,6 +10,7 @@ > > > > > #include <common.h> > > > > > #include <blk.h> > > > > > #include <dm.h> > > > > > +#include <dm/device-internal.h> > > > > > #include <efi_loader.h> > > > > > #include <fs.h> > > > > > #include <log.h> > > > > > @@ -484,6 +485,7 @@ error: > > > > > return ret; > > > > > } > > > > > > > > > > +#ifndef CONFIG_BLK > > > > > /** > > > > > * efi_disk_create_partitions() - create handles and protocols for partitions > > > > > * > > > > > @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > > > > > > > > > return disks; > > > > > } > > > > > +#endif /* CONFIG_BLK */ > > > > > + > > > > > +/* > > > > > + * Create a handle for a whole raw disk > > > > > + * > > > > > + * @dev uclass device > > > > > > > > ?? what type of device? > > > > > > (Will fix: UCLASS_BLK) > > > > > > > > > > > + * @return 0 on success, -1 otherwise > > > > > + */ > > > > > +static int efi_disk_create_raw(struct udevice *dev) > > > > > +{ > > > > > + struct efi_disk_obj *disk; > > > > > + struct blk_desc *desc; > > > > > + const char *if_typename; > > > > > + int diskid; > > > > > + efi_status_t ret; > > > > > + > > > > > + desc = dev_get_uclass_plat(dev); > > > > > + if_typename = blk_get_if_type_name(desc->if_type); > > > > > + diskid = desc->devnum; > > > > > + > > > > > + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, > > > > > + diskid, NULL, 0, &disk); > > > > > + if (ret != EFI_SUCCESS) { > > > > > > > > if (ret) > > > > > > > > is much shorter and easier to read > > > > > > Yeah, but I don't want to assume EFI_SUCCESS is *zero*. > > > > It is defined as 0 in 'Appendix D - Status Code' and cannot change, as > > I understand it. This is one of the things I don't like about the EFI > > code in U-Boot. Presumably the people who wrote the spec defined it as > > 0 to make use of C constructs. > > Yeah, I confirmed that, but still want to keep the code > as "ret != EFI_SUCCESS" is used everywhere in UEFI code :) OK that can be a clean-up for another day, along with the overly long types and naming in general. At least we managed to avoid all the capital letters and camel case... Regards, Simon
diff --git a/include/efi_loader.h b/include/efi_loader.h index c440962fe522..751fde7fb153 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -517,6 +517,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, void efi_carve_out_dt_rsv(void *fdt); /* Called by bootefi to make console interface available */ efi_status_t efi_console_register(void); +/* Called when a block devices has been probed */ +int efi_disk_create(struct udevice *dev); /* Called by bootefi to make all disk storage accessible as EFI objects */ efi_status_t efi_disk_register(void); /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */ diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index cd5528046251..3fae40e034fb 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -10,6 +10,7 @@ #include <common.h> #include <blk.h> #include <dm.h> +#include <dm/device-internal.h> #include <efi_loader.h> #include <fs.h> #include <log.h> @@ -484,6 +485,7 @@ error: return ret; } +#ifndef CONFIG_BLK /** * efi_disk_create_partitions() - create handles and protocols for partitions * @@ -531,6 +533,96 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, return disks; } +#endif /* CONFIG_BLK */ + +/* + * Create a handle for a whole raw disk + * + * @dev uclass device + * @return 0 on success, -1 otherwise + */ +static int efi_disk_create_raw(struct udevice *dev) +{ + struct efi_disk_obj *disk; + struct blk_desc *desc; + const char *if_typename; + int diskid; + efi_status_t ret; + + desc = dev_get_uclass_plat(dev); + if_typename = blk_get_if_type_name(desc->if_type); + diskid = desc->devnum; + + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc, + diskid, NULL, 0, &disk); + if (ret != EFI_SUCCESS) { + log_err("Adding disk %s%d failed\n", if_typename, diskid); + return -1; + } + disk->dev = dev; + dev->efi_obj = &disk->header; + + return 0; +} + +/* + * Create a handle for a disk partition + * + * @dev uclass device + * @return 0 on success, -1 otherwise + */ +static int efi_disk_create_part(struct udevice *dev) +{ + efi_handle_t parent; + struct blk_desc *desc; + const char *if_typename; + struct disk_part *part_data; + struct disk_partition *info; + unsigned int part; + int diskid; + struct efi_device_path *dp = NULL; + struct efi_disk_obj *disk; + efi_status_t ret; + + parent = dev->parent->efi_obj; + desc = dev_get_uclass_plat(dev->parent); + if_typename = blk_get_if_type_name(desc->if_type); + diskid = desc->devnum; + + part_data = dev_get_uclass_plat(dev); + part = part_data->partnum; + info = &part_data->gpt_part_info; + + /* TODO: should not use desc? */ + dp = efi_dp_from_part(desc, 0); + + ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid, + info, part, &disk); + if (ret != EFI_SUCCESS) { + log_err("Adding partition %s%d:%x failed\n", + if_typename, diskid, part); + return -1; + } + disk->dev = dev; + dev->efi_obj = &disk->header; + + return 0; +} + +int efi_disk_create(struct udevice *dev) +{ + enum uclass_id id; + + id = device_get_uclass_id(dev); + + if (id == UCLASS_BLK) + return efi_disk_create_raw(dev); + + if (id == UCLASS_PARTITION) + return efi_disk_create_part(dev); + + return -1; +} /** * efi_disk_register() - register block devices
Add efi_disk_create() function. Any UEFI handle created by efi_disk_create() can be treated as a efi_disk object, the udevice is either a UCLASS_BLK (a whole raw disk) or UCLASS_PARTITION (a disk partition). So this function is expected to be called every time such an udevice is detected and activated through a device model's "probe" interface. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/efi_loader.h | 2 + lib/efi_loader/efi_disk.c | 92 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) -- 2.33.0