[PATCH v2 32/32] lmb: add logic to print lmb flag strings

[Simon Glass]

Hi Sughosh,

On Wed, 14 Aug 2024 at 05:03, 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 V1: New patch
>
>  lib/lmb.c         | 18 ++++++++++++++++--
>  test/cmd/bdinfo.c |  4 ++--
>  2 files changed, 18 insertions(+), 4 deletions(-)

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

But see below

>
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 37d2a72951..5c5b3e9bb5 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -26,6 +26,19 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  static struct lmb lmb;
>
> +static void print_region_flags(enum lmb_flags flags)
> +{
> +       uint64_t bitpos;
> +       const char *flag_str[] = { "LMB_NONE", "LMB_NOMAP", "LMB_NOOVERWRITE" };

As mentioned, LMB_NONE shouldn't be a flag. For the other two, how
about "no-map" and "no-overwrite"?

> +
> +       do {
> +               bitpos = fls(flags) - 1;
> +               printf("%s", flag_str[bitpos]);
> +               flags &= ~(1ull << bitpos);
> +               flags ? puts(", ") : puts("\n");
> +       } 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);
> +               print_region_flags(flags);
>         }
>  }
>
> diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
> index 7dd3f7ca5b..887defc28a 100644
> --- a/test/cmd/bdinfo.c
> +++ b/test/cmd/bdinfo.c
> @@ -120,8 +120,8 @@ static int lmb_test_dump_region(struct unit_test_state *uts,
>                         ut_assert_nextlinen(" %s[%d]\t[", name, i);
>                         continue;
>                 }
> -               ut_assert_nextline(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x",
> -                                  name, i, base, end, size, flags);
> +               ut_assert_nextlinen(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: ",
> +                                  name, i, base, end, size);
>         }
>
>         return 0;
> --
> 2.34.1
>

Regards,
Simon