[PATCH v3 27/27] lmb: add logic to print lmb flag strings

[Simon Glass]

Hi Sughosh,

On Wed, 21 Aug 2024 at 05:00, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> Instead of printing the LMB flags as numerical values, print them as
> strings. This makes it easier to understand what flags are associated
> with the lmb region. Also make corresponding changes to the bdinfo
> command's test code.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> Changes since V2:
> * Change the logic to accomodate new value of LMB_NONE(0).
> * s/print_region_flags()/lmb_print_region_flags().
> * s/"LMB_NOMAP"/"no-map".
> * s/"LMB_NOOVERWRITE"/"no-overwrite".
>
>  lib/lmb.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

nit below

>
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 11959760b6..0379798837 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -26,6 +26,19 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  static struct lmb lmb;
>
> +static void lmb_print_region_flags(enum lmb_flags flags)
> +{
> +       uint64_t bitpos;
> +       const char *flag_str[] = { "none", "no-map", "no-overwrite" };

This implicitly follows the enum values, but I think that is fine
given that this is debug / dump code.

> +
> +       do {
> +               bitpos = flags ? fls(flags) - 1 : 0;
> +               printf("%s", flag_str[bitpos]);
> +               flags &= ~(1ull << bitpos);
> +               flags ? puts(", ") : puts("\n");

That line is pretty uncommon C :-) How about putting the ? inside the puts()?

> +       } while (flags);
> +}
> +
>  static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name)
>  {
>         struct lmb_region *rgn = lmb_rgn_lst->data;
> @@ -41,8 +54,9 @@ static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name)
>                 end = base + size - 1;
>                 flags = rgn[i].flags;
>
> -               printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x\n",
> -                      name, i, base, end, size, flags);
> +               printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: ",
> +                      name, i, base, end, size);
> +               lmb_print_region_flags(flags);
>         }
>  }
>
> --
> 2.34.1
>

Regards,
Simon