All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/14] Correct confusing lmb error message
@ 2023-09-01  1:13 Simon Glass
  2023-09-01  1:13 ` [PATCH v2 01/14] lmb: Drop surplus brackets and fix code style Simon Glass
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Bin Meng, Brandon Maier,
	Dzmitry Sankouski, Heinrich Schuchardt, Kautuk Consul,
	Leo Yu-Chi Liang, Marek Vasut, Mark Kettenis, Michal Simek,
	Nikhil M Jain, Oleksandr Suvorov, Patrice Chotard,
	Patrick Delaunay, Sam Shih, Sjoerd Simons, Sumit Garg, Wei Fu,
	Yixun Lan, uboot-stm32

I ran into a very confusing error message about overlapping memory. I
found that the message is not correct, so this series refactors the lmb
code a little to permit the real message to be displayed, which is that
the internal lmb table has overflowed.

It also tidies up the code a little.

Feedback on the initial series showed some confusion between one type of
region and another. I had the same confusion, so this series renames the
inner 'region' to 'area'.

Changes in v2:
- Add new patch to rename region array to area
- Add new patch to rename memory/reserved_regions to area
- Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
- Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
- Add new patch to update use of region in the header file
- Add new patch to update use of region in the C file

Simon Glass (14):
  lmb: Drop surplus brackets and fix code style
  lmb: Rename interior piece to area
  lmb: Rename region array to area
  lmb: Rename memory/reserved_regions to area
  lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
  lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
  lmb: Update use of region in the header file
  lmb: Update use of region in the C file
  lmb: Tidy up structure access
  lmb: Avoid long for loop counters and function returns
  lmb: Change functions returning long to return int
  lmb: Tidy up lmb_overlaps_region()
  lmb: Document and tidy lmb_add_region_flags()
  fs: Fix a confusing error about overlapping regions

 configs/a3y17lte_defconfig           |   2 +-
 configs/a5y17lte_defconfig           |   2 +-
 configs/a7y17lte_defconfig           |   2 +-
 configs/apple_m1_defconfig           |   2 +-
 configs/dragonboard845c_defconfig    |   2 +-
 configs/mt7981_emmc_rfb_defconfig    |   2 +-
 configs/mt7981_rfb_defconfig         |   2 +-
 configs/mt7981_sd_rfb_defconfig      |   2 +-
 configs/mt7986_rfb_defconfig         |   2 +-
 configs/mt7986a_bpir3_emmc_defconfig |   2 +-
 configs/mt7986a_bpir3_sd_defconfig   |   2 +-
 configs/qcs404evb_defconfig          |   2 +-
 configs/starqltechn_defconfig        |   2 +-
 configs/stm32mp13_defconfig          |   6 +-
 configs/stm32mp15_basic_defconfig    |   6 +-
 configs/stm32mp15_defconfig          |   6 +-
 configs/stm32mp15_trusted_defconfig  |   6 +-
 configs/th1520_lpi4a_defconfig       |   2 +-
 fs/fs.c                              |   7 +-
 include/lmb.h                        | 105 ++++----
 lib/Kconfig                          |  30 +--
 lib/lmb.c                            | 349 +++++++++++++++------------
 test/cmd/bdinfo.c                    |   6 +-
 test/lib/lmb.c                       | 150 ++++++------
 24 files changed, 382 insertions(+), 317 deletions(-)

-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 01/14] lmb: Drop surplus brackets and fix code style
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 02/14] lmb: Rename interior piece to area Simon Glass
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Use a blank line before the final return. Avoid using too many brackets
to avoid confusion about precedence. Fix some overly long lines.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/lmb.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index b2c233edb64e..ae1969893f00 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -60,7 +60,7 @@ static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
 	const phys_addr_t base1_end = base1 + size1 - 1;
 	const phys_addr_t base2_end = base2 + size2 - 1;
 
-	return ((base1 <= base2_end) && (base2 <= base1_end));
+	return base1 <= base2_end && base2 <= base1_end;
 }
 
 static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
@@ -278,7 +278,7 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 		}
 	}
 
-	if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
+	if (i < rgn->cnt - 1 && lmb_regions_adjacent(rgn, i, i + 1)) {
 		if (rgn->region[i].flags == rgn->region[i + 1].flags) {
 			lmb_coalesce_regions(rgn, i, i + 1);
 			coalesced++;
@@ -375,6 +375,7 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 	 * beginging of the hole and add the region after hole.
 	 */
 	rgn->region[i].size = base - rgn->region[i].base;
+
 	return lmb_add_region_flags(rgn, end + 1, rgnend - end,
 				    rgn->region[i].flags);
 }
@@ -404,7 +405,7 @@ static long lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
 			break;
 	}
 
-	return (i < rgn->cnt) ? i : -1;
+	return i < rgn->cnt ? i : -1;
 }
 
 phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align)
@@ -412,7 +413,8 @@ phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align)
 	return lmb_alloc_base(lmb, size, align, LMB_ALLOC_ANYWHERE);
 }
 
-phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phys_addr_t max_addr)
+phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
+			   phys_addr_t max_addr)
 {
 	phys_addr_t alloc;
 
@@ -430,7 +432,8 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size)
 	return addr & ~(size - 1);
 }
 
-phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phys_addr_t max_addr)
+phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
+			     phys_addr_t max_addr)
 {
 	long i, rgn;
 	phys_addr_t base = 0;
@@ -468,6 +471,7 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align, phy
 			base = lmb_align_down(res_base - size, align);
 		}
 	}
+
 	return 0;
 }
 
@@ -494,6 +498,7 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 				return base;
 		}
 	}
+
 	return 0;
 }
 
@@ -521,6 +526,7 @@ phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr)
 		return lmb->memory.region[lmb->memory.cnt - 1].base +
 		       lmb->memory.region[lmb->memory.cnt - 1].size - addr;
 	}
+
 	return 0;
 }
 
@@ -534,6 +540,7 @@ int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t addr, int flags)
 		if ((addr >= lmb->reserved.region[i].base) && (addr <= upper))
 			return (lmb->reserved.region[i].flags & flags) == flags;
 	}
+
 	return 0;
 }
 
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 02/14] lmb: Rename interior piece to area
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
  2023-09-01  1:13 ` [PATCH v2 01/14] lmb: Drop surplus brackets and fix code style Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  6:17   ` Heinrich Schuchardt
  2023-09-01  1:13 ` [PATCH v2 03/14] lmb: Rename region array " Simon Glass
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay

The lmb data structures use the word 'region' to describe the region in
which the allocations happen, as well as the individual allocations in
that region. The interior structure is called lmb_property but is
described as a region.

This is quite confusing. One of the reviewers in v1 asked why we are using
a property to describe a region!

It seems better to adopt the word 'area' for the internal pieces, since an
area is smaller than a region.

As a first step to improve this, rename struct lmb_property to lmb_area.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/lmb.h  | 12 ++++++------
 test/lib/lmb.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 231b68b27d91..4b7664f22c1d 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -23,13 +23,13 @@ enum lmb_flags {
 };
 
 /**
- * struct lmb_property - Description of one region.
+ * struct lmb_area - Description of one region.
  *
  * @base:	Base address of the region.
  * @size:	Size of the region
  * @flags:	memory region attributes
  */
-struct lmb_property {
+struct lmb_area {
 	phys_addr_t base;
 	phys_size_t size;
 	enum lmb_flags flags;
@@ -64,9 +64,9 @@ struct lmb_region {
 	unsigned long cnt;
 	unsigned long max;
 #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_property region[CONFIG_LMB_MAX_REGIONS];
+	struct lmb_area region[CONFIG_LMB_MAX_REGIONS];
 #else
-	struct lmb_property *region;
+	struct lmb_area *region;
 #endif
 };
 
@@ -87,8 +87,8 @@ struct lmb {
 	struct lmb_region memory;
 	struct lmb_region reserved;
 #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
-	struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+	struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
+	struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
 #endif
 };
 
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index 162887503588..59b140fde4ce 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -12,7 +12,7 @@
 #include <test/test.h>
 #include <test/ut.h>
 
-static inline bool lmb_is_nomap(struct lmb_property *m)
+static inline bool lmb_is_nomap(struct lmb_area *m)
 {
 	return m->flags & LMB_NOMAP;
 }
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 03/14] lmb: Rename region array to area
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
  2023-09-01  1:13 ` [PATCH v2 01/14] lmb: Drop surplus brackets and fix code style Simon Glass
  2023-09-01  1:13 ` [PATCH v2 02/14] lmb: Rename interior piece to area Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 04/14] lmb: Rename memory/reserved_regions " Simon Glass
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Bin Meng, Heinrich Schuchardt,
	Marek Vasut, Michal Simek, Nikhil M Jain, Patrick Delaunay,
	Sjoerd Simons

Adjust this so that the struct member matches its new name. For now, some
of the comments are wrong, but it difficult to do everything at once.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to rename region array to area

 include/lmb.h     |  14 +++---
 lib/lmb.c         | 112 +++++++++++++++++++++++-----------------------
 test/cmd/bdinfo.c |   6 +--
 test/lib/lmb.c    |  62 ++++++++++++-------------
 4 files changed, 97 insertions(+), 97 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 4b7664f22c1d..9d4a05670c23 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -23,11 +23,11 @@ enum lmb_flags {
 };
 
 /**
- * struct lmb_area - Description of one region.
+ * struct lmb_area - Description of one area within a region
  *
- * @base:	Base address of the region.
- * @size:	Size of the region
- * @flags:	memory region attributes
+ * @base:	Base address of the area
+ * @size:	Size of the area
+ * @flags:	memory-area attributes
  */
 struct lmb_area {
 	phys_addr_t base;
@@ -58,15 +58,15 @@ struct lmb_area {
  *
  * @cnt: Number of regions.
  * @max: Size of the region array, max value of cnt.
- * @region: Array of the region properties
+ * @area: Array of the areas within the region
  */
 struct lmb_region {
 	unsigned long cnt;
 	unsigned long max;
 #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_area region[CONFIG_LMB_MAX_REGIONS];
+	struct lmb_area area[CONFIG_LMB_MAX_REGIONS];
 #else
-	struct lmb_area *region;
+	struct lmb_area *area;
 #endif
 };
 
diff --git a/lib/lmb.c b/lib/lmb.c
index ae1969893f00..9336792df8ba 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -30,10 +30,10 @@ static void lmb_dump_region(struct lmb_region *rgn, char *name)
 	printf(" %s.cnt = 0x%lx / max = 0x%lx\n", name, rgn->cnt, rgn->max);
 
 	for (i = 0; i < rgn->cnt; i++) {
-		base = rgn->region[i].base;
-		size = rgn->region[i].size;
+		base = rgn->area[i].base;
+		size = rgn->area[i].size;
 		end = base + size - 1;
-		flags = rgn->region[i].flags;
+		flags = rgn->area[i].flags;
 
 		printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x\n",
 		       name, i, base, end, size, flags);
@@ -77,10 +77,10 @@ static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
 static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
 				 unsigned long r2)
 {
-	phys_addr_t base1 = rgn->region[r1].base;
-	phys_size_t size1 = rgn->region[r1].size;
-	phys_addr_t base2 = rgn->region[r2].base;
-	phys_size_t size2 = rgn->region[r2].size;
+	phys_addr_t base1 = rgn->area[r1].base;
+	phys_size_t size1 = rgn->area[r1].size;
+	phys_addr_t base2 = rgn->area[r2].base;
+	phys_size_t size2 = rgn->area[r2].size;
 
 	return lmb_addrs_adjacent(base1, size1, base2, size2);
 }
@@ -90,9 +90,9 @@ static void lmb_remove_region(struct lmb_region *rgn, unsigned long r)
 	unsigned long i;
 
 	for (i = r; i < rgn->cnt - 1; i++) {
-		rgn->region[i].base = rgn->region[i + 1].base;
-		rgn->region[i].size = rgn->region[i + 1].size;
-		rgn->region[i].flags = rgn->region[i + 1].flags;
+		rgn->area[i].base = rgn->area[i + 1].base;
+		rgn->area[i].size = rgn->area[i + 1].size;
+		rgn->area[i].flags = rgn->area[i + 1].flags;
 	}
 	rgn->cnt--;
 }
@@ -101,7 +101,7 @@ static void lmb_remove_region(struct lmb_region *rgn, unsigned long r)
 static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
 				 unsigned long r2)
 {
-	rgn->region[r1].size += rgn->region[r2].size;
+	rgn->area[r1].size += rgn->area[r2].size;
 	lmb_remove_region(rgn, r2);
 }
 
@@ -235,18 +235,18 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 	long adjacent, i;
 
 	if (rgn->cnt == 0) {
-		rgn->region[0].base = base;
-		rgn->region[0].size = size;
-		rgn->region[0].flags = flags;
+		rgn->area[0].base = base;
+		rgn->area[0].size = size;
+		rgn->area[0].flags = flags;
 		rgn->cnt = 1;
 		return 0;
 	}
 
 	/* First try and coalesce this LMB with another. */
 	for (i = 0; i < rgn->cnt; i++) {
-		phys_addr_t rgnbase = rgn->region[i].base;
-		phys_size_t rgnsize = rgn->region[i].size;
-		phys_size_t rgnflags = rgn->region[i].flags;
+		phys_addr_t rgnbase = rgn->area[i].base;
+		phys_size_t rgnsize = rgn->area[i].size;
+		phys_size_t rgnflags = rgn->area[i].flags;
 		phys_addr_t end = base + size - 1;
 		phys_addr_t rgnend = rgnbase + rgnsize - 1;
 
@@ -262,14 +262,14 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 		if (adjacent > 0) {
 			if (flags != rgnflags)
 				break;
-			rgn->region[i].base -= size;
-			rgn->region[i].size += size;
+			rgn->area[i].base -= size;
+			rgn->area[i].size += size;
 			coalesced++;
 			break;
 		} else if (adjacent < 0) {
 			if (flags != rgnflags)
 				break;
-			rgn->region[i].size += size;
+			rgn->area[i].size += size;
 			coalesced++;
 			break;
 		} else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) {
@@ -279,7 +279,7 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 	}
 
 	if (i < rgn->cnt - 1 && lmb_regions_adjacent(rgn, i, i + 1)) {
-		if (rgn->region[i].flags == rgn->region[i + 1].flags) {
+		if (rgn->area[i].flags == rgn->area[i + 1].flags) {
 			lmb_coalesce_regions(rgn, i, i + 1);
 			coalesced++;
 		}
@@ -292,22 +292,22 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 
 	/* Couldn't coalesce the LMB, so add it to the sorted table. */
 	for (i = rgn->cnt-1; i >= 0; i--) {
-		if (base < rgn->region[i].base) {
-			rgn->region[i + 1].base = rgn->region[i].base;
-			rgn->region[i + 1].size = rgn->region[i].size;
-			rgn->region[i + 1].flags = rgn->region[i].flags;
+		if (base < rgn->area[i].base) {
+			rgn->area[i + 1].base = rgn->area[i].base;
+			rgn->area[i + 1].size = rgn->area[i].size;
+			rgn->area[i + 1].flags = rgn->area[i].flags;
 		} else {
-			rgn->region[i + 1].base = base;
-			rgn->region[i + 1].size = size;
-			rgn->region[i + 1].flags = flags;
+			rgn->area[i + 1].base = base;
+			rgn->area[i + 1].size = size;
+			rgn->area[i + 1].flags = flags;
 			break;
 		}
 	}
 
-	if (base < rgn->region[0].base) {
-		rgn->region[0].base = base;
-		rgn->region[0].size = size;
-		rgn->region[0].flags = flags;
+	if (base < rgn->area[0].base) {
+		rgn->area[0].base = base;
+		rgn->area[0].size = size;
+		rgn->area[0].flags = flags;
 	}
 
 	rgn->cnt++;
@@ -340,8 +340,8 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 
 	/* Find the region where (base, size) belongs to */
 	for (i = 0; i < rgn->cnt; i++) {
-		rgnbegin = rgn->region[i].base;
-		rgnend = rgnbegin + rgn->region[i].size - 1;
+		rgnbegin = rgn->area[i].base;
+		rgnend = rgnbegin + rgn->area[i].size - 1;
 
 		if ((rgnbegin <= base) && (end <= rgnend))
 			break;
@@ -359,14 +359,14 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 
 	/* Check to see if region is matching at the front */
 	if (rgnbegin == base) {
-		rgn->region[i].base = end + 1;
-		rgn->region[i].size -= size;
+		rgn->area[i].base = end + 1;
+		rgn->area[i].size -= size;
 		return 0;
 	}
 
 	/* Check to see if the region is matching at the end */
 	if (rgnend == end) {
-		rgn->region[i].size -= size;
+		rgn->area[i].size -= size;
 		return 0;
 	}
 
@@ -374,10 +374,10 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 	 * We need to split the entry -  adjust the current one to the
 	 * beginging of the hole and add the region after hole.
 	 */
-	rgn->region[i].size = base - rgn->region[i].base;
+	rgn->area[i].size = base - rgn->area[i].base;
 
 	return lmb_add_region_flags(rgn, end + 1, rgnend - end,
-				    rgn->region[i].flags);
+				    rgn->area[i].flags);
 }
 
 long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
@@ -399,8 +399,8 @@ static long lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
 	unsigned long i;
 
 	for (i = 0; i < rgn->cnt; i++) {
-		phys_addr_t rgnbase = rgn->region[i].base;
-		phys_size_t rgnsize = rgn->region[i].size;
+		phys_addr_t rgnbase = rgn->area[i].base;
+		phys_size_t rgnsize = rgn->area[i].size;
 		if (lmb_addrs_overlap(base, size, rgnbase, rgnsize))
 			break;
 	}
@@ -440,8 +440,8 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 	phys_addr_t res_base;
 
 	for (i = lmb->memory.cnt - 1; i >= 0; i--) {
-		phys_addr_t lmbbase = lmb->memory.region[i].base;
-		phys_size_t lmbsize = lmb->memory.region[i].size;
+		phys_addr_t lmbbase = lmb->memory.area[i].base;
+		phys_size_t lmbsize = lmb->memory.area[i].size;
 
 		if (lmbsize < size)
 			continue;
@@ -465,7 +465,7 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 					return 0;
 				return base;
 			}
-			res_base = lmb->reserved.region[rgn].base;
+			res_base = lmb->reserved.area[rgn].base;
 			if (res_base < size)
 				break;
 			base = lmb_align_down(res_base - size, align);
@@ -490,8 +490,8 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 		 * Check if the requested end address is in the same memory
 		 * region we found.
 		 */
-		if (lmb_addrs_overlap(lmb->memory.region[rgn].base,
-				      lmb->memory.region[rgn].size,
+		if (lmb_addrs_overlap(lmb->memory.area[rgn].base,
+				      lmb->memory.area[rgn].size,
 				      base + size - 1, 1)) {
 			/* ok, reserve the memory */
 			if (lmb_reserve(lmb, base, size) >= 0)
@@ -512,19 +512,19 @@ phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr)
 	rgn = lmb_overlaps_region(&lmb->memory, addr, 1);
 	if (rgn >= 0) {
 		for (i = 0; i < lmb->reserved.cnt; i++) {
-			if (addr < lmb->reserved.region[i].base) {
+			if (addr < lmb->reserved.area[i].base) {
 				/* first reserved range > requested address */
-				return lmb->reserved.region[i].base - addr;
+				return lmb->reserved.area[i].base - addr;
 			}
-			if (lmb->reserved.region[i].base +
-			    lmb->reserved.region[i].size > addr) {
+			if (lmb->reserved.area[i].base +
+			    lmb->reserved.area[i].size > addr) {
 				/* requested addr is in this reserved range */
 				return 0;
 			}
 		}
 		/* if we come here: no reserved ranges above requested addr */
-		return lmb->memory.region[lmb->memory.cnt - 1].base +
-		       lmb->memory.region[lmb->memory.cnt - 1].size - addr;
+		return lmb->memory.area[lmb->memory.cnt - 1].base +
+		       lmb->memory.area[lmb->memory.cnt - 1].size - addr;
 	}
 
 	return 0;
@@ -535,10 +535,10 @@ int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t addr, int flags)
 	int i;
 
 	for (i = 0; i < lmb->reserved.cnt; i++) {
-		phys_addr_t upper = lmb->reserved.region[i].base +
-			lmb->reserved.region[i].size - 1;
-		if ((addr >= lmb->reserved.region[i].base) && (addr <= upper))
-			return (lmb->reserved.region[i].flags & flags) == flags;
+		phys_addr_t upper = lmb->reserved.area[i].base +
+			lmb->reserved.area[i].size - 1;
+		if (addr >= lmb->reserved.area[i].base && addr <= upper)
+			return (lmb->reserved.area[i].flags & flags) == flags;
 	}
 
 	return 0;
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 8c09281cac0d..b895a244f815 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -109,10 +109,10 @@ static int lmb_test_dump_region(struct unit_test_state *uts,
 	ut_assert_nextline(" %s.cnt = 0x%lx / max = 0x%lx", name, rgn->cnt, rgn->max);
 
 	for (i = 0; i < rgn->cnt; i++) {
-		base = rgn->region[i].base;
-		size = rgn->region[i].size;
+		base = rgn->area[i].base;
+		size = rgn->area[i].size;
 		end = base + size - 1;
-		flags = rgn->region[i].flags;
+		flags = rgn->area[i].flags;
 
 		ut_assert_nextline(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x",
 				   name, i, base, end, size, flags);
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index 59b140fde4ce..a5c96993f7f9 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -26,22 +26,22 @@ static int check_lmb(struct unit_test_state *uts, struct lmb *lmb,
 {
 	if (ram_size) {
 		ut_asserteq(lmb->memory.cnt, 1);
-		ut_asserteq(lmb->memory.region[0].base, ram_base);
-		ut_asserteq(lmb->memory.region[0].size, ram_size);
+		ut_asserteq(lmb->memory.area[0].base, ram_base);
+		ut_asserteq(lmb->memory.area[0].size, ram_size);
 	}
 
 	ut_asserteq(lmb->reserved.cnt, num_reserved);
 	if (num_reserved > 0) {
-		ut_asserteq(lmb->reserved.region[0].base, base1);
-		ut_asserteq(lmb->reserved.region[0].size, size1);
+		ut_asserteq(lmb->reserved.area[0].base, base1);
+		ut_asserteq(lmb->reserved.area[0].size, size1);
 	}
 	if (num_reserved > 1) {
-		ut_asserteq(lmb->reserved.region[1].base, base2);
-		ut_asserteq(lmb->reserved.region[1].size, size2);
+		ut_asserteq(lmb->reserved.area[1].base, base2);
+		ut_asserteq(lmb->reserved.area[1].size, size2);
 	}
 	if (num_reserved > 2) {
-		ut_asserteq(lmb->reserved.region[2].base, base3);
-		ut_asserteq(lmb->reserved.region[2].size, size3);
+		ut_asserteq(lmb->reserved.area[2].base, base3);
+		ut_asserteq(lmb->reserved.area[2].size, size3);
 	}
 	return 0;
 }
@@ -87,14 +87,14 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram,
 
 	if (ram0_size) {
 		ut_asserteq(lmb.memory.cnt, 2);
-		ut_asserteq(lmb.memory.region[0].base, ram0);
-		ut_asserteq(lmb.memory.region[0].size, ram0_size);
-		ut_asserteq(lmb.memory.region[1].base, ram);
-		ut_asserteq(lmb.memory.region[1].size, ram_size);
+		ut_asserteq(lmb.memory.area[0].base, ram0);
+		ut_asserteq(lmb.memory.area[0].size, ram0_size);
+		ut_asserteq(lmb.memory.area[1].base, ram);
+		ut_asserteq(lmb.memory.area[1].size, ram_size);
 	} else {
 		ut_asserteq(lmb.memory.cnt, 1);
-		ut_asserteq(lmb.memory.region[0].base, ram);
-		ut_asserteq(lmb.memory.region[0].size, ram_size);
+		ut_asserteq(lmb.memory.area[0].base, ram);
+		ut_asserteq(lmb.memory.area[0].size, ram_size);
 	}
 
 	/* reserve 64KiB somewhere */
@@ -165,14 +165,14 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram,
 
 	if (ram0_size) {
 		ut_asserteq(lmb.memory.cnt, 2);
-		ut_asserteq(lmb.memory.region[0].base, ram0);
-		ut_asserteq(lmb.memory.region[0].size, ram0_size);
-		ut_asserteq(lmb.memory.region[1].base, ram);
-		ut_asserteq(lmb.memory.region[1].size, ram_size);
+		ut_asserteq(lmb.memory.area[0].base, ram0);
+		ut_asserteq(lmb.memory.area[0].size, ram0_size);
+		ut_asserteq(lmb.memory.area[1].base, ram);
+		ut_asserteq(lmb.memory.area[1].size, ram_size);
 	} else {
 		ut_asserteq(lmb.memory.cnt, 1);
-		ut_asserteq(lmb.memory.region[0].base, ram);
-		ut_asserteq(lmb.memory.region[0].size, ram_size);
+		ut_asserteq(lmb.memory.area[0].base, ram);
+		ut_asserteq(lmb.memory.area[0].size, ram_size);
 	}
 
 	return 0;
@@ -725,10 +725,10 @@ static int lib_test_lmb_max_regions(struct unit_test_state *uts)
 
 	/*  check each regions */
 	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++)
-		ut_asserteq(lmb.memory.region[i].base, ram + 2 * i * ram_size);
+		ut_asserteq(lmb.memory.area[i].base, ram + 2 * i * ram_size);
 
 	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++)
-		ut_asserteq(lmb.reserved.region[i].base, ram + 2 * i * blk_size);
+		ut_asserteq(lmb.reserved.area[i].base, ram + 2 * i * blk_size);
 
 	return 0;
 }
@@ -767,7 +767,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
 	ASSERT_LMB(&lmb, ram, ram_size, 1, 0x40010000, 0x10000,
 		   0, 0, 0, 0);
 
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[0]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[0]), 1);
 
 	/* merge after */
 	ret = lmb_reserve_flags(&lmb, 0x40020000, 0x10000, LMB_NOMAP);
@@ -781,15 +781,15 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
 	ASSERT_LMB(&lmb, ram, ram_size, 1, 0x40000000, 0x30000,
 		   0, 0, 0, 0);
 
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[0]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[0]), 1);
 
 	ret = lmb_reserve_flags(&lmb, 0x40030000, 0x10000, LMB_NONE);
 	ut_asserteq(ret, 0);
 	ASSERT_LMB(&lmb, ram, ram_size, 2, 0x40000000, 0x30000,
 		   0x40030000, 0x10000, 0, 0);
 
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[0]), 1);
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[1]), 0);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[0]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[1]), 0);
 
 	/* test that old API use LMB_NONE */
 	ret = lmb_reserve(&lmb, 0x40040000, 0x10000);
@@ -797,8 +797,8 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
 	ASSERT_LMB(&lmb, ram, ram_size, 2, 0x40000000, 0x30000,
 		   0x40030000, 0x20000, 0, 0);
 
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[0]), 1);
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[1]), 0);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[0]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[1]), 0);
 
 	ret = lmb_reserve_flags(&lmb, 0x40070000, 0x10000, LMB_NOMAP);
 	ut_asserteq(ret, 0);
@@ -816,9 +816,9 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
 	ASSERT_LMB(&lmb, ram, ram_size, 3, 0x40000000, 0x30000,
 		   0x40030000, 0x20000, 0x40050000, 0x30000);
 
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[0]), 1);
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[1]), 0);
-	ut_asserteq(lmb_is_nomap(&lmb.reserved.region[2]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[0]), 1);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[1]), 0);
+	ut_asserteq(lmb_is_nomap(&lmb.reserved.area[2]), 1);
 
 	return 0;
 }
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 04/14] lmb: Rename memory/reserved_regions to area
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (2 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 03/14] lmb: Rename region array " Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 05/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS Simon Glass
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Rename these two fields since they are areas, not regions.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to rename memory/reserved_regions to area

 include/lmb.h | 8 ++++----
 lib/lmb.c     | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 9d4a05670c23..92332eb63069 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -80,15 +80,15 @@ struct lmb_region {
  *
  * @memory: Description of memory regions.
  * @reserved: Description of reserved regions.
- * @memory_regions: Array of the memory regions (statically allocated)
- * @reserved_regions: Array of the reserved regions (statically allocated)
+ * @memory_areas: Array of the memory areas (statically allocated)
+ * @reserved_areas: Array of the reserved areas (statically allocated)
  */
 struct lmb {
 	struct lmb_region memory;
 	struct lmb_region reserved;
 #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
-	struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
+	struct lmb_area memory_areas[CONFIG_LMB_MEMORY_REGIONS];
+	struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_REGIONS];
 #endif
 };
 
diff --git a/lib/lmb.c b/lib/lmb.c
index 9336792df8ba..2669868f0dda 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -113,8 +113,8 @@ void lmb_init(struct lmb *lmb)
 #else
 	lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
 	lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
-	lmb->memory.region = lmb->memory_regions;
-	lmb->reserved.region = lmb->reserved_regions;
+	lmb->memory.area = lmb->memory_areas;
+	lmb->reserved.area = lmb->reserved_areas;
 #endif
 	lmb->memory.cnt = 0;
 	lmb->reserved.cnt = 0;
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 05/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (3 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 04/14] lmb: Rename memory/reserved_regions " Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 06/14] " Simon Glass
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Brandon Maier, Heinrich Schuchardt,
	Kautuk Consul, Leo Yu-Chi Liang, Michal Simek, Oleksandr Suvorov,
	Patrice Chotard, Patrick Delaunay, Sjoerd Simons, uboot-stm32

These are the number of areas within a region, so rename them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS

 configs/stm32mp13_defconfig         |  4 ++--
 configs/stm32mp15_basic_defconfig   |  4 ++--
 configs/stm32mp15_defconfig         |  4 ++--
 configs/stm32mp15_trusted_defconfig |  4 ++--
 include/lmb.h                       | 10 +++++-----
 lib/Kconfig                         |  6 +++---
 lib/lmb.c                           |  4 ++--
 7 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig
index 82b62744f6db..98fcfeb5fa61 100644
--- a/configs/stm32mp13_defconfig
+++ b/configs/stm32mp13_defconfig
@@ -74,5 +74,5 @@ CONFIG_OPTEE=y
 # CONFIG_OPTEE_TA_AVB is not set
 CONFIG_ERRNO_STR=y
 # CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 9ea5aaa7145a..7b5655d957d9 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -189,5 +189,5 @@ CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
 # CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig
index 4d0a81f8a871..643ec201c644 100644
--- a/configs/stm32mp15_defconfig
+++ b/configs/stm32mp15_defconfig
@@ -165,5 +165,5 @@ CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
 # CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index 0a7d8624858d..2a8162a870c5 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -165,5 +165,5 @@ CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
 # CONFIG_LMB_USE_MAX_REGIONS is not set
-CONFIG_LMB_MEMORY_REGIONS=2
-CONFIG_LMB_RESERVED_REGIONS=16
+CONFIG_LMB_MEMORY_AREAS=2
+CONFIG_LMB_RESERVED_AREAS=16
diff --git a/include/lmb.h b/include/lmb.h
index 92332eb63069..d963ac28d9ac 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -46,11 +46,11 @@ struct lmb_area {
  *
  * case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each
  *         region is configurated *independently* with
- *         => CONFIG_LMB_MEMORY_REGIONS: struct lmb.memory_regions
- *         => CONFIG_LMB_RESERVED_REGIONS: struct lmb.reserved_regions
+ *         => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_regions
+ *         => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_regions
  *         lmb_region.region is only a pointer to the correct buffer,
  *         initialized in lmb_init(). This configuration is useful to manage
- *         more reserved memory regions with CONFIG_LMB_RESERVED_REGIONS.
+ *         more reserved memory regions with CONFIG_LMB_RESERVED_AREAS.
  */
 
 /**
@@ -87,8 +87,8 @@ struct lmb {
 	struct lmb_region memory;
 	struct lmb_region reserved;
 #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_area memory_areas[CONFIG_LMB_MEMORY_REGIONS];
-	struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_REGIONS];
+	struct lmb_area memory_areas[CONFIG_LMB_MEMORY_AREAS];
+	struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_AREAS];
 #endif
 };
 
diff --git a/lib/Kconfig b/lib/Kconfig
index 42e559ad0b51..53f1332a8f83 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1082,7 +1082,7 @@ config LMB_USE_MAX_REGIONS
 	  Define the number of supported memory regions in the library logical
 	  memory blocks.
 	  This feature allow to reduce the lmb library size by using compiler
-	  optimization when LMB_MEMORY_REGIONS == LMB_RESERVED_REGIONS.
+	  optimization when LMB_MEMORY_AREAS == LMB_RESERVED_AREAS.
 
 config LMB_MAX_REGIONS
 	int "Number of memory and reserved regions in lmb lib"
@@ -1092,7 +1092,7 @@ config LMB_MAX_REGIONS
 	  Define the number of supported regions, memory and reserved, in the
 	  library logical memory blocks.
 
-config LMB_MEMORY_REGIONS
+config LMB_MEMORY_AREAS
 	int "Number of memory regions in lmb lib"
 	depends on !LMB_USE_MAX_REGIONS
 	default 8
@@ -1101,7 +1101,7 @@ config LMB_MEMORY_REGIONS
 	  memory blocks.
 	  The minimal value is CONFIG_NR_DRAM_BANKS.
 
-config LMB_RESERVED_REGIONS
+config LMB_RESERVED_AREAS
 	int "Number of reserved regions in lmb lib"
 	depends on !LMB_USE_MAX_REGIONS
 	default 8
diff --git a/lib/lmb.c b/lib/lmb.c
index 2669868f0dda..f4321d10118b 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -111,8 +111,8 @@ void lmb_init(struct lmb *lmb)
 	lmb->memory.max = CONFIG_LMB_MAX_REGIONS;
 	lmb->reserved.max = CONFIG_LMB_MAX_REGIONS;
 #else
-	lmb->memory.max = CONFIG_LMB_MEMORY_REGIONS;
-	lmb->reserved.max = CONFIG_LMB_RESERVED_REGIONS;
+	lmb->memory.max = CONFIG_LMB_MEMORY_AREAS;
+	lmb->reserved.max = CONFIG_LMB_RESERVED_AREAS;
 	lmb->memory.area = lmb->memory_areas;
 	lmb->reserved.area = lmb->reserved_areas;
 #endif
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 06/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (4 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 05/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 07/14] lmb: Update use of region in the header file Simon Glass
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Brandon Maier, Dzmitry Sankouski,
	Heinrich Schuchardt, Kautuk Consul, Leo Yu-Chi Liang,
	Mark Kettenis, Michal Simek, Oleksandr Suvorov, Patrice Chotard,
	Patrick Delaunay, Sam Shih, Sjoerd Simons, Sumit Garg, Wei Fu,
	Yixun Lan, uboot-stm32

These refer to the maximum number of areas, so rename them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS

 configs/a3y17lte_defconfig           |  2 +-
 configs/a5y17lte_defconfig           |  2 +-
 configs/a7y17lte_defconfig           |  2 +-
 configs/apple_m1_defconfig           |  2 +-
 configs/dragonboard845c_defconfig    |  2 +-
 configs/mt7981_emmc_rfb_defconfig    |  2 +-
 configs/mt7981_rfb_defconfig         |  2 +-
 configs/mt7981_sd_rfb_defconfig      |  2 +-
 configs/mt7986_rfb_defconfig         |  2 +-
 configs/mt7986a_bpir3_emmc_defconfig |  2 +-
 configs/mt7986a_bpir3_sd_defconfig   |  2 +-
 configs/qcs404evb_defconfig          |  2 +-
 configs/starqltechn_defconfig        |  2 +-
 configs/stm32mp13_defconfig          |  2 +-
 configs/stm32mp15_basic_defconfig    |  2 +-
 configs/stm32mp15_defconfig          |  2 +-
 configs/stm32mp15_trusted_defconfig  |  2 +-
 configs/th1520_lpi4a_defconfig       |  2 +-
 include/lmb.h                        | 14 ++++-----
 lib/Kconfig                          | 24 +++++++--------
 lib/lmb.c                            |  6 ++--
 test/lib/lmb.c                       | 44 ++++++++++++++--------------
 22 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/configs/a3y17lte_defconfig b/configs/a3y17lte_defconfig
index 42fcd2a3d317..4db2274731ad 100644
--- a/configs/a3y17lte_defconfig
+++ b/configs/a3y17lte_defconfig
@@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x2000000
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_DM_I2C_GPIO=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/a5y17lte_defconfig b/configs/a5y17lte_defconfig
index 3b80536c12c8..4bcd8313630c 100644
--- a/configs/a5y17lte_defconfig
+++ b/configs/a5y17lte_defconfig
@@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x2000000
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_DM_I2C_GPIO=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/a7y17lte_defconfig b/configs/a7y17lte_defconfig
index 9390e35057eb..7236b70e4f88 100644
--- a/configs/a7y17lte_defconfig
+++ b/configs/a7y17lte_defconfig
@@ -24,4 +24,4 @@ CONFIG_SYS_BOOTM_LEN=0x2000000
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
 CONFIG_DM_I2C_GPIO=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index d58a9030dbd0..618baee132fa 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -22,4 +22,4 @@ CONFIG_SYS_WHITE_ON_BLACK=y
 CONFIG_NO_FB_CLEAR=y
 CONFIG_VIDEO_SIMPLE=y
 # CONFIG_GENERATE_SMBIOS_TABLE is not set
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/dragonboard845c_defconfig b/configs/dragonboard845c_defconfig
index a69d82761a8d..17b1e4ffd7df 100644
--- a/configs/dragonboard845c_defconfig
+++ b/configs/dragonboard845c_defconfig
@@ -26,4 +26,4 @@ CONFIG_DM_PMIC=y
 CONFIG_PMIC_QCOM=y
 CONFIG_MSM_GENI_SERIAL=y
 CONFIG_SPMI_MSM=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7981_emmc_rfb_defconfig b/configs/mt7981_emmc_rfb_defconfig
index b3b37b6e5ed4..44dbfa86e9b1 100644
--- a/configs/mt7981_emmc_rfb_defconfig
+++ b/configs/mt7981_emmc_rfb_defconfig
@@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y
 CONFIG_FAT_WRITE=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7981_rfb_defconfig b/configs/mt7981_rfb_defconfig
index b7ffb4dfa74d..4fc9dd92fe43 100644
--- a/configs/mt7981_rfb_defconfig
+++ b/configs/mt7981_rfb_defconfig
@@ -64,4 +64,4 @@ CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_MTK_SPIM=y
 CONFIG_HEXDUMP=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7981_sd_rfb_defconfig b/configs/mt7981_sd_rfb_defconfig
index 85be9bbc5030..2cbac6adde83 100644
--- a/configs/mt7981_sd_rfb_defconfig
+++ b/configs/mt7981_sd_rfb_defconfig
@@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y
 CONFIG_FAT_WRITE=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7986_rfb_defconfig b/configs/mt7986_rfb_defconfig
index ac91c93efb42..73b2cad4275e 100644
--- a/configs/mt7986_rfb_defconfig
+++ b/configs/mt7986_rfb_defconfig
@@ -64,4 +64,4 @@ CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_MTK_SPIM=y
 CONFIG_HEXDUMP=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7986a_bpir3_emmc_defconfig b/configs/mt7986a_bpir3_emmc_defconfig
index 5b76512a46f6..ee8f58bdbe09 100644
--- a/configs/mt7986a_bpir3_emmc_defconfig
+++ b/configs/mt7986a_bpir3_emmc_defconfig
@@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y
 CONFIG_FAT_WRITE=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/mt7986a_bpir3_sd_defconfig b/configs/mt7986a_bpir3_sd_defconfig
index 36547db91423..b301a626a946 100644
--- a/configs/mt7986a_bpir3_sd_defconfig
+++ b/configs/mt7986a_bpir3_sd_defconfig
@@ -62,4 +62,4 @@ CONFIG_MTK_SERIAL=y
 CONFIG_FAT_WRITE=y
 CONFIG_HEXDUMP=y
 # CONFIG_EFI_LOADER is not set
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/qcs404evb_defconfig b/configs/qcs404evb_defconfig
index 9e72f64f7849..811d8b2bd4c8 100644
--- a/configs/qcs404evb_defconfig
+++ b/configs/qcs404evb_defconfig
@@ -52,4 +52,4 @@ CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_DWC3_GENERIC=y
 CONFIG_USB_STORAGE=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/starqltechn_defconfig b/configs/starqltechn_defconfig
index 5b85ce5fe96f..4522928125f7 100644
--- a/configs/starqltechn_defconfig
+++ b/configs/starqltechn_defconfig
@@ -38,4 +38,4 @@ CONFIG_VIDEO_FONT_16X32=y
 CONFIG_SYS_WHITE_ON_BLACK=y
 CONFIG_VIDEO_SIMPLE=y
 CONFIG_VIDEO_DT_SIMPLEFB=y
-CONFIG_LMB_MAX_REGIONS=64
+CONFIG_LMB_MAX_AREAS=64
diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig
index 98fcfeb5fa61..0f6fa08c0002 100644
--- a/configs/stm32mp13_defconfig
+++ b/configs/stm32mp13_defconfig
@@ -73,6 +73,6 @@ CONFIG_TEE=y
 CONFIG_OPTEE=y
 # CONFIG_OPTEE_TA_AVB is not set
 CONFIG_ERRNO_STR=y
-# CONFIG_LMB_USE_MAX_REGIONS is not set
+# CONFIG_LMB_USE_MAX_AREAS is not set
 CONFIG_LMB_MEMORY_AREAS=2
 CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 7b5655d957d9..1aeaabb22a4e 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -188,6 +188,6 @@ CONFIG_WDT=y
 CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
-# CONFIG_LMB_USE_MAX_REGIONS is not set
+# CONFIG_LMB_USE_MAX_AREAS is not set
 CONFIG_LMB_MEMORY_AREAS=2
 CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig
index 643ec201c644..a17749411c62 100644
--- a/configs/stm32mp15_defconfig
+++ b/configs/stm32mp15_defconfig
@@ -164,6 +164,6 @@ CONFIG_WDT=y
 CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
-# CONFIG_LMB_USE_MAX_REGIONS is not set
+# CONFIG_LMB_USE_MAX_AREAS is not set
 CONFIG_LMB_MEMORY_AREAS=2
 CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index 2a8162a870c5..45ef9fe9aa12 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -164,6 +164,6 @@ CONFIG_WDT=y
 CONFIG_WDT_STM32MP=y
 # CONFIG_BINMAN_FDT is not set
 CONFIG_ERRNO_STR=y
-# CONFIG_LMB_USE_MAX_REGIONS is not set
+# CONFIG_LMB_USE_MAX_AREAS is not set
 CONFIG_LMB_MEMORY_AREAS=2
 CONFIG_LMB_RESERVED_AREAS=16
diff --git a/configs/th1520_lpi4a_defconfig b/configs/th1520_lpi4a_defconfig
index 710ec6abf520..ff88729541ff 100644
--- a/configs/th1520_lpi4a_defconfig
+++ b/configs/th1520_lpi4a_defconfig
@@ -79,4 +79,4 @@ CONFIG_BZIP2=y
 CONFIG_ZSTD=y
 CONFIG_LIB_RATIONAL=y
 # CONFIG_EFI_LOADER is not set
-# CONFIG_LMB_USE_MAX_REGIONS is not set
+# CONFIG_LMB_USE_MAX_AREAS is not set
diff --git a/include/lmb.h b/include/lmb.h
index d963ac28d9ac..3cf23d2f2346 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -37,14 +37,14 @@ struct lmb_area {
 
 /*
  * For regions size management, see LMB configuration in KConfig
- * all the #if test are done with CONFIG_LMB_USE_MAX_REGIONS (boolean)
+ * all the #if test are done with CONFIG_LMB_USE_MAX_AREAS (boolean)
  *
- * case 1. CONFIG_LMB_USE_MAX_REGIONS is defined (legacy mode)
- *         => CONFIG_LMB_MAX_REGIONS is used to configure the region size,
+ * case 1. CONFIG_LMB_USE_MAX_AREAS is defined (legacy mode)
+ *         => CONFIG_LMB_MAX_AREAS is used to configure the region size,
  *         directly in the array lmb_region.region[], with the same
  *         configuration for memory and reserved regions.
  *
- * case 2. CONFIG_LMB_USE_MAX_REGIONS is not defined, the size of each
+ * case 2. CONFIG_LMB_USE_MAX_AREAS is not defined, the size of each
  *         region is configurated *independently* with
  *         => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_regions
  *         => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_regions
@@ -63,8 +63,8 @@ struct lmb_area {
 struct lmb_region {
 	unsigned long cnt;
 	unsigned long max;
-#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	struct lmb_area area[CONFIG_LMB_MAX_REGIONS];
+#if IS_ENABLED(CONFIG_LMB_USE_MAX_AREAS)
+	struct lmb_area area[CONFIG_LMB_MAX_AREAS];
 #else
 	struct lmb_area *area;
 #endif
@@ -86,7 +86,7 @@ struct lmb_region {
 struct lmb {
 	struct lmb_region memory;
 	struct lmb_region reserved;
-#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
+#if !IS_ENABLED(CONFIG_LMB_USE_MAX_AREAS)
 	struct lmb_area memory_areas[CONFIG_LMB_MEMORY_AREAS];
 	struct lmb_area reserved_areas[CONFIG_LMB_RESERVED_AREAS];
 #endif
diff --git a/lib/Kconfig b/lib/Kconfig
index 53f1332a8f83..e8c88591ffa5 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -1075,38 +1075,38 @@ config LMB
 	help
 	  Support the library logical memory blocks.
 
-config LMB_USE_MAX_REGIONS
+config LMB_USE_MAX_AREAS
 	bool "Use a common number of memory and reserved regions in lmb lib"
 	default y
 	help
-	  Define the number of supported memory regions in the library logical
+	  Define the number of supported memory areas in the library logical
 	  memory blocks.
 	  This feature allow to reduce the lmb library size by using compiler
 	  optimization when LMB_MEMORY_AREAS == LMB_RESERVED_AREAS.
 
-config LMB_MAX_REGIONS
-	int "Number of memory and reserved regions in lmb lib"
-	depends on LMB_USE_MAX_REGIONS
+config LMB_MAX_AREAS
+	int "Number of memory and reserved areas in lmb lib"
+	depends on LMB_USE_MAX_AREAS
 	default 16
 	help
-	  Define the number of supported regions, memory and reserved, in the
+	  Define the number of supported areas, memory and reserved, in the
 	  library logical memory blocks.
 
 config LMB_MEMORY_AREAS
-	int "Number of memory regions in lmb lib"
-	depends on !LMB_USE_MAX_REGIONS
+	int "Number of memory areas in lmb lib"
+	depends on !LMB_USE_MAX_AREAS
 	default 8
 	help
-	  Define the number of supported memory regions in the library logical
+	  Define the number of supported memory areas in the library logical
 	  memory blocks.
 	  The minimal value is CONFIG_NR_DRAM_BANKS.
 
 config LMB_RESERVED_AREAS
-	int "Number of reserved regions in lmb lib"
-	depends on !LMB_USE_MAX_REGIONS
+	int "Number of reserved areas in lmb lib"
+	depends on !LMB_USE_MAX_AREAS
 	default 8
 	help
-	  Define the number of supported reserved regions in the library logical
+	  Define the number of supported reserved areas in the library logical
 	  memory blocks.
 
 config PHANDLE_CHECK_SEQ
diff --git a/lib/lmb.c b/lib/lmb.c
index f4321d10118b..6061c6f361c6 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -107,9 +107,9 @@ static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
 
 void lmb_init(struct lmb *lmb)
 {
-#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
-	lmb->memory.max = CONFIG_LMB_MAX_REGIONS;
-	lmb->reserved.max = CONFIG_LMB_MAX_REGIONS;
+#if IS_ENABLED(CONFIG_LMB_USE_MAX_AREAS)
+	lmb->memory.max = CONFIG_LMB_MAX_AREAS;
+	lmb->reserved.max = CONFIG_LMB_MAX_AREAS;
 #else
 	lmb->memory.max = CONFIG_LMB_MEMORY_AREAS;
 	lmb->reserved.max = CONFIG_LMB_RESERVED_AREAS;
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index a5c96993f7f9..b665d8dcdeb4 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -665,8 +665,8 @@ static int lib_test_lmb_get_free_size(struct unit_test_state *uts)
 DM_TEST(lib_test_lmb_get_free_size,
 	UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 
-#ifdef CONFIG_LMB_USE_MAX_REGIONS
-static int lib_test_lmb_max_regions(struct unit_test_state *uts)
+#ifdef CONFIG_LMB_USE_MAX_AREAS
+static int lib_test_LMB_MAX_AREAS(struct unit_test_state *uts)
 {
 	const phys_addr_t ram = 0x00000000;
 	/*
@@ -674,8 +674,8 @@ static int lib_test_lmb_max_regions(struct unit_test_state *uts)
 	 * we need to scale ram_size (which in this case is the size of the lmb
 	 * region) to match.
 	 */
-	const phys_size_t ram_size = ((0xFFFFFFFF >> CONFIG_LMB_MAX_REGIONS)
-			+ 1) * CONFIG_LMB_MAX_REGIONS;
+	const phys_size_t ram_size = ((0xFFFFFFFF >> CONFIG_LMB_MAX_AREAS)
+			+ 1) * CONFIG_LMB_MAX_AREAS;
 	const phys_size_t blk_size = 0x10000;
 	phys_addr_t offset;
 	struct lmb lmb;
@@ -684,57 +684,57 @@ static int lib_test_lmb_max_regions(struct unit_test_state *uts)
 	lmb_init(&lmb);
 
 	ut_asserteq(lmb.memory.cnt, 0);
-	ut_asserteq(lmb.memory.max, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.memory.max, CONFIG_LMB_MAX_AREAS);
 	ut_asserteq(lmb.reserved.cnt, 0);
-	ut_asserteq(lmb.reserved.max, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.reserved.max, CONFIG_LMB_MAX_AREAS);
 
-	/*  Add CONFIG_LMB_MAX_REGIONS memory regions */
-	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++) {
+	/*  Add CONFIG_LMB_MAX_AREAS memory regions */
+	for (i = 0; i < CONFIG_LMB_MAX_AREAS; i++) {
 		offset = ram + 2 * i * ram_size;
 		ret = lmb_add(&lmb, offset, ram_size);
 		ut_asserteq(ret, 0);
 	}
-	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
 	ut_asserteq(lmb.reserved.cnt, 0);
 
-	/*  error for the (CONFIG_LMB_MAX_REGIONS + 1) memory regions */
-	offset = ram + 2 * (CONFIG_LMB_MAX_REGIONS + 1) * ram_size;
+	/*  error for the (CONFIG_LMB_MAX_AREAS + 1) memory regions */
+	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * ram_size;
 	ret = lmb_add(&lmb, offset, ram_size);
 	ut_asserteq(ret, -1);
 
-	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
 	ut_asserteq(lmb.reserved.cnt, 0);
 
-	/*  reserve CONFIG_LMB_MAX_REGIONS regions */
-	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++) {
+	/*  reserve CONFIG_LMB_MAX_AREAS regions */
+	for (i = 0; i < CONFIG_LMB_MAX_AREAS; i++) {
 		offset = ram + 2 * i * blk_size;
 		ret = lmb_reserve(&lmb, offset, blk_size);
 		ut_asserteq(ret, 0);
 	}
 
-	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_REGIONS);
-	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
+	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_AREAS);
 
 	/*  error for the 9th reserved blocks */
-	offset = ram + 2 * (CONFIG_LMB_MAX_REGIONS + 1) * blk_size;
+	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * blk_size;
 	ret = lmb_reserve(&lmb, offset, blk_size);
 	ut_asserteq(ret, -1);
 
-	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_REGIONS);
-	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_REGIONS);
+	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
+	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_AREAS);
 
 	/*  check each regions */
-	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++)
+	for (i = 0; i < CONFIG_LMB_MAX_AREAS; i++)
 		ut_asserteq(lmb.memory.area[i].base, ram + 2 * i * ram_size);
 
-	for (i = 0; i < CONFIG_LMB_MAX_REGIONS; i++)
+	for (i = 0; i < CONFIG_LMB_MAX_AREAS; i++)
 		ut_asserteq(lmb.reserved.area[i].base, ram + 2 * i * blk_size);
 
 	return 0;
 }
 #endif
 
-DM_TEST(lib_test_lmb_max_regions,
+DM_TEST(lib_test_LMB_MAX_AREAS,
 	UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 
 static int lib_test_lmb_flags(struct unit_test_state *uts)
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 07/14] lmb: Update use of region in the header file
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (5 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 06/14] " Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 08/14] lmb: Update use of region in the C file Simon Glass
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay

Tidy up places in the header file where it says region but now means area.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to update use of region in the header file

 include/lmb.h | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 3cf23d2f2346..6f91df43d5d5 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -13,7 +13,7 @@
  */
 
 /**
- * enum lmb_flags - definition of memory region attributes
+ * enum lmb_flags - definition of memory area attributes
  * @LMB_NONE: no special request
  * @LMB_NOMAP: don't add to mmu configuration
  */
@@ -40,24 +40,24 @@ struct lmb_area {
  * all the #if test are done with CONFIG_LMB_USE_MAX_AREAS (boolean)
  *
  * case 1. CONFIG_LMB_USE_MAX_AREAS is defined (legacy mode)
- *         => CONFIG_LMB_MAX_AREAS is used to configure the region size,
- *         directly in the array lmb_region.region[], with the same
- *         configuration for memory and reserved regions.
+ *         => CONFIG_LMB_MAX_AREAS is used to configure the maximum number of
+ *	   areas, directly in the array lmb_region.area[], with the same
+ *         configuration for memory and reserved areas.
  *
- * case 2. CONFIG_LMB_USE_MAX_AREAS is not defined, the size of each
- *         region is configurated *independently* with
- *         => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_regions
- *         => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_regions
- *         lmb_region.region is only a pointer to the correct buffer,
+ * case 2. CONFIG_LMB_USE_MAX_AREAS is not defined, the maximum number of
+ *         areas is configured *independently* with
+ *         => CONFIG_LMB_MEMORY_AREAS: struct lmb.memory_areas
+ *         => CONFIG_LMB_RESERVED_AREAS: struct lmb.reserved_areas
+ *         lmb_region.area is only a pointer to the correct buffer,
  *         initialized in lmb_init(). This configuration is useful to manage
- *         more reserved memory regions with CONFIG_LMB_RESERVED_AREAS.
+ *         more reserved memory areas with CONFIG_LMB_RESERVED_AREAS.
  */
 
 /**
- * struct lmb_region - Description of a set of region.
+ * struct lmb_region - Description of a set of areas.
  *
- * @cnt: Number of regions.
- * @max: Size of the region array, max value of cnt.
+ * @cnt: Number of areas
+ * @max: Size of the area array, max value of cnt
  * @area: Array of the areas within the region
  */
 struct lmb_region {
@@ -99,13 +99,13 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
 long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 /**
- * lmb_reserve_flags - Reserve one region with a specific flags bitfield.
+ * lmb_reserve_flags - Reserve one area with a specific flags bitfield.
  *
  * @lmb:	the logical memory block struct
- * @base:	base address of the memory region
- * @size:	size of the memory region
- * @flags:	flags for the memory region
- * Return:	0 if OK, > 0 for coalesced region or a negative error code.
+ * @base:	base address of the memory area
+ * @size:	size of the memory area
+ * @flags:	flags for the memory area
+ * Return:	0 if OK, > 0 for coalesced area or a negative error code.
  */
 long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base,
 		       phys_size_t size, enum lmb_flags flags);
@@ -118,9 +118,9 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
 
 /**
- * lmb_is_reserved() - test if address is in reserved region
+ * lmb_is_reserved() - test if address is in reserved area
  *
- * The function checks if a reserved region comprising @addr exists.
+ * The function checks if a reserved area comprising @addr exists.
  *
  * @lmb:	the logical memory block struct
  * @addr:	address to be tested
@@ -129,9 +129,9 @@ phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
 int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr);
 
 /**
- * lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
+ * lmb_is_reserved_flags() - test if address is in reserved area with flag bits set
  *
- * The function checks if a reserved region comprising @addr exists which has
+ * The function checks if a reserved area comprising @addr exists which has
  * all flag bits set which are set in @flags.
  *
  * @lmb:	the logical memory block struct
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 08/14] lmb: Update use of region in the C file
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (6 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 07/14] lmb: Update use of region in the header file Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 09/14] lmb: Tidy up structure access Simon Glass
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Change places that should say area to do so. Change the name of the 'rgn'
variable to 'area' in places where it refers to an area number.

Fix the 'beginging' typo while here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to update use of region in the C file

 lib/lmb.c | 126 +++++++++++++++++++++++++++---------------------------
 1 file changed, 62 insertions(+), 64 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index 6061c6f361c6..0b03cc4d1c23 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -74,22 +74,21 @@ static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
 	return 0;
 }
 
-static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
-				 unsigned long r2)
+static long lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
 {
-	phys_addr_t base1 = rgn->area[r1].base;
-	phys_size_t size1 = rgn->area[r1].size;
-	phys_addr_t base2 = rgn->area[r2].base;
-	phys_size_t size2 = rgn->area[r2].size;
+	phys_addr_t base1 = rgn->area[a1].base;
+	phys_size_t size1 = rgn->area[a1].size;
+	phys_addr_t base2 = rgn->area[a2].base;
+	phys_size_t size2 = rgn->area[a2].size;
 
 	return lmb_addrs_adjacent(base1, size1, base2, size2);
 }
 
-static void lmb_remove_region(struct lmb_region *rgn, unsigned long r)
+static void lmb_remove_area(struct lmb_region *rgn, unsigned long area)
 {
 	unsigned long i;
 
-	for (i = r; i < rgn->cnt - 1; i++) {
+	for (i = area; i < rgn->cnt - 1; i++) {
 		rgn->area[i].base = rgn->area[i + 1].base;
 		rgn->area[i].size = rgn->area[i + 1].size;
 		rgn->area[i].flags = rgn->area[i + 1].flags;
@@ -97,12 +96,11 @@ static void lmb_remove_region(struct lmb_region *rgn, unsigned long r)
 	rgn->cnt--;
 }
 
-/* Assumption: base addr of region 1 < base addr of region 2 */
-static void lmb_coalesce_regions(struct lmb_region *rgn, unsigned long r1,
-				 unsigned long r2)
+/* Assumption: base addr of area 1 < base addr of area 2 */
+static void lmb_coalesce_areas(struct lmb_region *rgn, uint a1, uint a2)
 {
-	rgn->area[r1].size += rgn->area[r2].size;
-	lmb_remove_region(rgn, r2);
+	rgn->area[a1].size += rgn->area[a2].size;
+	lmb_remove_area(rgn, a2);
 }
 
 void lmb_init(struct lmb *lmb)
@@ -228,8 +226,8 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
 }
 
 /* This routine called with relocation disabled. */
-static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
-				 phys_size_t size, enum lmb_flags flags)
+static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
+			       phys_size_t size, enum lmb_flags flags)
 {
 	unsigned long coalesced = 0;
 	long adjacent, i;
@@ -244,43 +242,43 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 
 	/* First try and coalesce this LMB with another. */
 	for (i = 0; i < rgn->cnt; i++) {
-		phys_addr_t rgnbase = rgn->area[i].base;
-		phys_size_t rgnsize = rgn->area[i].size;
-		phys_size_t rgnflags = rgn->area[i].flags;
+		phys_addr_t abase = rgn->area[i].base;
+		phys_size_t asize = rgn->area[i].size;
+		phys_size_t aflags = rgn->area[i].flags;
 		phys_addr_t end = base + size - 1;
-		phys_addr_t rgnend = rgnbase + rgnsize - 1;
+		phys_addr_t rgnend = abase + asize - 1;
 
-		if (rgnbase <= base && end <= rgnend) {
-			if (flags == rgnflags)
-				/* Already have this region, so we're done */
+		if (abase <= base && end <= rgnend) {
+			if (flags == aflags)
+				/* Already have this area, so we're done */
 				return 0;
 			else
-				return -1; /* regions with new flags */
+				return -1; /* areas with new flags */
 		}
 
-		adjacent = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
+		adjacent = lmb_addrs_adjacent(base, size, abase, asize);
 		if (adjacent > 0) {
-			if (flags != rgnflags)
+			if (flags != aflags)
 				break;
 			rgn->area[i].base -= size;
 			rgn->area[i].size += size;
 			coalesced++;
 			break;
 		} else if (adjacent < 0) {
-			if (flags != rgnflags)
+			if (flags != aflags)
 				break;
 			rgn->area[i].size += size;
 			coalesced++;
 			break;
-		} else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) {
-			/* regions overlap */
+		} else if (lmb_addrs_overlap(base, size, abase, asize)) {
+			/* areas overlap */
 			return -1;
 		}
 	}
 
-	if (i < rgn->cnt - 1 && lmb_regions_adjacent(rgn, i, i + 1)) {
+	if (i < rgn->cnt - 1 && lmb_areas_adjacent(rgn, i, i + 1)) {
 		if (rgn->area[i].flags == rgn->area[i + 1].flags) {
-			lmb_coalesce_regions(rgn, i, i + 1);
+			lmb_coalesce_areas(rgn, i, i + 1);
 			coalesced++;
 		}
 	}
@@ -315,10 +313,10 @@ static long lmb_add_region_flags(struct lmb_region *rgn, phys_addr_t base,
 	return 0;
 }
 
-static long lmb_add_region(struct lmb_region *rgn, phys_addr_t base,
-			   phys_size_t size)
+static long lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
+			 phys_size_t size)
 {
-	return lmb_add_region_flags(rgn, base, size, LMB_NONE);
+	return lmb_add_area_flags(rgn, base, size, LMB_NONE);
 }
 
 /* This routine may be called with relocation disabled. */
@@ -326,7 +324,7 @@ long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 {
 	struct lmb_region *_rgn = &(lmb->memory);
 
-	return lmb_add_region(_rgn, base, size);
+	return lmb_add_area(_rgn, base, size);
 }
 
 long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
@@ -338,7 +336,7 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 
 	rgnbegin = rgnend = 0; /* supress gcc warnings */
 
-	/* Find the region where (base, size) belongs to */
+	/* Find the area where (base, size) belongs to */
 	for (i = 0; i < rgn->cnt; i++) {
 		rgnbegin = rgn->area[i].base;
 		rgnend = rgnbegin + rgn->area[i].size - 1;
@@ -347,24 +345,24 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 			break;
 	}
 
-	/* Didn't find the region */
+	/* Didn't find the area */
 	if (i == rgn->cnt)
 		return -1;
 
-	/* Check to see if we are removing entire region */
+	/* Check to see if we are removing entire area */
 	if ((rgnbegin == base) && (rgnend == end)) {
-		lmb_remove_region(rgn, i);
+		lmb_remove_area(rgn, i);
 		return 0;
 	}
 
-	/* Check to see if region is matching at the front */
+	/* Check to see if areaz is matching at the front */
 	if (rgnbegin == base) {
 		rgn->area[i].base = end + 1;
 		rgn->area[i].size -= size;
 		return 0;
 	}
 
-	/* Check to see if the region is matching at the end */
+	/* Check to see if the area is matching at the end */
 	if (rgnend == end) {
 		rgn->area[i].size -= size;
 		return 0;
@@ -372,11 +370,11 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 
 	/*
 	 * We need to split the entry -  adjust the current one to the
-	 * beginging of the hole and add the region after hole.
+	 * beginning of the hole and add the area after hole.
 	 */
 	rgn->area[i].size = base - rgn->area[i].base;
 
-	return lmb_add_region_flags(rgn, end + 1, rgnend - end,
+	return lmb_add_area_flags(rgn, end + 1, rgnend - end,
 				    rgn->area[i].flags);
 }
 
@@ -385,7 +383,7 @@ long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
 {
 	struct lmb_region *_rgn = &(lmb->reserved);
 
-	return lmb_add_region_flags(_rgn, base, size, flags);
+	return lmb_add_area_flags(_rgn, base, size, flags);
 }
 
 long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
@@ -399,9 +397,10 @@ static long lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
 	unsigned long i;
 
 	for (i = 0; i < rgn->cnt; i++) {
-		phys_addr_t rgnbase = rgn->area[i].base;
-		phys_size_t rgnsize = rgn->area[i].size;
-		if (lmb_addrs_overlap(base, size, rgnbase, rgnsize))
+		phys_addr_t abase = rgn->area[i].base;
+		phys_size_t asize = rgn->area[i].size;
+
+		if (lmb_addrs_overlap(base, size, abase, asize))
 			break;
 	}
 
@@ -435,7 +434,7 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size)
 phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			     phys_addr_t max_addr)
 {
-	long i, rgn;
+	long i, area;
 	phys_addr_t base = 0;
 	phys_addr_t res_base;
 
@@ -457,15 +456,15 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			continue;
 
 		while (base && lmbbase <= base) {
-			rgn = lmb_overlaps_region(&lmb->reserved, base, size);
-			if (rgn < 0) {
+			area = lmb_overlaps_region(&lmb->reserved, base, size);
+			if (area < 0) {
 				/* This area isn't reserved, take it */
-				if (lmb_add_region(&lmb->reserved, base,
-						   size) < 0)
+				if (lmb_add_area(&lmb->reserved, base, size)
+				    < 0)
 					return 0;
 				return base;
 			}
-			res_base = lmb->reserved.area[rgn].base;
+			res_base = lmb->reserved.area[area].base;
 			if (res_base < size)
 				break;
 			base = lmb_align_down(res_base - size, align);
@@ -481,17 +480,17 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
  */
 phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 {
-	long rgn;
+	int area;
 
-	/* Check if the requested address is in one of the memory regions */
-	rgn = lmb_overlaps_region(&lmb->memory, base, size);
-	if (rgn >= 0) {
+	/* Check if the requested address is in one of the memory areas */
+	area = lmb_overlaps_region(&lmb->memory, base, size);
+	if (area >= 0) {
 		/*
 		 * Check if the requested end address is in the same memory
-		 * region we found.
+		 * area we found.
 		 */
-		if (lmb_addrs_overlap(lmb->memory.area[rgn].base,
-				      lmb->memory.area[rgn].size,
+		if (lmb_addrs_overlap(lmb->memory.area[area].base,
+				      lmb->memory.area[area].size,
 				      base + size - 1, 1)) {
 			/* ok, reserve the memory */
 			if (lmb_reserve(lmb, base, size) >= 0)
@@ -505,12 +504,11 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 /* Return number of bytes from a given address that are free */
 phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr)
 {
-	int i;
-	long rgn;
+	int i, area;
 
-	/* check if the requested address is in the memory regions */
-	rgn = lmb_overlaps_region(&lmb->memory, addr, 1);
-	if (rgn >= 0) {
+	/* check if the requested address is in the memory areas */
+	area = lmb_overlaps_region(&lmb->memory, addr, 1);
+	if (area >= 0) {
 		for (i = 0; i < lmb->reserved.cnt; i++) {
 			if (addr < lmb->reserved.area[i].base) {
 				/* first reserved range > requested address */
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 09/14] lmb: Tidy up structure access
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (7 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 08/14] lmb: Update use of region in the C file Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 10/14] lmb: Avoid long for loop counters and function returns Simon Glass
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

In some cases it helps to define a local variable pointing to the
structure being accessed. This avoids lots of repeated code.

There is no need to individually assign each struct member, so use a
structure assignment instead.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/lmb.c | 58 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index 0b03cc4d1c23..862b1cd04953 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -23,20 +23,19 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static void lmb_dump_region(struct lmb_region *rgn, char *name)
 {
-	unsigned long long base, size, end;
-	enum lmb_flags flags;
 	int i;
 
 	printf(" %s.cnt = 0x%lx / max = 0x%lx\n", name, rgn->cnt, rgn->max);
 
 	for (i = 0; i < rgn->cnt; i++) {
-		base = rgn->area[i].base;
-		size = rgn->area[i].size;
-		end = base + size - 1;
-		flags = rgn->area[i].flags;
+		struct lmb_area *area = &rgn->area[i];
+		unsigned long long end;
+
+		end = area->base + area->size - 1;
 
 		printf(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x\n",
-		       name, i, base, end, size, flags);
+		       name, i, (unsigned long long)area->base, end,
+		       (unsigned long long)area->size, area->flags);
 	}
 }
 
@@ -88,11 +87,8 @@ static void lmb_remove_area(struct lmb_region *rgn, unsigned long area)
 {
 	unsigned long i;
 
-	for (i = area; i < rgn->cnt - 1; i++) {
-		rgn->area[i].base = rgn->area[i + 1].base;
-		rgn->area[i].size = rgn->area[i + 1].size;
-		rgn->area[i].flags = rgn->area[i + 1].flags;
-	}
+	for (i = area; i < rgn->cnt - 1; i++)
+		rgn->area[i] = rgn->area[i + 1];
 	rgn->cnt--;
 }
 
@@ -120,6 +116,7 @@ void lmb_init(struct lmb *lmb)
 
 void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align)
 {
+	struct bd_info *bd = gd->bd;
 	ulong bank_end;
 	int bank;
 
@@ -133,12 +130,10 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align)
 	/* adjust sp by 4K to be safe */
 	sp -= align;
 	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
-		if (!gd->bd->bi_dram[bank].size ||
-		    sp < gd->bd->bi_dram[bank].start)
+		if (!bd->bi_dram[bank].size || sp < bd->bi_dram[bank].start)
 			continue;
 		/* Watch out for RAM at end of address space! */
-		bank_end = gd->bd->bi_dram[bank].start +
-			gd->bd->bi_dram[bank].size - 1;
+		bank_end = bd->bi_dram[bank].start + bd->bi_dram[bank].size - 1;
 		if (sp > bank_end)
 			continue;
 		if (bank_end > end)
@@ -242,13 +237,15 @@ static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 
 	/* First try and coalesce this LMB with another. */
 	for (i = 0; i < rgn->cnt; i++) {
-		phys_addr_t abase = rgn->area[i].base;
-		phys_size_t asize = rgn->area[i].size;
-		phys_size_t aflags = rgn->area[i].flags;
+		struct lmb_area *area = &rgn->area[i];
+
+		phys_addr_t abase = area->base;
+		phys_size_t asize = area->size;
+		phys_size_t aflags = area->flags;
 		phys_addr_t end = base + size - 1;
-		phys_addr_t rgnend = abase + asize - 1;
+		phys_addr_t aend = abase + asize - 1;
 
-		if (abase <= base && end <= rgnend) {
+		if (abase <= base && end <= aend) {
 			if (flags == aflags)
 				/* Already have this area, so we're done */
 				return 0;
@@ -260,14 +257,14 @@ static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 		if (adjacent > 0) {
 			if (flags != aflags)
 				break;
-			rgn->area[i].base -= size;
-			rgn->area[i].size += size;
+			area->base -= size;
+			area->size += size;
 			coalesced++;
 			break;
 		} else if (adjacent < 0) {
 			if (flags != aflags)
 				break;
-			rgn->area[i].size += size;
+			area->size += size;
 			coalesced++;
 			break;
 		} else if (lmb_addrs_overlap(base, size, abase, asize)) {
@@ -291,9 +288,7 @@ static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 	/* Couldn't coalesce the LMB, so add it to the sorted table. */
 	for (i = rgn->cnt-1; i >= 0; i--) {
 		if (base < rgn->area[i].base) {
-			rgn->area[i + 1].base = rgn->area[i].base;
-			rgn->area[i + 1].size = rgn->area[i].size;
-			rgn->area[i + 1].flags = rgn->area[i].flags;
+			rgn->area[i + 1] = rgn->area[i];
 		} else {
 			rgn->area[i + 1].base = base;
 			rgn->area[i + 1].size = size;
@@ -533,10 +528,11 @@ int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t addr, int flags)
 	int i;
 
 	for (i = 0; i < lmb->reserved.cnt; i++) {
-		phys_addr_t upper = lmb->reserved.area[i].base +
-			lmb->reserved.area[i].size - 1;
-		if (addr >= lmb->reserved.area[i].base && addr <= upper)
-			return (lmb->reserved.area[i].flags & flags) == flags;
+		struct lmb_area *area = &lmb->reserved.area[i];
+		phys_addr_t upper = area->base + area->size - 1;
+
+		if (addr >= area->base && addr <= upper)
+			return (area->flags & flags) == flags;
 	}
 
 	return 0;
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 10/14] lmb: Avoid long for loop counters and function returns
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (8 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 09/14] lmb: Tidy up structure access Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 11/14] lmb: Change functions returning long to return int Simon Glass
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Use int for loop counters since it is more common. Do the same with the
return value of some internal functions.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/lmb.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index 862b1cd04953..579f98baef87 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -53,8 +53,8 @@ void lmb_dump_all(struct lmb *lmb)
 #endif
 }
 
-static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
-			      phys_addr_t base2, phys_size_t size2)
+static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
+			     phys_addr_t base2, phys_size_t size2)
 {
 	const phys_addr_t base1_end = base1 + size1 - 1;
 	const phys_addr_t base2_end = base2 + size2 - 1;
@@ -62,8 +62,8 @@ static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
 	return base1 <= base2_end && base2 <= base1_end;
 }
 
-static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
-			       phys_addr_t base2, phys_size_t size2)
+static int lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
+			      phys_addr_t base2, phys_size_t size2)
 {
 	if (base2 == base1 + size1)
 		return 1;
@@ -73,7 +73,7 @@ static long lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
 	return 0;
 }
 
-static long lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
+static int lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
 {
 	phys_addr_t base1 = rgn->area[a1].base;
 	phys_size_t size1 = rgn->area[a1].size;
@@ -85,7 +85,7 @@ static long lmb_areas_adjacent(struct lmb_region *rgn, ulong a1, ulong a2)
 
 static void lmb_remove_area(struct lmb_region *rgn, unsigned long area)
 {
-	unsigned long i;
+	uint i;
 
 	for (i = area; i < rgn->cnt - 1; i++)
 		rgn->area[i] = rgn->area[i + 1];
@@ -221,11 +221,11 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
 }
 
 /* This routine called with relocation disabled. */
-static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
-			       phys_size_t size, enum lmb_flags flags)
+static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
+			      phys_size_t size, enum lmb_flags flags)
 {
-	unsigned long coalesced = 0;
-	long adjacent, i;
+	uint coalesced = 0;
+	int adjacent, i;
 
 	if (rgn->cnt == 0) {
 		rgn->area[0].base = base;
@@ -308,8 +308,8 @@ static long lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 	return 0;
 }
 
-static long lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
-			 phys_size_t size)
+static int lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
+			phys_size_t size)
 {
 	return lmb_add_area_flags(rgn, base, size, LMB_NONE);
 }
@@ -386,10 +386,10 @@ long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 	return lmb_reserve_flags(lmb, base, size, LMB_NONE);
 }
 
-static long lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
-				phys_size_t size)
+static int lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
+			       phys_size_t size)
 {
-	unsigned long i;
+	uint i;
 
 	for (i = 0; i < rgn->cnt; i++) {
 		phys_addr_t abase = rgn->area[i].base;
@@ -429,7 +429,7 @@ static phys_addr_t lmb_align_down(phys_addr_t addr, phys_size_t size)
 phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			     phys_addr_t max_addr)
 {
-	long i, area;
+	int i, area;
 	phys_addr_t base = 0;
 	phys_addr_t res_base;
 
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 11/14] lmb: Change functions returning long to return int
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (9 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 10/14] lmb: Avoid long for loop counters and function returns Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 12/14] lmb: Tidy up lmb_overlaps_region() Simon Glass
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

It makes no sense to return long when an int is plenty to provide an error
code and a region position. It is just confusing.

Update the return-value types to keep to this rule.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/lmb.h | 10 +++++-----
 lib/lmb.c     | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index 6f91df43d5d5..c81716b3f1d1 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -96,8 +96,8 @@ void lmb_init(struct lmb *lmb);
 void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void *fdt_blob);
 void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
 				phys_size_t size, void *fdt_blob);
-long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
-long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 /**
  * lmb_reserve_flags - Reserve one area with a specific flags bitfield.
  *
@@ -107,8 +107,8 @@ long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size);
  * @flags:	flags for the memory area
  * Return:	0 if OK, > 0 for coalesced area or a negative error code.
  */
-long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base,
-		       phys_size_t size, enum lmb_flags flags);
+int lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+		      enum lmb_flags flags);
 phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align);
 phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			   phys_addr_t max_addr);
@@ -141,7 +141,7 @@ int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr);
  */
 int lmb_is_reserved_flags(struct lmb *lmb, phys_addr_t addr, int flags);
 
-long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+int lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 
 void lmb_dump_all(struct lmb *lmb);
 void lmb_dump_all_force(struct lmb *lmb);
diff --git a/lib/lmb.c b/lib/lmb.c
index 579f98baef87..0c6244ca47f8 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -315,14 +315,14 @@ static int lmb_add_area(struct lmb_region *rgn, phys_addr_t base,
 }
 
 /* This routine may be called with relocation disabled. */
-long lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_add(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 {
 	struct lmb_region *_rgn = &(lmb->memory);
 
 	return lmb_add_area(_rgn, base, size);
 }
 
-long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 {
 	struct lmb_region *rgn = &(lmb->reserved);
 	phys_addr_t rgnbegin, rgnend;
@@ -373,15 +373,15 @@ long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 				    rgn->area[i].flags);
 }
 
-long lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
-		       enum lmb_flags flags)
+int lmb_reserve_flags(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+		      enum lmb_flags flags)
 {
 	struct lmb_region *_rgn = &(lmb->reserved);
 
 	return lmb_add_area_flags(_rgn, base, size, flags);
 }
 
-long lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 {
 	return lmb_reserve_flags(lmb, base, size, LMB_NONE);
 }
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 12/14] lmb: Tidy up lmb_overlaps_region()
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (10 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 11/14] lmb: Change functions returning long to return int Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 13/14] lmb: Document and tidy lmb_add_region_flags() Simon Glass
  2023-09-01  1:13 ` [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions Simon Glass
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Add a comment to define what this returns. Return a specific error when
no overlap is found.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/lmb.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index 0c6244ca47f8..8a299c97b3ad 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -386,6 +386,13 @@ int lmb_reserve(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 	return lmb_reserve_flags(lmb, base, size, LMB_NONE);
 }
 
+/**
+ * lmb_overlaps_region() - Check if a region overlaps a given base/size
+ *
+ * @base:	base address of the memory region
+ * @size:	size of the memory region
+ * Returns: Region number of overlapping region, if found, else -ENOENT
+ */
 static int lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
 			       phys_size_t size)
 {
@@ -396,10 +403,10 @@ static int lmb_overlaps_region(struct lmb_region *rgn, phys_addr_t base,
 		phys_size_t asize = rgn->area[i].size;
 
 		if (lmb_addrs_overlap(base, size, abase, asize))
-			break;
+			return i;
 	}
 
-	return i < rgn->cnt ? i : -1;
+	return -ENOENT;
 }
 
 phys_addr_t lmb_alloc(struct lmb *lmb, phys_size_t size, ulong align)
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 13/14] lmb: Document and tidy lmb_add_region_flags()
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (11 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 12/14] lmb: Tidy up lmb_overlaps_region() Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  1:13 ` [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions Simon Glass
  13 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

Update this to return an error code so we can tell when it just ran out of
space in its lmb list. Make lmb_addrs_overlap() return a bool.

Add a few function comments while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/lmb.c | 47 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/lib/lmb.c b/lib/lmb.c
index 8a299c97b3ad..fd6326cbf7fe 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -53,8 +53,17 @@ void lmb_dump_all(struct lmb *lmb)
 #endif
 }
 
-static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
-			     phys_addr_t base2, phys_size_t size2)
+/**
+ * lmb_addrs_overlap() - Compare two address ranges for overlap
+ *
+ * @base1:	base address of region 1
+ * @size1:	size of the region 1
+ * @base2:	base address of region 2
+ * @size2:	size of the region 2
+ * Returns: true if the regions overlap, else false
+ */
+static bool lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
+			      phys_addr_t base2, phys_size_t size2)
 {
 	const phys_addr_t base1_end = base1 + size1 - 1;
 	const phys_addr_t base2_end = base2 + size2 - 1;
@@ -62,6 +71,17 @@ static int lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
 	return base1 <= base2_end && base2 <= base1_end;
 }
 
+/**
+ * lmb_addrs_adjacent() - Compare two address ranges for adjacency
+ *
+ * @base1:	base address of region 1
+ * @size1:	size of the region 1
+ * @base2:	base address of region 2
+ * @size2:	size of the region 2
+ * Returns: 1 if region 2 start at the end of region 1,
+ *	-1 if region 1 starts at the end of region 2,
+ *	else 0
+ */
 static int lmb_addrs_adjacent(phys_addr_t base1, phys_size_t size1,
 			      phys_addr_t base2, phys_size_t size2)
 {
@@ -220,7 +240,22 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
 	lmb_reserve_common(lmb, fdt_blob);
 }
 
-/* This routine called with relocation disabled. */
+/**
+ * lmb_add_area_flags() - Add a region or coalesce with another similar one
+ *
+ * This will coalesce with an existing regions so long as @flags is the same.
+ *
+ * This routine is called with relocation disabled.
+ *
+ * @rgn:	Region to process
+ * @base:	base address of the memory region to add
+ * @size:	size of the memory region to add
+ * @flags:	flags for this new memory region
+ * Returns: 0 if OK, -EBUSY if an existing enveloping region has different
+ * flags, -EPERM if there is an existing non-adjacent region, -ENOSPC if there
+ * is no more room in the list of regions, ekse region number that was coalesced
+ * with this one
+ **/
 static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 			      phys_size_t size, enum lmb_flags flags)
 {
@@ -250,7 +285,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 				/* Already have this area, so we're done */
 				return 0;
 			else
-				return -1; /* areas with new flags */
+				return -EBUSY; /* areas with new flags */
 		}
 
 		adjacent = lmb_addrs_adjacent(base, size, abase, asize);
@@ -269,7 +304,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 			break;
 		} else if (lmb_addrs_overlap(base, size, abase, asize)) {
 			/* areas overlap */
-			return -1;
+			return -EPERM;
 		}
 	}
 
@@ -283,7 +318,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 	if (coalesced)
 		return coalesced;
 	if (rgn->cnt >= rgn->max)
-		return -1;
+		return -ENOSPC;
 
 	/* Couldn't coalesce the LMB, so add it to the sorted table. */
 	for (i = rgn->cnt-1; i >= 0; i--) {
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions
  2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
                   ` (12 preceding siblings ...)
  2023-09-01  1:13 ` [PATCH v2 13/14] lmb: Document and tidy lmb_add_region_flags() Simon Glass
@ 2023-09-01  1:13 ` Simon Glass
  2023-09-01  6:23   ` Heinrich Schuchardt
  13 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2023-09-01  1:13 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Tom Rini, Simon Glass, Heinrich Schuchardt, Michal Simek,
	Patrick Delaunay, Sjoerd Simons

When lmb runs out of space in its internal tables, it gives errors on
every fs load operation. This is horribly confusing, as the poor user
tries different memory regions one at a time.

Use the updated API to check the error code and print a helpful message.
Also, allow the operation to proceed.

Update the tests accordingly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 fs/fs.c        |  7 ++++++-
 include/lmb.h  | 19 ++++++++++++++++++-
 lib/lmb.c      | 20 +++++++++++---------
 test/lib/lmb.c | 42 ++++++++++++++++++++----------------------
 4 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 2b815b1db0fe..1a84cb410bf9 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -569,8 +569,13 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
 	lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
 	lmb_dump_all(&lmb);
 
-	if (lmb_alloc_addr(&lmb, addr, read_len) == addr)
+	ret = lmb_alloc_addr(&lmb, addr, read_len, NULL);
+	if (!ret)
 		return 0;
+	if (ret == -E2BIG) {
+		log_warning("Reservation tables exhausted: see CONFIG_LMB_USE_MAX_REGIONS\n");
+		return 0;
+	}
 
 	log_err("** Reading file would overwrite reserved memory **\n");
 	return -ENOSPC;
diff --git a/include/lmb.h b/include/lmb.h
index c81716b3f1d1..f28a872c78d0 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -114,7 +114,24 @@ phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			   phys_addr_t max_addr);
 phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 			     phys_addr_t max_addr);
-phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size);
+
+/**
+ * lmb_alloc_addr() - Allocate memory within a region
+ *
+ * Try to allocate a specific address range: must be in defined memory but not
+ * reserved
+ *
+ * @lmb:	the logical memory block struct
+ * @base:	base address of the memory region
+ * @size:	size of the memory region
+ * @addrp:	if non-NULL, returns the allocated address, on success
+ * Return:	0 if OK, -EPERM if the memory is already allocated, -E2BIG if
+ * there is not enough room in the reservation tables, so
+ * CONFIG_LMB_USE_MAX_REGIONS should be increased
+ */
+int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+		   phys_addr_t *addrp);
+
 phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
 
 /**
diff --git a/lib/lmb.c b/lib/lmb.c
index fd6326cbf7fe..c3d14db07e45 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -252,7 +252,7 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
  * @size:	size of the memory region to add
  * @flags:	flags for this new memory region
  * Returns: 0 if OK, -EBUSY if an existing enveloping region has different
- * flags, -EPERM if there is an existing non-adjacent region, -ENOSPC if there
+ * flags, -EPERM if there is an existing non-adjacent region, -E2BIG if there
  * is no more room in the list of regions, ekse region number that was coalesced
  * with this one
  **/
@@ -318,7 +318,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
 	if (coalesced)
 		return coalesced;
 	if (rgn->cnt >= rgn->max)
-		return -ENOSPC;
+		return -E2BIG;
 
 	/* Couldn't coalesce the LMB, so add it to the sorted table. */
 	for (i = rgn->cnt-1; i >= 0; i--) {
@@ -511,11 +511,8 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
 	return 0;
 }
 
-/*
- * Try to allocate a specific address range: must be in defined memory but not
- * reserved
- */
-phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
+int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
+		   phys_addr_t *addrp)
 {
 	int area;
 
@@ -529,9 +526,14 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
 		if (lmb_addrs_overlap(lmb->memory.area[area].base,
 				      lmb->memory.area[area].size,
 				      base + size - 1, 1)) {
+			int ret;
+
 			/* ok, reserve the memory */
-			if (lmb_reserve(lmb, base, size) >= 0)
-				return base;
+			ret = lmb_reserve(lmb, base, size);
+			if (ret < 0)
+				return ret;
+			if (addrp)
+				*addrp = base;
 		}
 	}
 
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index b665d8dcdeb4..c8dbdb3b73eb 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -497,22 +497,22 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
 		   alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);
 
 	/* allocate blocks */
-	a = lmb_alloc_addr(&lmb, ram, alloc_addr_a - ram);
+	ut_assertok(lmb_alloc_addr(&lmb, ram, alloc_addr_a - ram, &a));
 	ut_asserteq(a, ram);
 	ASSERT_LMB(&lmb, ram, ram_size, 3, ram, 0x8010000,
 		   alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);
-	b = lmb_alloc_addr(&lmb, alloc_addr_a + 0x10000,
-			   alloc_addr_b - alloc_addr_a - 0x10000);
+	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_a + 0x10000,
+				   alloc_addr_b - alloc_addr_a - 0x10000, &b));
 	ut_asserteq(b, alloc_addr_a + 0x10000);
 	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x10010000,
 		   alloc_addr_c, 0x10000, 0, 0);
-	c = lmb_alloc_addr(&lmb, alloc_addr_b + 0x10000,
-			   alloc_addr_c - alloc_addr_b - 0x10000);
+	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_b + 0x10000,
+				   alloc_addr_c - alloc_addr_b - 0x10000, &c));
 	ut_asserteq(c, alloc_addr_b + 0x10000);
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
 		   0, 0, 0, 0);
-	d = lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000,
-			   ram_end - alloc_addr_c - 0x10000);
+	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000,
+				   ram_end - alloc_addr_c - 0x10000, &d));
 	ut_asserteq(d, alloc_addr_c + 0x10000);
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, ram_size,
 		   0, 0, 0, 0);
@@ -528,7 +528,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
 
 	/* allocate at 3 points in free range */
 
-	d = lmb_alloc_addr(&lmb, ram_end - 4, 4);
+	ut_assertok(lmb_alloc_addr(&lmb, ram_end - 4, 4, &d));
 	ut_asserteq(d, ram_end - 4);
 	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x18010000,
 		   d, 4, 0, 0);
@@ -537,7 +537,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
 		   0, 0, 0, 0);
 
-	d = lmb_alloc_addr(&lmb, ram_end - 128, 4);
+	ut_assertok(lmb_alloc_addr(&lmb, ram_end - 128, 4, &d));
 	ut_asserteq(d, ram_end - 128);
 	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x18010000,
 		   d, 4, 0, 0);
@@ -546,7 +546,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
 		   0, 0, 0, 0);
 
-	d = lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000, 4);
+	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000, 4, &d));
 	ut_asserteq(d, alloc_addr_c + 0x10000);
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010004,
 		   0, 0, 0, 0);
@@ -560,19 +560,19 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
 	ut_asserteq(ret, 0);
 	ASSERT_LMB(&lmb, ram, ram_size, 1, ram + 0x8000000, 0x10010000,
 		   0, 0, 0, 0);
-	d = lmb_alloc_addr(&lmb, ram, 4);
+	ut_assertok(lmb_alloc_addr(&lmb, ram, 4, &d));
 	ut_asserteq(d, ram);
 	ASSERT_LMB(&lmb, ram, ram_size, 2, d, 4,
 		   ram + 0x8000000, 0x10010000, 0, 0);
 
 	/* check that allocating outside memory fails */
 	if (ram_end != 0) {
-		ret = lmb_alloc_addr(&lmb, ram_end, 1);
-		ut_asserteq(ret, 0);
+		ut_assertok(lmb_alloc_addr(&lmb, ram_end, 1, &a));
+		ut_asserteq(0x40000000, a);
 	}
 	if (ram != 0) {
-		ret = lmb_alloc_addr(&lmb, ram - 1, 1);
-		ut_asserteq(ret, 0);
+		ut_assertok(lmb_alloc_addr(&lmb, ram - 1, 1, &a));
+		ut_asserteq(ram, a);
 	}
 
 	return 0;
@@ -588,7 +588,7 @@ static int lib_test_lmb_alloc_addr(struct unit_test_state *uts)
 		return ret;
 
 	/* simulate 512 MiB RAM beginning at 1.5GiB */
-	return test_alloc_addr(uts, 0xE0000000);
+	return test_alloc_addr(uts, 0xe0000000);
 }
 
 DM_TEST(lib_test_lmb_alloc_addr, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
@@ -699,8 +699,7 @@ static int lib_test_LMB_MAX_AREAS(struct unit_test_state *uts)
 
 	/*  error for the (CONFIG_LMB_MAX_AREAS + 1) memory regions */
 	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * ram_size;
-	ret = lmb_add(&lmb, offset, ram_size);
-	ut_asserteq(ret, -1);
+	ut_asserteq(-E2BIG, lmb_add(&lmb, offset, ram_size));
 
 	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
 	ut_asserteq(lmb.reserved.cnt, 0);
@@ -717,8 +716,7 @@ static int lib_test_LMB_MAX_AREAS(struct unit_test_state *uts)
 
 	/*  error for the 9th reserved blocks */
 	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * blk_size;
-	ret = lmb_reserve(&lmb, offset, blk_size);
-	ut_asserteq(ret, -1);
+	ut_asserteq(-E2BIG, lmb_reserve(&lmb, offset, blk_size));
 
 	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
 	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_AREAS);
@@ -762,8 +760,8 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
 		   0, 0, 0, 0);
 
 	/* reserve again, new flag */
-	ret = lmb_reserve_flags(&lmb, 0x40010000, 0x10000, LMB_NONE);
-	ut_asserteq(ret, -1);
+	ut_asserteq(-EBUSY,
+		    lmb_reserve_flags(&lmb, 0x40010000, 0x10000, LMB_NONE));
 	ASSERT_LMB(&lmb, ram, ram_size, 1, 0x40010000, 0x10000,
 		   0, 0, 0, 0);
 
-- 
2.42.0.283.g2d96d420d3-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 02/14] lmb: Rename interior piece to area
  2023-09-01  1:13 ` [PATCH v2 02/14] lmb: Rename interior piece to area Simon Glass
@ 2023-09-01  6:17   ` Heinrich Schuchardt
  2023-09-01 14:29     ` Simon Glass
  0 siblings, 1 reply; 19+ messages in thread
From: Heinrich Schuchardt @ 2023-09-01  6:17 UTC (permalink / raw)
  To: Simon Glass; +Cc: Tom Rini, Michal Simek, Patrick Delaunay, U-Boot Mailing List

On 9/1/23 03:13, Simon Glass wrote:
> The lmb data structures use the word 'region' to describe the region in
> which the allocations happen, as well as the individual allocations in
> that region. The interior structure is called lmb_property but is
> described as a region.
>
> This is quite confusing. One of the reviewers in v1 asked why we are using
> a property to describe a region!

We currently have:

struct lmb_region - Description of a set of region.
struct lmb_property - Description of one region.

The word 'region' implies that it is contiguous, while 'region set'
implies that its members might not be contiguous.

Calling a set region is what starts the confusion.

Your patch is creating new confusion by calling a member of "a set of
regions" "area".

Please, rename as follows:

lmb_region -> lmb_region_set
lmb_property -> lmb_region

The comments below are only valid if you stick with area which I
strongly discourage.

>
> It seems better to adopt the word 'area' for the internal pieces, since an
> area is smaller than a region.
>
> As a first step to improve this, rename struct lmb_property to lmb_area.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>   include/lmb.h  | 12 ++++++------
>   test/lib/lmb.c |  2 +-
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/lmb.h b/include/lmb.h
> index 231b68b27d91..4b7664f22c1d 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -23,13 +23,13 @@ enum lmb_flags {
>   };
>
>   /**
> - * struct lmb_property - Description of one region.
> + * struct lmb_area - Description of one region.

%s/region/memory area/

For struct lmb_area we need a long text explaining what it describes.

>    *
>    * @base:	Base address of the region.
>    * @size:	Size of the region
>    * @flags:	memory region attributes
>    */
> -struct lmb_property {
> +struct lmb_area {
>   	phys_addr_t base;
>   	phys_size_t size;
>   	enum lmb_flags flags;
> @@ -64,9 +64,9 @@ struct lmb_region {
>   	unsigned long cnt;
>   	unsigned long max;
>   #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)

%s/MAX_REGIONS/MAX_AREAS/

> -	struct lmb_property region[CONFIG_LMB_MAX_REGIONS];
> +	struct lmb_area region[CONFIG_LMB_MAX_REGIONS];

This is really confusing: An area called region and indexed by something
related to regions not areas.

>   #else
> -	struct lmb_property *region;
> +	struct lmb_area *region;

ditto

Best regards

Heinrich

>   #endif
>   };
>
> @@ -87,8 +87,8 @@ struct lmb {
>   	struct lmb_region memory;
>   	struct lmb_region reserved;
>   #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> -	struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
> -	struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
> +	struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
> +	struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
>   #endif
>   };
>
> diff --git a/test/lib/lmb.c b/test/lib/lmb.c
> index 162887503588..59b140fde4ce 100644
> --- a/test/lib/lmb.c
> +++ b/test/lib/lmb.c
> @@ -12,7 +12,7 @@
>   #include <test/test.h>
>   #include <test/ut.h>
>
> -static inline bool lmb_is_nomap(struct lmb_property *m)
> +static inline bool lmb_is_nomap(struct lmb_area *m)
>   {
>   	return m->flags & LMB_NOMAP;
>   }


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions
  2023-09-01  1:13 ` [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions Simon Glass
@ 2023-09-01  6:23   ` Heinrich Schuchardt
  0 siblings, 0 replies; 19+ messages in thread
From: Heinrich Schuchardt @ 2023-09-01  6:23 UTC (permalink / raw)
  To: Simon Glass
  Cc: Tom Rini, Michal Simek, Patrick Delaunay, Sjoerd Simons,
	U-Boot Mailing List

On 9/1/23 03:13, Simon Glass wrote:
> When lmb runs out of space in its internal tables, it gives errors on
> every fs load operation. This is horribly confusing, as the poor user
> tries different memory regions one at a time.
>
> Use the updated API to check the error code and print a helpful message.
> Also, allow the operation to proceed.
>
> Update the tests accordingly.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>   fs/fs.c        |  7 ++++++-
>   include/lmb.h  | 19 ++++++++++++++++++-
>   lib/lmb.c      | 20 +++++++++++---------
>   test/lib/lmb.c | 42 ++++++++++++++++++++----------------------
>   4 files changed, 55 insertions(+), 33 deletions(-)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index 2b815b1db0fe..1a84cb410bf9 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -569,8 +569,13 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
>   	lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>   	lmb_dump_all(&lmb);
>
> -	if (lmb_alloc_addr(&lmb, addr, read_len) == addr)
> +	ret = lmb_alloc_addr(&lmb, addr, read_len, NULL);
> +	if (!ret)
>   		return 0;
> +	if (ret == -E2BIG) {
> +		log_warning("Reservation tables exhausted: see CONFIG_LMB_USE_MAX_REGIONS\n");
> +		return 0;

This is dangerous:

You cannot check if a load operation will override protected memory but
still continue.

Please, return -E2BIG here.

Best regards

Heinrich

> +	}
>
>   	log_err("** Reading file would overwrite reserved memory **\n");
>   	return -ENOSPC;
> diff --git a/include/lmb.h b/include/lmb.h
> index c81716b3f1d1..f28a872c78d0 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -114,7 +114,24 @@ phys_addr_t lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
>   			   phys_addr_t max_addr);
>   phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
>   			     phys_addr_t max_addr);
> -phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size);
> +
> +/**
> + * lmb_alloc_addr() - Allocate memory within a region
> + *
> + * Try to allocate a specific address range: must be in defined memory but not
> + * reserved
> + *
> + * @lmb:	the logical memory block struct
> + * @base:	base address of the memory region
> + * @size:	size of the memory region
> + * @addrp:	if non-NULL, returns the allocated address, on success
> + * Return:	0 if OK, -EPERM if the memory is already allocated, -E2BIG if
> + * there is not enough room in the reservation tables, so
> + * CONFIG_LMB_USE_MAX_REGIONS should be increased
> + */
> +int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
> +		   phys_addr_t *addrp);
> +
>   phys_size_t lmb_get_free_size(struct lmb *lmb, phys_addr_t addr);
>
>   /**
> diff --git a/lib/lmb.c b/lib/lmb.c
> index fd6326cbf7fe..c3d14db07e45 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -252,7 +252,7 @@ void lmb_init_and_reserve_range(struct lmb *lmb, phys_addr_t base,
>    * @size:	size of the memory region to add
>    * @flags:	flags for this new memory region
>    * Returns: 0 if OK, -EBUSY if an existing enveloping region has different
> - * flags, -EPERM if there is an existing non-adjacent region, -ENOSPC if there
> + * flags, -EPERM if there is an existing non-adjacent region, -E2BIG if there
>    * is no more room in the list of regions, ekse region number that was coalesced
>    * with this one
>    **/
> @@ -318,7 +318,7 @@ static int lmb_add_area_flags(struct lmb_region *rgn, phys_addr_t base,
>   	if (coalesced)
>   		return coalesced;
>   	if (rgn->cnt >= rgn->max)
> -		return -ENOSPC;
> +		return -E2BIG;
>
>   	/* Couldn't coalesce the LMB, so add it to the sorted table. */
>   	for (i = rgn->cnt-1; i >= 0; i--) {
> @@ -511,11 +511,8 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, ulong align,
>   	return 0;
>   }
>
> -/*
> - * Try to allocate a specific address range: must be in defined memory but not
> - * reserved
> - */
> -phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
> +int lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size,
> +		   phys_addr_t *addrp)
>   {
>   	int area;
>
> @@ -529,9 +526,14 @@ phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, phys_size_t size)
>   		if (lmb_addrs_overlap(lmb->memory.area[area].base,
>   				      lmb->memory.area[area].size,
>   				      base + size - 1, 1)) {
> +			int ret;
> +
>   			/* ok, reserve the memory */
> -			if (lmb_reserve(lmb, base, size) >= 0)
> -				return base;
> +			ret = lmb_reserve(lmb, base, size);
> +			if (ret < 0)
> +				return ret;
> +			if (addrp)
> +				*addrp = base;
>   		}
>   	}
>
> diff --git a/test/lib/lmb.c b/test/lib/lmb.c
> index b665d8dcdeb4..c8dbdb3b73eb 100644
> --- a/test/lib/lmb.c
> +++ b/test/lib/lmb.c
> @@ -497,22 +497,22 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
>   		   alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);
>
>   	/* allocate blocks */
> -	a = lmb_alloc_addr(&lmb, ram, alloc_addr_a - ram);
> +	ut_assertok(lmb_alloc_addr(&lmb, ram, alloc_addr_a - ram, &a));
>   	ut_asserteq(a, ram);
>   	ASSERT_LMB(&lmb, ram, ram_size, 3, ram, 0x8010000,
>   		   alloc_addr_b, 0x10000, alloc_addr_c, 0x10000);
> -	b = lmb_alloc_addr(&lmb, alloc_addr_a + 0x10000,
> -			   alloc_addr_b - alloc_addr_a - 0x10000);
> +	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_a + 0x10000,
> +				   alloc_addr_b - alloc_addr_a - 0x10000, &b));
>   	ut_asserteq(b, alloc_addr_a + 0x10000);
>   	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x10010000,
>   		   alloc_addr_c, 0x10000, 0, 0);
> -	c = lmb_alloc_addr(&lmb, alloc_addr_b + 0x10000,
> -			   alloc_addr_c - alloc_addr_b - 0x10000);
> +	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_b + 0x10000,
> +				   alloc_addr_c - alloc_addr_b - 0x10000, &c));
>   	ut_asserteq(c, alloc_addr_b + 0x10000);
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
>   		   0, 0, 0, 0);
> -	d = lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000,
> -			   ram_end - alloc_addr_c - 0x10000);
> +	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000,
> +				   ram_end - alloc_addr_c - 0x10000, &d));
>   	ut_asserteq(d, alloc_addr_c + 0x10000);
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, ram_size,
>   		   0, 0, 0, 0);
> @@ -528,7 +528,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
>
>   	/* allocate at 3 points in free range */
>
> -	d = lmb_alloc_addr(&lmb, ram_end - 4, 4);
> +	ut_assertok(lmb_alloc_addr(&lmb, ram_end - 4, 4, &d));
>   	ut_asserteq(d, ram_end - 4);
>   	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x18010000,
>   		   d, 4, 0, 0);
> @@ -537,7 +537,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
>   		   0, 0, 0, 0);
>
> -	d = lmb_alloc_addr(&lmb, ram_end - 128, 4);
> +	ut_assertok(lmb_alloc_addr(&lmb, ram_end - 128, 4, &d));
>   	ut_asserteq(d, ram_end - 128);
>   	ASSERT_LMB(&lmb, ram, ram_size, 2, ram, 0x18010000,
>   		   d, 4, 0, 0);
> @@ -546,7 +546,7 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010000,
>   		   0, 0, 0, 0);
>
> -	d = lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000, 4);
> +	ut_assertok(lmb_alloc_addr(&lmb, alloc_addr_c + 0x10000, 4, &d));
>   	ut_asserteq(d, alloc_addr_c + 0x10000);
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram, 0x18010004,
>   		   0, 0, 0, 0);
> @@ -560,19 +560,19 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram)
>   	ut_asserteq(ret, 0);
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, ram + 0x8000000, 0x10010000,
>   		   0, 0, 0, 0);
> -	d = lmb_alloc_addr(&lmb, ram, 4);
> +	ut_assertok(lmb_alloc_addr(&lmb, ram, 4, &d));
>   	ut_asserteq(d, ram);
>   	ASSERT_LMB(&lmb, ram, ram_size, 2, d, 4,
>   		   ram + 0x8000000, 0x10010000, 0, 0);
>
>   	/* check that allocating outside memory fails */
>   	if (ram_end != 0) {
> -		ret = lmb_alloc_addr(&lmb, ram_end, 1);
> -		ut_asserteq(ret, 0);
> +		ut_assertok(lmb_alloc_addr(&lmb, ram_end, 1, &a));
> +		ut_asserteq(0x40000000, a);
>   	}
>   	if (ram != 0) {
> -		ret = lmb_alloc_addr(&lmb, ram - 1, 1);
> -		ut_asserteq(ret, 0);
> +		ut_assertok(lmb_alloc_addr(&lmb, ram - 1, 1, &a));
> +		ut_asserteq(ram, a);
>   	}
>
>   	return 0;
> @@ -588,7 +588,7 @@ static int lib_test_lmb_alloc_addr(struct unit_test_state *uts)
>   		return ret;
>
>   	/* simulate 512 MiB RAM beginning at 1.5GiB */
> -	return test_alloc_addr(uts, 0xE0000000);
> +	return test_alloc_addr(uts, 0xe0000000);
>   }
>
>   DM_TEST(lib_test_lmb_alloc_addr, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
> @@ -699,8 +699,7 @@ static int lib_test_LMB_MAX_AREAS(struct unit_test_state *uts)
>
>   	/*  error for the (CONFIG_LMB_MAX_AREAS + 1) memory regions */
>   	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * ram_size;
> -	ret = lmb_add(&lmb, offset, ram_size);
> -	ut_asserteq(ret, -1);
> +	ut_asserteq(-E2BIG, lmb_add(&lmb, offset, ram_size));
>
>   	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
>   	ut_asserteq(lmb.reserved.cnt, 0);
> @@ -717,8 +716,7 @@ static int lib_test_LMB_MAX_AREAS(struct unit_test_state *uts)
>
>   	/*  error for the 9th reserved blocks */
>   	offset = ram + 2 * (CONFIG_LMB_MAX_AREAS + 1) * blk_size;
> -	ret = lmb_reserve(&lmb, offset, blk_size);
> -	ut_asserteq(ret, -1);
> +	ut_asserteq(-E2BIG, lmb_reserve(&lmb, offset, blk_size));
>
>   	ut_asserteq(lmb.memory.cnt, CONFIG_LMB_MAX_AREAS);
>   	ut_asserteq(lmb.reserved.cnt, CONFIG_LMB_MAX_AREAS);
> @@ -762,8 +760,8 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
>   		   0, 0, 0, 0);
>
>   	/* reserve again, new flag */
> -	ret = lmb_reserve_flags(&lmb, 0x40010000, 0x10000, LMB_NONE);
> -	ut_asserteq(ret, -1);
> +	ut_asserteq(-EBUSY,
> +		    lmb_reserve_flags(&lmb, 0x40010000, 0x10000, LMB_NONE));
>   	ASSERT_LMB(&lmb, ram, ram_size, 1, 0x40010000, 0x10000,
>   		   0, 0, 0, 0);
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 02/14] lmb: Rename interior piece to area
  2023-09-01  6:17   ` Heinrich Schuchardt
@ 2023-09-01 14:29     ` Simon Glass
  2023-09-04 14:46       ` Simon Glass
  0 siblings, 1 reply; 19+ messages in thread
From: Simon Glass @ 2023-09-01 14:29 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Tom Rini, Michal Simek, Patrick Delaunay, U-Boot Mailing List

Hi Heinrich,

On Fri, 1 Sept 2023 at 00:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/1/23 03:13, Simon Glass wrote:
> > The lmb data structures use the word 'region' to describe the region in
> > which the allocations happen, as well as the individual allocations in
> > that region. The interior structure is called lmb_property but is
> > described as a region.
> >
> > This is quite confusing. One of the reviewers in v1 asked why we are using
> > a property to describe a region!
>
> We currently have:
>
> struct lmb_region - Description of a set of region.
> struct lmb_property - Description of one region.
>
> The word 'region' implies that it is contiguous, while 'region set'
> implies that its members might not be contiguous.

From what I can tell, the areas inside a region are contiguous. The
code is so badly commented that it is hard to know what the intent
was.

Actually I just looked it up and found it came from Linux, so I
suppose that explains the lack of comments. There it has been renamed
memblock.

Could you take a look and see if we could adopt the same naming?

>
> Calling a set region is what starts the confusion.
>
> Your patch is creating new confusion by calling a member of "a set of
> regions" "area".
>
> Please, rename as follows:
>
> lmb_region -> lmb_region_set

That sounds like it sets a region

> lmb_property -> lmb_region
>
> The comments below are only valid if you stick with area which I
> strongly discourage.

Fair enough, I don't like it much either.

>
> >
> > It seems better to adopt the word 'area' for the internal pieces, since an
> > area is smaller than a region.
> >
> > As a first step to improve this, rename struct lmb_property to lmb_area.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >   include/lmb.h  | 12 ++++++------
> >   test/lib/lmb.c |  2 +-
> >   2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/lmb.h b/include/lmb.h
> > index 231b68b27d91..4b7664f22c1d 100644
> > --- a/include/lmb.h
> > +++ b/include/lmb.h
> > @@ -23,13 +23,13 @@ enum lmb_flags {
> >   };
> >
> >   /**
> > - * struct lmb_property - Description of one region.
> > + * struct lmb_area - Description of one region.
>
> %s/region/memory area/
>
> For struct lmb_area we need a long text explaining what it describes.
>
> >    *
> >    * @base:   Base address of the region.
> >    * @size:   Size of the region
> >    * @flags:  memory region attributes
> >    */
> > -struct lmb_property {
> > +struct lmb_area {
> >       phys_addr_t base;
> >       phys_size_t size;
> >       enum lmb_flags flags;
> > @@ -64,9 +64,9 @@ struct lmb_region {
> >       unsigned long cnt;
> >       unsigned long max;
> >   #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
>
> %s/MAX_REGIONS/MAX_AREAS/
>
> > -     struct lmb_property region[CONFIG_LMB_MAX_REGIONS];
> > +     struct lmb_area region[CONFIG_LMB_MAX_REGIONS];
>
> This is really confusing: An area called region and indexed by something
> related to regions not areas.
>
> >   #else
> > -     struct lmb_property *region;
> > +     struct lmb_area *region;
>
> ditto
>
> Best regards
>
> Heinrich
>
> >   #endif
> >   };
> >
> > @@ -87,8 +87,8 @@ struct lmb {
> >       struct lmb_region memory;
> >       struct lmb_region reserved;
> >   #if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> > -     struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
> > -     struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
> > +     struct lmb_area memory_regions[CONFIG_LMB_MEMORY_REGIONS];
> > +     struct lmb_area reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
> >   #endif
> >   };
> >
> > diff --git a/test/lib/lmb.c b/test/lib/lmb.c
> > index 162887503588..59b140fde4ce 100644
> > --- a/test/lib/lmb.c
> > +++ b/test/lib/lmb.c
> > @@ -12,7 +12,7 @@
> >   #include <test/test.h>
> >   #include <test/ut.h>
> >
> > -static inline bool lmb_is_nomap(struct lmb_property *m)
> > +static inline bool lmb_is_nomap(struct lmb_area *m)
> >   {
> >       return m->flags & LMB_NOMAP;
> >   }
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 02/14] lmb: Rename interior piece to area
  2023-09-01 14:29     ` Simon Glass
@ 2023-09-04 14:46       ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2023-09-04 14:46 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Tom Rini, Michal Simek, Patrick Delaunay, U-Boot Mailing List

Hi Heinrich,

On Fri, 1 Sept 2023 at 08:29, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Heinrich,
>
> On Fri, 1 Sept 2023 at 00:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > On 9/1/23 03:13, Simon Glass wrote:
> > > The lmb data structures use the word 'region' to describe the region in
> > > which the allocations happen, as well as the individual allocations in
> > > that region. The interior structure is called lmb_property but is
> > > described as a region.
> > >
> > > This is quite confusing. One of the reviewers in v1 asked why we are using
> > > a property to describe a region!
> >
> > We currently have:
> >
> > struct lmb_region - Description of a set of region.
> > struct lmb_property - Description of one region.
> >
> > The word 'region' implies that it is contiguous, while 'region set'
> > implies that its members might not be contiguous.
>
> From what I can tell, the areas inside a region are contiguous. The
> code is so badly commented that it is hard to know what the intent
> was.
>
> Actually I just looked it up and found it came from Linux, so I
> suppose that explains the lack of comments. There it has been renamed
> memblock.
>
> Could you take a look and see if we could adopt the same naming?

I am waiting for your thoughts on this one.

>
> >
> > Calling a set region is what starts the confusion.
> >
> > Your patch is creating new confusion by calling a member of "a set of
> > regions" "area".
> >
> > Please, rename as follows:
> >
> > lmb_region -> lmb_region_set
>
> That sounds like it sets a region
>
> > lmb_property -> lmb_region
> >
> > The comments below are only valid if you stick with area which I
> > strongly discourage.
>
> Fair enough, I don't like it much either.
>
[..]

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-09-04 14:46 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01  1:13 [PATCH v2 00/14] Correct confusing lmb error message Simon Glass
2023-09-01  1:13 ` [PATCH v2 01/14] lmb: Drop surplus brackets and fix code style Simon Glass
2023-09-01  1:13 ` [PATCH v2 02/14] lmb: Rename interior piece to area Simon Glass
2023-09-01  6:17   ` Heinrich Schuchardt
2023-09-01 14:29     ` Simon Glass
2023-09-04 14:46       ` Simon Glass
2023-09-01  1:13 ` [PATCH v2 03/14] lmb: Rename region array " Simon Glass
2023-09-01  1:13 ` [PATCH v2 04/14] lmb: Rename memory/reserved_regions " Simon Glass
2023-09-01  1:13 ` [PATCH v2 05/14] lmb: Rename LMB_MAX_REGIONS and LMB_USE_MAX_REGIONS Simon Glass
2023-09-01  1:13 ` [PATCH v2 06/14] " Simon Glass
2023-09-01  1:13 ` [PATCH v2 07/14] lmb: Update use of region in the header file Simon Glass
2023-09-01  1:13 ` [PATCH v2 08/14] lmb: Update use of region in the C file Simon Glass
2023-09-01  1:13 ` [PATCH v2 09/14] lmb: Tidy up structure access Simon Glass
2023-09-01  1:13 ` [PATCH v2 10/14] lmb: Avoid long for loop counters and function returns Simon Glass
2023-09-01  1:13 ` [PATCH v2 11/14] lmb: Change functions returning long to return int Simon Glass
2023-09-01  1:13 ` [PATCH v2 12/14] lmb: Tidy up lmb_overlaps_region() Simon Glass
2023-09-01  1:13 ` [PATCH v2 13/14] lmb: Document and tidy lmb_add_region_flags() Simon Glass
2023-09-01  1:13 ` [PATCH v2 14/14] fs: Fix a confusing error about overlapping regions Simon Glass
2023-09-01  6:23   ` Heinrich Schuchardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.