[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCHv3 3/6] plat/common: Introduce fdt_get_address helper
Hi, On 3/21/19 8:05 AM, Jia He wrote: From: Wei Chen <wei.chen@xxxxxxx> This helper will be used very frequently for device libraries to parse their addresses. Introduce this helper to avoid using fdt_address_cells and fdt_size_cells everywhere. Signed-off-by: Wei Chen <wei.chen@xxxxxxx> Signed-off-by: Jianyong Wu <jianyong.wu@xxxxxxx> Signed-off-by: Jia He <justin.he@xxxxxxx> --- plat/common/arm/irq_fdt.c | 65 +++++++++++++++++++++++++++++++ plat/common/include/arm/irq_fdt.h | 19 +++++++++ 2 files changed, 84 insertions(+) diff --git a/plat/common/arm/irq_fdt.c b/plat/common/arm/irq_fdt.c index 9b4bd48..90260c4 100644 --- a/plat/common/arm/irq_fdt.c +++ b/plat/common/arm/irq_fdt.c @@ -38,6 +38,8 @@ #include <libfdt.h>#include "libfdt_internal.h"+#include "uk/print.h" + int fdt_getprop_u32_by_offset(const void *fdt, int offset, const char *name, uint32_t *out) { @@ -97,3 +99,66 @@ int fdt_interrupt_cells(const void *fdt, int offset)return val;} + +static uint64_t fdt_reg_read_number(const fdt32_t *regs, uint32_t size) +{ + uint64_t number = 0; + + if (size >= 3 || size <= 0) + return -FDT_ERR_BADNCELLS; + + for (uint32_t i = 0; i < size; i++) { + number <<= 32; + number |= fdt32_to_cpu(*regs); + regs++; + } + + return number; +} + +int fdt_get_address(const void *fdt, int nodeoffset, int index, NIT: Please use "unsigned int" if you number cannot be signed. For instance, I doubt you support signed index. AFAICT, fdt_address_cells will return the #address-cells property for the given node. However, here, you want the #address-cells of the parent.+ uint64_t *addr, uint64_t *size) +{ + int len, prop_addr, prop_size; + int naddr, nsize, term_size; + const void *regs; + const uint32_t *ranges; + + naddr = fdt_address_cells(fdt, nodeoffset); + if (naddr < 0 || naddr >= FDT_MAX_NCELLS) + return -FDT_ERR_BADNCELLS; + + nsize = fdt_size_cells(fdt, nodeoffset); Same here. Furthermore, the default value returned by fdt_size_cells will not match the DT spec. The spec requires to return 1 and not 2. + if (nsize < 0 || nsize >= FDT_MAX_NCELLS) + return -FDT_ERR_BADNCELLS; + + /* Handle ranges property */ + ranges = fdt_getprop(fdt, nodeoffset, "ranges", &len); + if (ranges == NULL) { + uk_pr_warn("ranges property is not found\n"); Similar as above, this should be lookup in the parent. Also, the "root" node does not contain a property "ranges". So you will not be able to translate node that are directly under the "root" node. A lot of the comments here could have been caught by reading DTS example along with the spec and testing the code. Can you please make sure you do the both before sending a new version. + return -FDT_ERR_NOTFOUND; + } + /* Only 1:1 translation is supported */ This check is a bit fragile. It only promises you the address space of this bus and the address space of this bus node's parent are the same. It does not tell you if the bus above you is using a different address space. + if (len != 0) { + uk_pr_warn("Only 1:1 translation is supported\n"); + return -FDT_ERR_BADVALUE; + } + + /* Get reg content */ + regs = fdt_getprop(fdt, nodeoffset, "reg", &len); + if (regs == NULL) + return -FDT_ERR_NOTFOUND; + + term_size = (int)sizeof(fdt32_t) * (nsize + naddr); Why do you need to cast to int? + prop_addr = term_size * index; + prop_size = prop_addr + (int)sizeof(fdt32_t) * naddr; Same here. + + /* The reg content must cover the reg term[index] at least */ + if (len < (prop_addr + term_size)) + return -FDT_ERR_NOSPACE; + + *addr = fdt_reg_read_number(regs + prop_addr, naddr); + *size = fdt_reg_read_number(regs + prop_size, nsize); + + return 0; +} diff --git a/plat/common/include/arm/irq_fdt.h b/plat/common/include/arm/irq_fdt.h index 4b76c98..2637a58 100644 --- a/plat/common/include/arm/irq_fdt.h +++ b/plat/common/include/arm/irq_fdt.h @@ -73,3 +73,22 @@ int fdt_getprop_u32_by_offset(const void *fdt, int nodeoffset, * -FDT_ERR_TRUNCATED, standard meanings */ int fdt_interrupt_cells(const void *fdt, int nodeoffset); + +/** + * fdt_get_address - retrieve device address of a given index + * @fdt: pointer to the device tree blob + * @nodeoffset: offset of the node to find the address for. + * @index: index of region + * @addr: return the region address + * @size: return the region size + * + * returns: + * 0, on success + * -FDT_ERR_BADNCELLS, if the node has a badly formatted or invalid + * address property + * -FDT_ERR_NOTFOUND, if the node doesn't have address property + * -FDT_ERR_NOSPACE, if the node doesn't have address for index + */ +int fdt_get_address(const void *fdt, int nodeoffset, int index, + uint64_t *addr, uint64_t *size); + Cheers, -- Julien Grall _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |