diff mbox series

[RFC,v2,19/20] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI) devices

Message ID 20211210064947.73361-20-takahiro.akashi@linaro.org
State Superseded
Headers show
Series efi_loader: more tightly integrate UEFI disks to driver model | expand

Commit Message

AKASHI Takahiro Dec. 10, 2021, 6:49 a.m. UTC
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(-)

Comments

Simon Glass Dec. 13, 2021, 12:51 p.m. UTC | #1
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>
Heinrich Schuchardt Jan. 2, 2022, 9:19 a.m. UTC | #2
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);
AKASHI Takahiro Jan. 6, 2022, 6:59 a.m. UTC | #3
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 mbox series

Patch

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);