Hi Sughosh,
On Wed, 14 Aug 2024 at 05:03, Sughosh Ganu <sughosh.g...@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.g...@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 <s...@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
