On Wed, 16 Jan 2019 at 14:49, Tom Rini trini@konsulko.com wrote:
On Wed, Jan 16, 2019 at 10:44:16PM +0100, Simon Goldschmidt wrote:
Am Mi., 16. Jan. 2019, 22:34 hat Simon Glass sjg@chromium.org geschrieben:
Hi Simon,
On Mon, 14 Jan 2019 at 14:38, Simon Goldschmidt simon.k.r.goldschmidt@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.goldschmidt@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 sjg@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 memoryregions */
j = lmb_overlaps_region(&lmb->memory, base, size);if (j >= 0) {/** Check if the requested end address is in the samememory
* 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 > requestedaddress */
return lmb->reserved.region[i].base -addr;
}if (lmb->reserved.region[i].base +lmb->reserved.region[i].size > addr) {/* requested addr is in this reservedrange */
return 0;}}/* if we come here: no reserved ranges above requestedaddr */
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...
Do it as a follow-up please, I'm testing v10 atm. Thanks!
Yes, these are all minor points and you have my review tag, thank you.
Regards, Simon