| Message ID | 20211210064947.73361-20-takahiro.akashi@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: more tightly integrate UEFI disks to driver model | expand |
On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > When we create an efi_disk device with an UEFI application using driver > binding protocol, the 'efi_driver' framework tries to create > a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to > calling a PROBE callback, efi_disk_probe(). > In this case, however, we don't need to create another "efi_disk" device > as we already have this device instance. > > So we should avoid recursively invoke further processing in the callback > function. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/efi_disk.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org>
On 12/10/21 07:49, AKASHI Takahiro wrote: > When we create an efi_disk device with an UEFI application using driver > binding protocol, the 'efi_driver' framework tries to create > a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to I assume this is IF_TYPE_EFI_LOADER now? Best regards Heinrich > calling a PROBE callback, efi_disk_probe(). > In this case, however, we don't need to create another "efi_disk" device > as we already have this device instance. > > So we should avoid recursively invoke further processing in the callback > function. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/efi_disk.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > index 8e33af76711f..f37f8c1ab0f1 100644 > --- a/lib/efi_loader/efi_disk.c > +++ b/lib/efi_loader/efi_disk.c > @@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event) > { > struct udevice *dev; > enum uclass_id id; > + struct blk_desc *desc; > struct udevice *child; > int ret; > > @@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event) > return 0; > } > > - ret = efi_disk_create_raw(dev); > - if (ret) > - return -1; > + /* > + * avoid creating duplicated objects now that efi_driver > + * has already created an efi_disk at this moment. > + */ > + desc = dev_get_uclass_plat(dev); > + if (desc->if_type != IF_TYPE_EFI) { > + ret = efi_disk_create_raw(dev); > + if (ret) > + return -1; > + } > > device_foreach_child(child, dev) { > ret = efi_disk_create_part(child); > @@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event) > static int efi_disk_delete_raw(struct udevice *dev) > { > efi_handle_t handle; > + struct blk_desc *desc; > struct efi_disk_obj *diskobj; > > if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) > return -1; > > - diskobj = container_of(handle, struct efi_disk_obj, header); > - efi_free_pool(diskobj->dp); > + desc = dev_get_uclass_plat(dev); > + if (desc->if_type != IF_TYPE_EFI) { > + diskobj = container_of(handle, struct efi_disk_obj, header); > + efi_free_pool(diskobj->dp); > + } > > efi_delete_handle(handle); > dev_tag_del(dev, DM_TAG_EFI);
On Sun, Jan 02, 2022 at 10:19:21AM +0100, Heinrich Schuchardt wrote: > On 12/10/21 07:49, AKASHI Takahiro wrote: > > When we create an efi_disk device with an UEFI application using driver > > binding protocol, the 'efi_driver' framework tries to create > > a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to > > I assume this is IF_TYPE_EFI_LOADER now? Yes in v2022.01-rc4 and later. -Takahiro Akashi > Best regards > > Heinrich > > > calling a PROBE callback, efi_disk_probe(). > > In this case, however, we don't need to create another "efi_disk" device > > as we already have this device instance. > > > > So we should avoid recursively invoke further processing in the callback > > function. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > lib/efi_loader/efi_disk.c | 22 +++++++++++++++++----- > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > index 8e33af76711f..f37f8c1ab0f1 100644 > > --- a/lib/efi_loader/efi_disk.c > > +++ b/lib/efi_loader/efi_disk.c > > @@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event) > > { > > struct udevice *dev; > > enum uclass_id id; > > + struct blk_desc *desc; > > struct udevice *child; > > int ret; > > > > @@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event) > > return 0; > > } > > > > - ret = efi_disk_create_raw(dev); > > - if (ret) > > - return -1; > > + /* > > + * avoid creating duplicated objects now that efi_driver > > + * has already created an efi_disk at this moment. > > + */ > > + desc = dev_get_uclass_plat(dev); > > + if (desc->if_type != IF_TYPE_EFI) { > > + ret = efi_disk_create_raw(dev); > > + if (ret) > > + return -1; > > + } > > > > device_foreach_child(child, dev) { > > ret = efi_disk_create_part(child); > > @@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event) > > static int efi_disk_delete_raw(struct udevice *dev) > > { > > efi_handle_t handle; > > + struct blk_desc *desc; > > struct efi_disk_obj *diskobj; > > > > if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) > > return -1; > > > > - diskobj = container_of(handle, struct efi_disk_obj, header); > > - efi_free_pool(diskobj->dp); > > + desc = dev_get_uclass_plat(dev); > > + if (desc->if_type != IF_TYPE_EFI) { > > + diskobj = container_of(handle, struct efi_disk_obj, header); > > + efi_free_pool(diskobj->dp); > > + } > > > > efi_delete_handle(handle); > > dev_tag_del(dev, DM_TAG_EFI); >
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8e33af76711f..f37f8c1ab0f1 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -603,6 +603,7 @@ static int efi_disk_probe(void *ctx, struct event *event) { struct udevice *dev; enum uclass_id id; + struct blk_desc *desc; struct udevice *child; int ret; @@ -616,9 +617,16 @@ static int efi_disk_probe(void *ctx, struct event *event) return 0; } - ret = efi_disk_create_raw(dev); - if (ret) - return -1; + /* + * avoid creating duplicated objects now that efi_driver + * has already created an efi_disk at this moment. + */ + desc = dev_get_uclass_plat(dev); + if (desc->if_type != IF_TYPE_EFI) { + ret = efi_disk_create_raw(dev); + if (ret) + return -1; + } device_foreach_child(child, dev) { ret = efi_disk_create_part(child); @@ -642,13 +650,17 @@ static int efi_disk_probe(void *ctx, struct event *event) static int efi_disk_delete_raw(struct udevice *dev) { efi_handle_t handle; + struct blk_desc *desc; struct efi_disk_obj *diskobj; if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) return -1; - diskobj = container_of(handle, struct efi_disk_obj, header); - efi_free_pool(diskobj->dp); + desc = dev_get_uclass_plat(dev); + if (desc->if_type != IF_TYPE_EFI) { + diskobj = container_of(handle, struct efi_disk_obj, header); + efi_free_pool(diskobj->dp); + } efi_delete_handle(handle); dev_tag_del(dev, DM_TAG_EFI);
When we create an efi_disk device with an UEFI application using driver binding protocol, the 'efi_driver' framework tries to create a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to calling a PROBE callback, efi_disk_probe(). In this case, however, we don't need to create another "efi_disk" device as we already have this device instance. So we should avoid recursively invoke further processing in the callback function. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_loader/efi_disk.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)