Hi Heinrich, On Wed, 23 Aug 2023 at 09:44, Heinrich Schuchardt <xypron.g...@gmx.de> wrote: > > On 23.08.23 15:41, Simon Glass wrote: > > 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 <s...@chromium.org> > > --- > > > > lib/lmb.c | 54 +++++++++++++++++++++++++----------------------------- > > 1 file changed, 25 insertions(+), 29 deletions(-) > > > > diff --git a/lib/lmb.c b/lib/lmb.c > > index ae1969893f00..8b9a611c5216 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->region[i].base; > > - size = rgn->region[i].size; > > - end = base + size - 1; > > - flags = rgn->region[i].flags; > > + struct lmb_property *prop = &rgn->region[i]; > > + unsigned long long end; > > + > > + end = prop->base + prop->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)prop->base, end, > > + (unsigned long long)prop->size, prop->flags); > > } > > } > > > > @@ -89,11 +88,8 @@ 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; > > - } > > + for (i = r; i < rgn->cnt - 1; i++) > > + rgn->region[i] = rgn->region[i + 1]; > > rgn->cnt--; > > } > > > > @@ -122,6 +118,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; > > > > @@ -135,12 +132,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) > > @@ -244,9 +239,11 @@ 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->region[i].base; > > - phys_size_t rgnsize = rgn->region[i].size; > > - phys_size_t rgnflags = rgn->region[i].flags; > > + struct lmb_property *prop = &rgn->region[i]; > > Why call a region prop? Can't we call it "region" or "rgn_ptr" or if you > want to allude to the position in the array "pos"? This would avoid > confusion.
We always have rgn so that would be hopelessly confusing. I am using prop just because that is the struct name. I did not invent the confusion: struct lmb_region { unsigned long cnt; unsigned long max; struct lmb_property region[CONFIG_LMB_MAX_REGIONS]; }; There is a region within a region, and also: struct lmb { struct lmb_region memory; struct lmb_region reserved; struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS]; struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS]; }; I did consider trying to rename one of them...but 'region' is used extensively in the C code to mean the inner region. We could perhaps rename the outer 'region' to an 'area'? Regards, Simon