| Message ID | 20240821105839.2870293-28-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make LMB memory map global and persistent | expand |
Hi Sughosh, On Wed, 21 Aug 2024 at 05:00, Sughosh Ganu <sughosh.ganu@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@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@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
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" }; + + do { + bitpos = flags ? fls(flags) - 1 : 0; + 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); + lmb_print_region_flags(flags); } }
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@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(-)