[PATCH v2 1/3] part: add part_get_info_cached() API

[Ilias Apalodimas]

Hi Neil,

On Tue, 8 Apr 2025 at 12:13, Neil Armstrong <neil.armstrong at linaro.org> wrote:
>
> Introduce a new part_get_info_cached() API that's used to
> get the part_info of a blk_desc allowing to use an eventual
> partition scanning cache to avoid rescanning the entire disk
> partition scheme for each partition number.
>
> The part_get_info_cached_free() is also added to hint the
> partition code to flush any cached data from this disk.
>
> The equivalent ops are added that directly maps to those
> added functions.
>
> This API is designed to be used as a direct replacement of
> part_get_info() in codes scanning all partitions for a same
> disk, like in blk-uclass. With this, a lot of unnecessary
> computation is saved, leading to a faster boot time when
> partitions are scanned, especially with storage medias
> with potentially multiple large hardware partitions like
> UFS, NVMe or eMMC.
>
> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
> ---
>  disk/part.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
>  include/part.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 303178161c083ec6e1b767b4f06ac5773576ca60..1d09c0511c75d457c81cab040c3f5caa924ee945 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -335,8 +335,8 @@ void part_print(struct blk_desc *desc)
>                 drv->print(desc);
>  }
>
> -int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
> -                         struct disk_partition *info)
> +static int _part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
> +                                 struct disk_partition *info, bool cached)
>  {
>         struct part_driver *drv;
>
> @@ -356,24 +356,57 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>                               desc->part_type);
>                         return -EPROTONOSUPPORT;
>                 }
> -               if (!drv->get_info) {
> -                       PRINTF("## Driver %s does not have the get_info() method\n",
> -                              drv->name);
> -                       return -ENOSYS;
> -               }
> -               if (drv->get_info(desc, part, info) == 0) {
> -                       PRINTF("## Valid %s partition found ##\n", drv->name);
> -                       return 0;
> +               if (cached && drv->get_info_cached) {
> +                       if (drv->get_info_cached(desc, part, info) == 0) {
> +                               PRINTF("## Valid %s partition found ##\n", drv->name);
> +                               return 0;
> +                       }
> +               } else {
> +                       if (!drv->get_info) {
> +                               PRINTF("## Driver %s does not have the get_info() method\n",
> +                                      drv->name);
> +                               return -ENOSYS;
> +                       }
> +                       if (drv->get_info(desc, part, info) == 0) {
> +                               PRINTF("## Valid %s partition found ##\n", drv->name);
> +                               return 0;
> +                       }
>                 }

That's fine but since you'll send a v3 with a cleanup on top mind also
switching the logic around on this?

if (!blk_enabled())
    return -ENOENT; etc

This will make it a bit more readable

>         }
>
>         return -ENOENT;
>  }
>
> +int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
> +                         struct disk_partition *info)
> +{
> +       return _part_get_info_by_type(desc, part, part_type, info, false);
> +}
> +
> +int part_get_info_cached(struct blk_desc *desc, int part,
> +                        struct disk_partition *info)
> +{
> +       return _part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info, true);
> +}
> +
>  int part_get_info(struct blk_desc *desc, int part,
>                   struct disk_partition *info)
>  {
> -       return part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info);
> +       return _part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info, false);
> +}
> +
> +void part_get_info_cached_free(struct blk_desc *desc)
> +{
> +       struct part_driver *drv;
> +
> +       if (blk_enabled()) {

Same here
if (!blk_enabled())
    return;
etc

[...]

Other than that LGTM

Cheers
/Ilias