On Fri, Jun 06, 2014 at 08:38:35PM +0200, Christoffer Dall wrote: > On Thu, Apr 10, 2014 at 06:56:49PM +0200, Andrew Jones wrote: > > Add some device tree functions, built on libfdt, to the arch-neutral > > lib code in order to facilitate the extraction of boot info and device > > base addresses. These functions should work on device trees conforming > > to section III of the kernel doc > > Documentation/devicetree/booting-without-of.txt. > > > > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx> > > --- > > v4: reworked everything, added lots of comments to devicetree.h > > => reworked everything > that will make it easy to review again ;) > > > --- > > lib/devicetree.c | 289 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > lib/devicetree.h | 230 +++++++++++++++++++++++++++++++++++++++++++ > > lib/libcflat.h | 2 + > > 3 files changed, 521 insertions(+) > > create mode 100644 lib/devicetree.c > > create mode 100644 lib/devicetree.h > > > > diff --git a/lib/devicetree.c b/lib/devicetree.c > > new file mode 100644 > > index 0000000000000..482028f066346 > > --- /dev/null > > +++ b/lib/devicetree.c > > @@ -0,0 +1,289 @@ > > +/* > > + * Copyright (C) 2014, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx> > > + * > > + * This work is licensed under the terms of the GNU LGPL, version 2. > > + */ > > +#include "libcflat.h" > > +#include "libfdt/libfdt.h" > > +#include "devicetree.h" > > + > > +static const void *fdt; > > +static u32 root_nr_address_cells, root_nr_size_cells; > > + > > +const void *dt_fdt(void) > > +{ > > + return fdt; > > +} > > + > > +bool dt_available(void) > > +{ > > + return fdt_check_header(fdt) == 0; > > +} > > + > > +int dt_get_nr_cells(int fdtnode, u32 *nr_address_cells, u32 *nr_size_cells) > > +{ > > + const struct fdt_property *prop; > > + u32 *nr_cells; > > + int len; > > + > > + prop = fdt_get_property(fdt, fdtnode, "#address-cells", &len); > > + if (prop == NULL) > > + return len; > > + > > + nr_cells = (u32 *)prop->data; > > + *nr_address_cells = fdt32_to_cpu(*nr_cells); > > + > > + prop = fdt_get_property(fdt, fdtnode, "#size-cells", &len); > > + if (prop == NULL) > > + return len; > > + > > + nr_cells = (u32 *)prop->data; > > + *nr_size_cells = fdt32_to_cpu(*nr_cells); > > + > > + return 0; > > +} > > + > > +void dt_reg_init(struct dt_reg *reg, u32 nr_address_cells, u32 nr_size_cells) > > +{ > > + memset(reg, 0, sizeof(struct dt_reg)); > > + reg->nr_address_cells = nr_address_cells; > > + reg->nr_size_cells = nr_size_cells; > > +} > > + > > +int dt_get_reg(int fdtnode, int regidx, struct dt_reg *reg) > > +{ > > + const struct fdt_property *prop; > > + u32 *cells, i; > > + unsigned nr_tuple_cells; > > + int len; > > + > > + prop = fdt_get_property(fdt, fdtnode, "reg", &len); > > + if (prop == NULL) > > + return len; > > + > > + cells = (u32 *)prop->data; > > + nr_tuple_cells = reg->nr_address_cells + reg->nr_size_cells; > > + regidx *= nr_tuple_cells; > > + > > + if (regidx + nr_tuple_cells > len/sizeof(u32)) > > Shouldn't this be >= ? Not this time. The LHS is the total number of words needed (not an index). > > > + return -FDT_ERR_NOTFOUND; > > + > > + for (i = 0; i < reg->nr_address_cells; ++i) > > + reg->address_cells[i] = fdt32_to_cpu(cells[regidx + i]); > > + > > + regidx += reg->nr_address_cells; > > + for (i = 0; i < reg->nr_size_cells; ++i) > > + reg->size_cells[i] = fdt32_to_cpu(cells[regidx + i]); > > + > > + return 0; > > +} > > + > > +int dt_pbus_translate_node(int fdtnode, int regidx, void *reg) > > why doesn't this function take a struct dt_pbus_reg * instead of a > void *? Guess I forgot my coffee when I wrote it... I'll change that. > > > +{ > > + struct dt_pbus_reg *pbus_reg = (struct dt_pbus_reg *)reg; > > + struct dt_reg raw_reg; > > + int ret; > > + > > + dt_reg_init(&raw_reg, root_nr_address_cells, root_nr_size_cells); > > + > > + ret = dt_get_reg(fdtnode, regidx, &raw_reg); > > + if (ret < 0) > > + return ret; > > + > > + pbus_reg->addr = dt_pbus_read_cells(raw_reg.nr_address_cells, > > + raw_reg.address_cells); > > + pbus_reg->size = dt_pbus_read_cells(raw_reg.nr_size_cells, > > + raw_reg.size_cells); > > + > > + return 0; > > +} > > + > > +int dt_pbus_translate(const struct dt_device *dev, int regidx, > > + void *reg) > > +{ > > + return dt_pbus_translate_node(dev->fdtnode, regidx, reg); > > +} > > + > > +int dt_pbus_get_baseaddr(const struct dt_device *dev, dt_pbus_addr_t *base) > > +{ > > + struct dt_pbus_reg reg; > > + int ret; > > + > > + ret = dt_pbus_translate(dev, 0, ®); > > + if (ret < 0) > > + return ret; > > + > > + *base = reg.addr; > > + return 0; > > +} > > + > > +int dt_bus_match_any(const struct dt_device *dev __unused, int fdtnode) > > +{ > > + /* matches any device with a valid node */ > > + return fdtnode < 0 ? fdtnode : 1; > > +} > > + > > +static const struct dt_bus dt_default_bus = { > > + .match = dt_bus_match_any, > > + .translate = dt_pbus_translate, > > +}; > > + > > +void dt_bus_init_defaults(struct dt_bus *bus) > > +{ > > + memcpy(bus, &dt_default_bus, sizeof(struct dt_bus)); > > +} > > + > > +void dt_device_init(struct dt_device *dev, const struct dt_bus *bus, > > + const void *info) > > +{ > > + memset(dev, 0, sizeof(struct dt_device)); > > + dev->bus = bus; > > + dev->info = (void *)info; > > +} > > + > > +int dt_device_find_compatible(const struct dt_device *dev, > > + const char *compatible) > > +{ > > + int node, ret; > > + > > + node = fdt_node_offset_by_compatible(fdt, -1, compatible); > > + while (node >= 0) { > > + ret = dev->bus->match(dev, node); > > + if (ret < 0) > > + return ret; > > + else if (ret) > > + break; > > + node = fdt_node_offset_by_compatible(fdt, node, compatible); > > + } > > + return node; > > +} > > + > > +int dt_pbus_get_baseaddr_compatible(const char *compatible, > > + dt_pbus_addr_t *baseaddr) > > +{ > > + struct dt_device dev; > > + int node, ret; > > + > > + dt_device_init(&dev, &dt_default_bus, NULL); > > + > > + node = dt_device_find_compatible(&dev, compatible); > > + if (node < 0) > > + return node; > > + > > + dt_device_bind_node(&dev, node); > > + > > + ret = dt_pbus_get_baseaddr(&dev, baseaddr); > > + if (ret < 0) > > + return ret; > > + > > + return 0; > > +} > > + > > +int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs) > > +{ > > + const char *pn = "device_type", *pv = "memory"; > > + int node, ret, pl = strlen(pv) + 1, nr = 0; > > + struct dt_pbus_reg reg; > > + > > + node = fdt_node_offset_by_prop_value(fdt, -1, pn, pv, pl); > > + > > + while (node >= 0) { > > + > > + while (nr < nr_regs) { > > + ret = dt_pbus_translate_node(node, nr, ®); > > + if (ret == -FDT_ERR_NOTFOUND) > > + break; > > + if (ret < 0) > > + return ret; > > + regs[nr].addr = reg.addr; > > + regs[nr].size = reg.size; > > + ++nr; > > + } > > + > > + node = fdt_node_offset_by_prop_value(fdt, node, pn, pv, pl); > > + } > > + > > + return node != -FDT_ERR_NOTFOUND ? node : nr; > > +} > > + > > +int dt_for_each_cpu_node(void (*func)(int fdtnode, u32 regval, void *info), > > + void *info) > > +{ > > + const struct fdt_property *prop; > > + int cpus, cpu, ret, len; > > + struct dt_reg raw_reg; > > + u32 nac, nsc; > > + > > + cpus = fdt_path_offset(fdt, "/cpus"); > > + if (cpus < 0) > > + return cpus; > > + > > + ret = dt_get_nr_cells(cpus, &nac, &nsc); > > + if (ret < 0) > > + return ret; > > + > > + dt_reg_init(&raw_reg, nac, nsc); > > + > > + dt_for_each_subnode(cpus, cpu) { > > + > > + prop = fdt_get_property(fdt, cpu, "device_type", &len); > > + if (prop == NULL) > > + return len; > > + > > + if (len != 4 || strcmp((char *)prop->data, "cpu")) > > + continue; > > + > > + ret = dt_get_reg(cpu, 0, &raw_reg); > > + if (ret < 0) > > + return ret; > > + > > + func(cpu, raw_reg.address_cells[0], info); > > + } > > + > > + return 0; > > +} > > + > > +int dt_get_bootargs(const char **bootargs) > > +{ > > + const struct fdt_property *prop; > > + int node, len; > > + > > + *bootargs = NULL; > > + > > + node = fdt_path_offset(fdt, "/chosen"); > > + if (node < 0) > > + return node; > > + > > + prop = fdt_get_property(fdt, node, "bootargs", &len); > > + if (prop) > > + *bootargs = (char *)prop->data; > > + else if (len < 0 && len != -FDT_ERR_NOTFOUND) > > + return len; > > so if you get FDT_ERR_NOTFOUND you still return success but the bootargs > won't be set? why? bootargs will be NULL in that case, which is enough to know there wasn't any. I also wanted to be able to easily check for any other error without having to filter FDT_ERR_NOTFOUND at every call site. I can skip this special behavior though, if a more consistent interface is preferred. > > > + > > + return 0; > > +} > > + > > +int dt_init(const void *fdt_ptr) > > +{ > > + struct dt_bus *defbus = (struct dt_bus *)&dt_default_bus; > > + int root, ret; > > + > > + ret = fdt_check_header(fdt_ptr); > > + if (ret < 0) > > + return ret; > > + fdt = fdt_ptr; > > + > > + root = fdt_path_offset(fdt, "/"); > > + if (root < 0) > > + return root; > > + > > + ret = dt_get_nr_cells(root, &root_nr_address_cells, > > + &root_nr_size_cells); > > + if (ret < 0) > > + return ret; > > + > > + defbus->nr_address_cells = root_nr_address_cells; > > + defbus->nr_size_cells = root_nr_size_cells; > > + > > + return 0; > > +} > > diff --git a/lib/devicetree.h b/lib/devicetree.h > > new file mode 100644 > > index 0000000000000..032b5497f9db1 > > --- /dev/null > > +++ b/lib/devicetree.h > > @@ -0,0 +1,230 @@ > > +#ifndef _DEVICETREE_H_ > > +#define _DEVICETREE_H_ > > +/* > > + * devicetree builds on libfdt to implement abstractions and accessors > > + * for Linux required device tree content. The accessors provided are > > + * common across architectures. See section III of the kernel doc > > + * Documentation/devicetree/booting-without-of.txt > > + * > > + * Copyright (C) 2014, Red Hat Inc, Andrew Jones <drjones@xxxxxxxxxx> > > + * > > + * This work is licensed under the terms of the GNU LGPL, version 2. > > + */ > > +#include "libcflat.h" > > +#include "libfdt/libfdt.h" > > + > > +/********************************************************************** > > + * devicetree init and libfdt helpers > > + **********************************************************************/ > > + > > +/* dt_init initializes devicetree with a pointer to an fdt, @fdt_ptr */ > > +extern int dt_init(const void *fdt_ptr); > > + > > +/* get the fdt pointer that devicetree is using */ > > +const void *dt_fdt(void); > > nit: why no extern here? Just forgot it. Will add it. > > > + > > +/* check for an initialized, valid devicetree */ > > +extern bool dt_available(void); > > + > > +/* traverse child nodes */ > > +#define dt_for_each_subnode(n, s) \ > > + for (s = fdt_first_subnode(dt_fdt(), n); \ > > + s != -FDT_ERR_NOTFOUND; \ > > + s = fdt_next_subnode(dt_fdt(), s)) > > + > > +/********************************************************************** > > + * Abstractions for required node types and properties > > + **********************************************************************/ > > + > > +struct dt_device { > > + int fdtnode; > > + const struct dt_bus *bus; > > + > > + /* > > + * info is a pointer to device specific data, which may be > > + * used by the bus match() and translate() functions > > + */ > > + void *info; > > +}; > > + > > +struct dt_bus { > > + /* > > + * match a device @dev to an fdt node @fdtnode > > + * returns > > + * - a positive value on match > > + * - zero on no match > > + * - a negative FDT_ERR_* value on failure > > + */ > > + int (*match)(const struct dt_device *dev, int fdtnode); > > + > > + /* > > + * translate the @regidx'th "address size" tuple of > > + * @dev's fdt node's "reg" property, and store the result > > + * in @reg, a bus specific structure > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > + int (*translate)(const struct dt_device *dev, int regidx, void *reg); > > + > > + /* the bus #address-cells and #size-cells properties */ > > + u32 nr_address_cells, nr_size_cells; > > +}; > > + > > +/* dt_bus_match_any matches any fdt node, i.e. it always returns true */ > > +extern int dt_bus_match_any(const struct dt_device *dev, int fdtnode); > > + > > +/* the processor bus (pbus) address type and register tuple */ > > +typedef u64 dt_pbus_addr_t; > > +struct dt_pbus_reg { > > + dt_pbus_addr_t addr; > > + dt_pbus_addr_t size; > > +}; > > + > > +static inline dt_pbus_addr_t dt_pbus_read_cells(u32 nr_cells, u32 *cells) > > +{ > > + return nr_cells == 2 ? ((u64)cells[0] << 32) | cells[1] > > + : nr_cells == 1 ? cells[0] : (~0ULL); > > Why the tertiary operator here? A simple if or select statement should > make the code much easier to read and you don't seem to handle I'll change it. > #address-cells == 4or #size-cells == 4, despite the fact that you define > MAX_ADDRESS_CELLS=4 etc. This seems a bit weird to me. The default translator only knows how to handle nr_cells==2 or nr_cells==1. DT may have nodes with nr_cells > 2 (up to 4), but users will have to provide their own translators for those. > > > +} > > + > > +/* > > + * dt_pbus_translate translates device node regs for the > > + * processor bus using the root node's #address-cells and > > + * #size-cells and dt_pbus_read_cells() > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_pbus_translate(const struct dt_device *dev, int regidx, > > + void *reg); > > + > > +/* > > + * dt_pbus_translate_node is the same as dt_pbus_translate but > > + * operates on an fdt node instead of a dt_device > > + */ > > +extern int dt_pbus_translate_node(int fdtnode, int regidx, void *reg); > > + > > +/* > > + * dt_pbus_get_baseaddr is a shortcut for > > + * dt_pbus_translate(dev, 0, ®); > > + * *base = reg.addr; > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_pbus_get_baseaddr(const struct dt_device *dev, > > + dt_pbus_addr_t *base); > > + > > +/* > > + * dt_bus_init_defaults initializes @bus with > > + * match <- dt_bus_match_any > > + * translate <- dt_pbus_translate > > + * nr_address_cells <- #address-cells of the root node > > + * nr_size_cells <- #size-cells of the root node > > + */ > > +extern void dt_bus_init_defaults(struct dt_bus *bus); > > + > > +/* > > + * dt_device_init initializes a dt_device with the given parameters > > + */ > > +extern void dt_device_init(struct dt_device *dev, const struct dt_bus *bus, > > + const void *info); > > + > > +static inline void dt_device_bind_node(struct dt_device *dev, int fdtnode) > > +{ > > + dev->fdtnode = fdtnode; > > +} > > + > > +/* > > + * dt_device_find_compatible finds a @compatible node > > + * returns > > + * - node (>= 0) on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_device_find_compatible(const struct dt_device *dev, > > + const char *compatible); > > + > > +/* > > + * dt_pbus_get_baseaddr_compatible simply bundles many functions into > > + * one. It finds the first @compatible fdt node and then translates the > > + * 0th reg tuple (the base address) using the processor bus translation, > > + * and finally stores that address in @baseaddr. > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_pbus_get_baseaddr_compatible(const char *compatible, > > + dt_pbus_addr_t *baseaddr); > > + > > +/********************************************************************** > > + * Low-level accessors for required node types and properties > > + **********************************************************************/ > > + > > +/* > > + * dt_get_nr_cells sets @nr_address_cells and @nr_size_cells to the > > + * #address-cells and #size-cells properties of @fdtnode > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_get_nr_cells(int fdtnode, u32 *nr_address_cells, > > + u32 *nr_size_cells); > > + > > +/* dt_reg is a structure for "raw" reg tuples */ > > +#define MAX_ADDRESS_CELLS 4 > > +#define MAX_SIZE_CELLS 4 > > +struct dt_reg { > > + u32 nr_address_cells, nr_size_cells; > > + u32 address_cells[MAX_ADDRESS_CELLS]; > > + u32 size_cells[MAX_SIZE_CELLS]; > > +}; > > + > > +/* > > + * dt_reg_init initialize a dt_reg struct to zero and sets > > + * nr_address_cells and nr_size_cells to @nr_address_cells and > > + * @nr_size_cells respectively. > > + */ > > +extern void dt_reg_init(struct dt_reg *reg, u32 nr_address_cells, > > + u32 nr_size_cells); > > + > > +/* > > + * dt_get_reg gets the @regidx'th reg tuple of @fdtnode's reg property > > + * and stores it in @reg. @reg must be initialized. > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_get_reg(int fdtnode, int regidx, struct dt_reg *reg); > > + > > +/********************************************************************** > > + * High-level accessors for required node types and properties > > + **********************************************************************/ > > + > > +/* > > + * dt_get_bootargs gets a pointer to /chosen/bootargs > > + * returns > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_get_bootargs(const char **bootargs); > > + > > +/* > > + * dt_get_memory_params gets the memory parameters from the /memory node(s) > > + * storing each memory region ("address size" tuple) in consecutive entries > > + * of @regs, up to @nr_regs > > + * returns > > + * - number of memory regions found on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_get_memory_params(struct dt_pbus_reg *regs, int nr_regs); > > + > > +/* > > + * dt_for_each_cpu_node runs @func on each cpu node in the /cpus node > > + * passing it its fdt node, its reg property value, and @info > > + * - zero on success > > + * - a negative FDT_ERR_* value on failure > > + */ > > +extern int dt_for_each_cpu_node(void (*func)(int fdtnode, u32 regval, > > + void *info), void *info); > > + > > +#endif /* _DEVICETREE_H_ */ > > diff --git a/lib/libcflat.h b/lib/libcflat.h > > index 76448a33cde5f..99d1cd533dd03 100644 > > --- a/lib/libcflat.h > > +++ b/lib/libcflat.h > > @@ -22,6 +22,8 @@ > > > > #include <stdarg.h> > > > > +#define __unused __attribute__((__unused__)) > > + > > typedef unsigned char u8; > > typedef signed char s8; > > typedef unsigned short u16; > > -- > > 1.8.1.4 > > > > -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html