[U-Boot] [PATCH 1/2] disk: Provide API to get partition by name for specific type

[Sam Protsenko]

Hi Simon,

On 24 September 2017 at 19:14, Simon Glass <sjg at chromium.org> wrote:
> Hi Sam,
>
> On 21 September 2017 at 16:51, Sam Protsenko <semen.protsenko at linaro.org> wrote:
>> There is already existing function part_get_info_by_name().
>> But sometimes user is particularly interested in looking for only
>> specific partition type. This patch implements such an API that
>> provides partition searching by name for specified partition type.
>>
>> Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
>> ---
>>  disk/part.c    | 15 +++++++++++++--
>>  include/part.h | 15 +++++++++++++++
>>  2 files changed, 28 insertions(+), 2 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> nit below
>
>>
>> diff --git a/disk/part.c b/disk/part.c
>> index aa9183d696..66b8101f98 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -21,6 +21,9 @@
>>  #define PRINTF(fmt,args...)
>>  #endif
>>
>> +/* Check all partition types */
>> +#define PART_TYPE_ALL          -1
>> +
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>>  #ifdef HAVE_BLOCK_DEVICE
>> @@ -626,8 +629,8 @@ cleanup:
>>         return ret;
>>  }
>>
>> -int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
>> -       disk_partition_t *info)
>> +int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
>> +                              disk_partition_t *info, int part_type)
>>  {
>>         struct part_driver *first_drv =
>>                 ll_entry_start(struct part_driver, part_driver);
>> @@ -638,6 +641,8 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
>>                 int ret;
>>                 int i;
>>                 for (i = 1; i < part_drv->max_entries; i++) {
>> +                       if (part_type >= 0 && part_type != part_drv->part_type)
>> +                               break;
>>                         ret = part_drv->get_info(dev_desc, i, info);
>>                         if (ret != 0) {
>>                                 /* no more entries in table */
>> @@ -652,6 +657,12 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
>>         return -1;
>>  }
>>
>> +int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
>> +                         disk_partition_t *info)
>> +{
>> +       return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL);
>> +}
>> +
>>  void part_set_generic_name(const struct blk_desc *dev_desc,
>>         int part_num, char *name)
>>  {
>> diff --git a/include/part.h b/include/part.h
>> index 86117a7ce5..1a61518722 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -173,6 +173,21 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
>>                             struct blk_desc **dev_desc,
>>                             disk_partition_t *info, int allow_whole_dev);
>>
>> +/**
>> + * part_get_info_by_name_type() - Search for a partition by name
>> + *                                for only specified partition type
>> + *
>> + * @param dev_desc - block device descriptor
>> + * @param gpt_name - the specified table entry name
>> + * @param info - returns the disk partition info
>> + * @param part_type - only search in partitions of this type
>
> Can you reference the PART_TYPE_ #define here (since we don't have an enum).
>

If you mean PART_TYPE_ALL, I'd prefer not to, because for all driver
types you need to use part_get_info_by_name() API instead of new
part_get_info_by_name_type(). Basically I just copied that doxygen
block from part_get_info_by_name(), so if we should fix the
description, we should probably do that for both functions, in
separate patch. We can create corresponding enum in that patch as
well. You agree?

Also, the format of those doxygen comments is a bit wrong, we should
either fix them, or switch to kernel-doc format. Actually, it bring
the question: which format (doxygen or kernel-doc) we should use in
U-Boot? Because right now it's a mix of both. So if you have an idea
how it should look like, please share, so that I can rework this in
one patch.

As for this patch -- I'd really like it to be applied as is, so we can
keep things atomic. Hope it's fine with you?

Thanks.

>> + *
>> + * @return - the partition number on match (starting on 1), -1 on no match,
>> + * otherwise error
>> + */
>> +int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name,
>> +                              disk_partition_t *info, int part_type);
>> +
>>  /**
>>   * part_get_info_by_name() - Search for a partition by name
>>   *                           among all available registered partitions
>> --
>> 2.14.1
>>
>
> Regards,
> Simon