Am Mi., 16. Jan. 2019, 22:34 hat Simon Glass <s...@chromium.org> geschrieben:
> Hi Simon, > > On Mon, 14 Jan 2019 at 14:38, Simon Goldschmidt > <simon.k.r.goldschm...@gmail.com> wrote: > > > > This adds two new functions, lmb_alloc_addr and > > lmb_get_unreserved_size. > > > > lmb_alloc_addr behaves like lmb_alloc, but it tries to allocate a > > pre-specified address range. Unlike lmb_reserve, this address range > > must be inside one of the memory ranges that has been set up with > > lmb_add. > > > > lmb_get_unreserved_size returns the number of bytes that can be > > used up to the next reserved region or the end of valid ram. This > > can be 0 if the address passed is reserved. > > > > Added test for these new functions. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com> > > --- > > > > Changes in v10: None > > Changes in v9: None > > Changes in v8: None > > Changes in v7: None > > Changes in v6: None > > Changes in v5: > > - fixed lmb_alloc_addr when resulting reserved ranges get combined > > - added test for these new functions > > > > Changes in v4: None > > Changes in v2: > > - added lmb_get_unreserved_size() for tftp > > > > include/lmb.h | 3 + > > lib/lmb.c | 53 +++++++++++++ > > test/lib/lmb.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 258 insertions(+) > > Reviewed-by: Simon Glass <s...@chromium.org> > > But please see suggestions/nits below. > > > > > diff --git a/include/lmb.h b/include/lmb.h > > index f04d058093..7d7e2a78dc 100644 > > --- a/include/lmb.h > > +++ b/include/lmb.h > > @@ -38,6 +38,9 @@ extern phys_addr_t lmb_alloc_base(struct lmb *lmb, > phys_size_t size, ulong align > > phys_addr_t max_addr); > > extern phys_addr_t __lmb_alloc_base(struct lmb *lmb, phys_size_t size, > ulong align, > > phys_addr_t max_addr); > > +extern phys_addr_t lmb_alloc_addr(struct lmb *lmb, phys_addr_t base, > > + phys_size_t size); > > Can you please add full comments in the header for new functions. > Sure I can but wouldn't it look odd to have one function documented in the header but not the rest? > > +extern phys_size_t lmb_get_unreserved_size(struct lmb *lmb, phys_addr_t > addr); > > extern int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr); > > extern long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t > size); > > > > diff --git a/lib/lmb.c b/lib/lmb.c > > index cd297f8202..e380a0a722 100644 > > --- a/lib/lmb.c > > +++ b/lib/lmb.c > > @@ -313,6 +313,59 @@ phys_addr_t __lmb_alloc_base(struct lmb *lmb, > phys_size_t size, ulong align, phy > > 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) > > +{ > > + long j; > > How about addr instead of j? I think single-char vars are OK for loop > counters, etc. but this is not that. > Sure. > > + > > + /* Check if the requested address is in one of the memory > regions */ > > + j = lmb_overlaps_region(&lmb->memory, base, size); > > + if (j >= 0) { > > + /* > > + * Check if the requested end address is in the same > memory > > + * region we found. > > + */ > > + if (lmb_addrs_overlap(lmb->memory.region[j].base, > > + lmb->memory.region[j].size, base + > size - > > + 1, 1)) { > > + /* ok, reserve the memory */ > > + if (lmb_reserve(lmb, base, size) >= 0) > > + return base; > > + } > > + } > > + return 0; > > +} > > + > > +/* Return number of bytes from a given address that are free */ > > +phys_size_t lmb_get_unreserved_size(struct lmb *lmb, phys_addr_t addr) > > I support you use 'unreserved' instead of 'free' due to some subtle > difference in meaning? Can you add a comment somewhere about this? > Actually no, the name could be changed to 'lmb_get_free_size' if you like. > > +{ > > + int i; > > + long j; > > Here too - addr? > Yes. > > + > > + /* check if the requested address is in the memory regions */ > > + j = lmb_overlaps_region(&lmb->memory, addr, 1); > > + if (j >= 0) { > > + for (i = 0; i < lmb->reserved.cnt; i++) { > > + if (addr < lmb->reserved.region[i].base) { > > + /* first reserved range > requested > address */ > > + return lmb->reserved.region[i].base - > addr; > > + } > > + if (lmb->reserved.region[i].base + > > + lmb->reserved.region[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 0; > > +} > > + > Sigh, I'll re-spin again, but I won't find the time to do so before next week... Regards, Simon > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot