On Wed, 21 Aug 2024 at 07:41, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 20 Aug 2024 at 04:23, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Fri, 16 Aug 2024 at 02:03, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Wed, 14 Aug 2024 at 05:03, 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 V1: New patch
lib/lmb.c | 18 ++++++++++++++++-- test/cmd/bdinfo.c | 4 ++-- 2 files changed, 18 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@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"?
So, you don't want any value to be shown with LMB_NONE? I guess LMB_NONE is indicative of the fact that the region does not have any attributes, no?
That's my understanding, yes. This could be a later cleanup I suppose, but since you are adding flags, you may as well remove this one.
Maybe I am not getting your point. But we have the flags member in the struct lmb_region, and a region without any flags set will have a value of 0. And we are calling that LMB_NONE. So, if we consider the boot_fdt_reserve_region() function, we need to pass a flags argument to the function. And a value of 0 would mean LMB_NONE. Is it not instructive to show a value of 0 as LMB_NONE(okay, maybe not in uppercase), instead of having to just print 0? Not printing anything is a little confusing imo.
-sughosh
[..]
Regards, Simon