Heinrich, On Mon, Apr 27, 2020 at 09:57:25PM +0200, Heinrich Schuchardt wrote: > On 4/27/20 11:48 AM, AKASHI Takahiro wrote: > > This function will check if a given handle to device is a EFI system > > partition. It will be utilised in implementing capsule-on-disk feature. > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > Just a reminder for me: > > This patch depends on > "part: detect EFI system partition" > https://lists.denx.de/pipermail/u-boot/2020-April/408150.html
Yes, and I explicitly mentioned it in "prerequisite patches" of the cover letter. (I objected to the patch 2/2 though.) > > --- > > include/efi_loader.h | 2 ++ > > lib/efi_loader/efi_disk.c | 22 ++++++++++++++++++++++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > > index b7bccf50b30c..d4510462d616 100644 > > --- a/include/efi_loader.h > > +++ b/include/efi_loader.h > > @@ -393,6 +393,8 @@ efi_status_t efi_disk_register(void); > > int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, > > const char *if_typename, int diskid, > > const char *pdevname); > > +/* Check if it is EFI system partition */ > > +bool efi_disk_is_system_part(efi_handle_t handle); > > /* Called by bootefi to make GOP (graphical) interface available */ > > efi_status_t efi_gop_register(void); > > /* Called by bootefi to make the network interface available */ > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > index fd3df80b0b96..9f58a8642c85 100644 > > --- a/lib/efi_loader/efi_disk.c > > +++ b/lib/efi_loader/efi_disk.c > > @@ -588,3 +588,25 @@ efi_status_t efi_disk_register(void) > > > > return EFI_SUCCESS; > > } > > + > > Please, provide a function description, cf. > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation That is the thing that I've forgot to address in this version. > > +bool efi_disk_is_system_part(efi_handle_t handle) > > +{ > > + struct efi_handler *handler; > > + struct efi_disk_obj *diskobj; > > + disk_partition_t info; > > + efi_status_t ret; > > + int r; > > + > > + /* check if this is a block device */ > > + ret = efi_search_protocol(handle, &efi_block_io_guid, &handler); > > + if (ret != EFI_SUCCESS) > > + return false; > > + > > + diskobj = container_of(handle, struct efi_disk_obj, header); > > This is just a complicated way of saying: > > diskobj = (struct efi_disk_obj *)handle; > > I would rather avoid container_of() here though it is not wrong. My code doesn't rely on the fact that 'handle' is the first element of 'struct efi_disk_obj' So use of container_of has no disadvantage. Thanks, -Takahiro Akashi > Best regards > > Heinrich > > > + > > + r = part_get_info(diskobj->desc, diskobj->part, &info); > > + if (r) > > + return false; > > + > > + return info.bootable & PART_EFI_SYSTEM_PARTITION; > > +} > >