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

2022-01-05 Thread AKASHI Takahiro 
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 
> > ---
> >   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 **)))
> > 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);
>

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

2022-01-02 Thread Heinrich Schuchardt 

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

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

2021-12-13 Thread Simon Glass 
On Thu, 9 Dec 2021 at 23:58, 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
> 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 
> ---
>  lib/efi_loader/efi_disk.c | 22 +-
>  1 file changed, 17 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass

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

2021-12-09 Thread AKASHI Takahiro 
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 
---
 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 **)))
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);
-- 
2.33.0