On 5/1/20 9:06 AM, Heinrich Schuchardt wrote: > On 4/28/20 1:54 AM, AKASHI Takahiro wrote: >> 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' > > It is efi_delete_handle() that relies on handle pointing to the start of > the EFI objects. > >> So use of container_of has no disadvantage. > > The generated assembler code is the same. > >> >> 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; > > As you decided that the return value shall be boolean you have to use: > > return !!(info.bootable & PART_EFI_SYSTEM_PARTITION); > > Best regards > > Heinrich > >>>> +} >>>> >
This patch is merged adding function description and adjusting return to be bool. Thanks Heinrich