[U-Boot] [PATCH RESEND 2/2] cmd: fdt: add fdt overlay application subcommand
Pantelis Antoniou
pantelis.antoniou at konsulko.com
Wed Apr 6 00:03:42 CEST 2016
Hi Maxime,
> On Apr 4, 2016, at 11:25 , Maxime Ripard <maxime.ripard at free-electrons.com> wrote:
>
> The device tree overlays are a good way to deal with user-modifyable
> boards or boards with some kind of an expansion mechanism where we can
> easily plug new board in (like the BBB or the raspberry pi).
>
> However, so far, the usual mechanism to deal with it was to have in Linux
> some driver detecting the expansion boards plugged in and then request
> these overlays using the firmware interface.
>
> That works in most cases, but in some cases, you might want to have the
> overlays applied before the userspace comes in. Either because the new
> board requires some kind of an early initialization, or because your root
> filesystem is accessed through that expansion board.
>
> The easiest solution in such a case is to simply have the component before
> Linux applying that overlay, removing all these drawbacks.
>
This is an interesting patch. My comments inline.
> Signed-off-by: Maxime Ripard <maxime.ripard at free-electrons.com>
> ---
> cmd/Makefile | 2 +-
> cmd/fdt.c | 19 +++
> cmd/fdt_overlay.c | 464 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/fdt_support.h | 2 +-
> 4 files changed, 485 insertions(+), 2 deletions(-)
> create mode 100644 cmd/fdt_overlay.c
>
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 03f7e0a21d2f..09f993971d93 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -52,7 +52,7 @@ obj-$(CONFIG_CMD_EXT4) += ext4.o
> obj-$(CONFIG_CMD_EXT2) += ext2.o
> obj-$(CONFIG_CMD_FAT) += fat.o
> obj-$(CONFIG_CMD_FDC) += fdc.o
> -obj-$(CONFIG_OF_LIBFDT) += fdt.o
> +obj-$(CONFIG_OF_LIBFDT) += fdt.o fdt_overlay.o
> obj-$(CONFIG_CMD_FITUPD) += fitupd.o
> obj-$(CONFIG_CMD_FLASH) += flash.o
> ifdef CONFIG_FPGA
> diff --git a/cmd/fdt.c b/cmd/fdt.c
> index ca6c707dfb48..c875ba688d3b 100644
> --- a/cmd/fdt.c
> +++ b/cmd/fdt.c
> @@ -639,6 +639,24 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> #endif
>
> }
> + /* apply an overlay */
> + else if (strncmp(argv[1], "ap", 2) == 0) {
> + unsigned long addr;
> + struct fdt_header *blob;
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + if (!working_fdt)
> + return CMD_RET_FAILURE;
> +
> + addr = simple_strtoul(argv[2], NULL, 16);
> + blob = map_sysmem(addr, 0);
> + if (!fdt_valid(&blob))
> + return CMD_RET_FAILURE;
> +
> + fdt_overlay_apply(working_fdt, blob);
> + }
> /* resize the fdt */
> else if (strncmp(argv[1], "re", 2) == 0) {
> fdt_shrink_to_minimum(working_fdt);
> @@ -1025,6 +1043,7 @@ static int fdt_print(const char *pathp, char *prop, int depth)
> #ifdef CONFIG_SYS_LONGHELP
> static char fdt_help_text[] =
> "addr [-c] <addr> [<length>] - Set the [control] fdt location to <addr>\n"
> + "fdt apply <addr> - Apply overlay to the DT\n"
> #ifdef CONFIG_OF_BOARD_SETUP
> "fdt boardsetup - Do board-specific set up\n"
> #endif
> diff --git a/cmd/fdt_overlay.c b/cmd/fdt_overlay.c
This looks it’s general libfdt code.
It should end up in libfdt/ so that others can use it, and eventually
be pushed upstream.
> new file mode 100644
> index 000000000000..d2784d791a09
> --- /dev/null
> +++ b/cmd/fdt_overlay.c
> @@ -0,0 +1,464 @@
> +#include <common.h>
> +#include <malloc.h>
> +#include <libfdt.h>
> +
> +#define fdt_for_each_property(fdt, node, property) \
> + for (property = fdt_first_property_offset(fdt, node); \
> + property >= 0; \
> + property = fdt_next_property_offset(fdt, property))
> +
> +struct fdt_overlay_fixup {
> + char label[64];
> + char name[64];
> + char path[64];
^ I understand why you’re using fixed character arrays but there is no
guarantee that the sizes are going to be large enough.
The path is especially worrisome.
Since you’re going to dynamically allocate the fixup, make them pointers;
char *label, *name, *path;
> + int index;
> +};
> +
> +static int fdt_get_max_phandle(const void *dt)
> +{
> + int offset, phandle = 0, new_phandle;
> +
phandle is a uint32_t or u32. Since this is libfdt uint32_t.
> + for (offset = fdt_next_node(dt, -1, NULL); offset >= 0;
> + offset = fdt_next_node(dt, offset, NULL)) {
> + new_phandle = fdt_get_phandle(dt, offset);
> + if (new_phandle > phandle)
> + phandle = new_phandle;
> + }
> +
> + return phandle;
> +}
> +
> +static uint32_t fdt_overlay_get_target_phandle(const void *dt, int node)
> +{
> + const uint32_t *val;
> + int len;
> +
> + val = fdt_getprop(dt, node, "target", &len);
> + if (!val || (len != sizeof(*val)))
> + return 0;
> +
> + return fdt32_to_cpu(*val);
> +}
> +
> +static int fdt_overlay_get_target(const void *dt, const void *dto, int fragment)
> +{
> + uint32_t phandle;
> + const char *path;
> +
> + /* Try first to do a phandle based lookup */
> + phandle = fdt_overlay_get_target_phandle(dto, fragment);
> + if (phandle)
> + return fdt_node_offset_by_phandle(dt, phandle);
> +
> + /* And then a path based lookup */
> + path = fdt_getprop(dto, fragment, "target-path", NULL);
> + if (!path)
> + return -FDT_ERR_NOTFOUND;
> +
> + return fdt_path_offset(dt, path);
> +}
There are going to be more target options here FYI. For now you’re OK.
> +
> +static int fdt_overlay_adjust_node_phandles(void *dto, int node,
> + uint32_t delta)
> +{
> + int property;
> + int child;
> +
> + fdt_for_each_property(dto, node, property) {
> + const uint32_t *val;
> + const char *name;
> + uint32_t adj_val;
> + int len;
> + int ret;
> +
> + val = fdt_getprop_by_offset(dto, property,
> + &name, &len);
> + if (!val || (len != 4))
> + continue;
> +
sizeof(uint32_t)
> + if (strcmp(name, "phandle") && strcmp(name, "linux,phandle"))
> + continue;
> +
> + adj_val = fdt32_to_cpu(*val);
> + adj_val += delta;
> + ret = fdt_setprop_inplace_u32(dto, node, name, adj_val);
> + if (ret) {
> + printf("Failed to ajust property %s phandle\n", name);
> + return ret;
> + }
> + }
> +
> + fdt_for_each_subnode(dto, child, node)
> + fdt_overlay_adjust_node_phandles(dto, child, delta);
> +
> + return 0;
> +}
> +
> +static int fdt_overlay_adjust_local_phandles(void *overlay, uint32_t delta)
> +{
> + int root;
> +
> + root = fdt_path_offset(overlay, "/");
> + if (root < 0) {
> + printf("Couldn't locate the root of the overlay\n");
> + return root;
> + }
> +
> + return fdt_overlay_adjust_node_phandles(overlay, root, delta);
> +}
> +
> +static int _fdt_overlay_update_local_references(void *dto,
> + int tree_node,
> + int fixup_node,
> + uint32_t delta)
> +{
> + int fixup_prop;
> + int fixup_child;
> +
> + fdt_for_each_property(dto, fixup_node, fixup_prop) {
> + const char *fixup_name, *tree_name;
> + const uint32_t *val;
> + uint32_t adj_val;
> + int fixup_len;
> + int tree_prop;
> + int ret;
> +
> + fdt_getprop_by_offset(dto, fixup_prop,
> + &fixup_name, &fixup_len);
> +
> + if (!strcmp(fixup_name, "phandle") ||
> + !strcmp(fixup_name, "linux,phandle"))
> + continue;
> +
> + if (fixup_len != 4) {
> + printf("Illegal property size (%d) %s\n",
> + fixup_len, fixup_name);
> + return -FDT_ERR_NOTFOUND;
> + }
> +
> + fdt_for_each_property(dto, tree_node, tree_prop) {
> + int tree_len;
> +
> + val = fdt_getprop_by_offset(dto, tree_prop,
> + &tree_name, &tree_len);
> +
> + if (!strcmp(tree_name, fixup_name))
> + break;
> + }
> +
> + if (!tree_name) {
> + printf("Couldn't find target property %s\n",
> + fixup_name);
> + return -FDT_ERR_NOTFOUND;
> + }
> +
> + adj_val = fdt32_to_cpu(*val);
> + adj_val += delta;
> + ret = fdt_setprop_inplace_u32(dto, tree_node, fixup_name,
> + adj_val);
> + if (ret) {
> + printf("Failed to ajust property %s phandle\n",
> + fixup_name);
> + return ret;
> + }
> + }
> +
> + fdt_for_each_subnode(dto, fixup_child, fixup_node) {
> + const char *fixup_child_name = fdt_get_name(dto,
> + fixup_child, NULL);
> + int tree_child;
> +
> + fdt_for_each_subnode(dto, tree_child, tree_node) {
> + const char *tree_child_name = fdt_get_name(dto,
> + tree_child,
> + NULL);
> +
> + if (!strcmp(fixup_child_name, tree_child_name))
> + break;
> + }
> +
> + _fdt_overlay_update_local_references(dto,
> + tree_child,
> + fixup_child,
> + delta);
> + }
> +
> + return 0;
> +}
> +
> +static int fdt_overlay_update_local_references(void *dto, uint32_t delta)
> +{
> + int root, fixups;
> +
> + root = fdt_path_offset(dto, "/");
> + if (root < 0) {
> + printf("Couldn't locate the root of the overlay\n");
> + return root;
> + }
> +
> + fixups = fdt_path_offset(dto, "/__local_fixups__");
> + if (root < 0) {
> + printf("Couldn't locate the local fixups in the overlay\n");
> + return root;
> + }
> +
> + return _fdt_overlay_update_local_references(dto, root, fixups,
> + delta);
> +}
> +
> +static struct fdt_overlay_fixup *fdt_fixups_parse_property(void *dto,
> + int property,
> + int *number)
> +{
> + struct fdt_overlay_fixup *fixups;
> + const char *value, *tmp_value;
> + const char *name;
> + int tmp_len, len, next;
> + int table = 0;
> + int i;
> +
> + value = fdt_getprop_by_offset(dto, property,
> + &name, &len);
> + tmp_value = value;
> + tmp_len = len;
> +
> + do {
> + next = strlen(tmp_value) + 1;
> + tmp_len -= next;
> + tmp_value += next;
> + table++;
> + } while (tmp_len > 0);
> +
> + fixups = malloc(table * sizeof(*fixups));
> + if (!fixups)
> + return NULL;
> +
Make a list instead of table here and return that.
> + for (i = 0; i < table; i++) {
> + struct fdt_overlay_fixup *fixup = fixups + i;
> + const char *prop_string = value;
> + char *sep;
> + int stlen;
> +
> +
*fixup = malloc(sizeof(*fixup) + strlen(path) + 1 + strlen(name) + 1 + strlen(label) + 1
point the path, name, label pointers to the buffer space.
> stlen = strlen(prop_string);
> +
> + sep = strchr(prop_string, ':');
> + strncpy(fixup->path, prop_string, sep - prop_string);
> + fixup->path[sep - prop_string] = '\0';
> + prop_string = sep + 1;
> +
> + sep = strchr(prop_string, ':');
> + strncpy(fixup->name, prop_string, sep - prop_string);
> + fixup->name[sep - prop_string] = '\0';
> + prop_string = sep + 1;
> +
> + fixup->index = simple_strtol(prop_string, NULL, 10);
> + strncpy(fixup->label, name, 64);
> +
> + value += stlen + 1;
> + len -= stlen + 1;
> + }
> +
> + *number = table;
> + return fixups;
> +}
> +
> +static int fdt_overlay_fixup_phandles(void *dt, void *dto)
> +{
> + int fixups_off, symbols_off;
> + int property;
> +
> + symbols_off = fdt_path_offset(dt, "/__symbols__");
> + fixups_off = fdt_path_offset(dto, "/__fixups__");
> +
> + fdt_for_each_property(dto, fixups_off, property) {
> + struct fdt_overlay_fixup *fixups;
> + int n_fixups;
> + int i;
> +
> + fixups = fdt_fixups_parse_property(dto, property, &n_fixups);
> + if (!fixups || n_fixups == 0)
> + continue;
> +
^ list iteration now
> + for (i = 0; i < n_fixups; i++) {
> + struct fdt_overlay_fixup *fixup = fixups + i;
> + const uint32_t *prop_val;
> + const char *symbol_path;
> + uint32_t *fixup_val;
> + uint32_t phandle;
> + int symbol_off, fixup_off;
> + int prop_len;
> + int ret;
> +
> + symbol_path = fdt_getprop(dt, symbols_off, fixup->label,
> + &prop_len);
> + if (!symbol_path) {
> + printf("Couldn't lookup symbol %s in the main DT... Skipping\n",
> + fixup->label);
> + continue;
> + }
> +
> + symbol_off = fdt_path_offset(dt, symbol_path);
> + if (symbol_off < 0) {
> + printf("Couldn't match the symbol %s to node %s... Skipping\n",
> + fixup->label, symbol_path);
> + continue;
> + }
> +
> + phandle = fdt_get_phandle(dt, symbol_off);
> + if (!phandle) {
> + printf("Symbol node %s has no phandle... Skipping\n",
> + symbol_path);
> + continue;
> + }
> +
> + fixup_off = fdt_path_offset(dto, fixup->path);
> + if (fixup_off < 0) {
> + printf("Invalid overlay node %s to fixup... Skipping\n",
> + fixup->path);
> + continue;
> + }
> +
> + prop_val = fdt_getprop(dto, fixup_off, fixup->name,
> + &prop_len);
> + if (!prop_val) {
> + printf("Couldn't retrieve property %s/%s value... Skipping\n",
> + fixup->path, fixup->name);
> + continue;
> + }
> +
> + fixup_val = malloc(prop_len);
> + if (!fixup_val)
> + return -FDT_ERR_INTERNAL;
> + memcpy(fixup_val, prop_val, prop_len);
> +
> + if (fdt32_to_cpu(fixup_val[fixup->index]) != 0xdeadbeef) {
> + printf("Value pointed (0x%x) is not supposed to be fixed up... Skipping\n",
> + fdt32_to_cpu(fixup_val[fixup->index]));
> + continue;
> + }
> +
> + fixup_val[fixup->index] = cpu_to_fdt32(phandle);
> + ret = fdt_setprop_inplace(dto, fixup_off,
> + fixup->name, fixup_val,
> + prop_len);
> + if (ret) {
> + printf("Couldn't fixup phandle in overlay property %s/%s (%d)... Skipping\n",
> + fixup->path, fixup->name, ret);
> + }
> + }
> +
> + free(fixups);
> + }
> +
> + return 0;
> +}
> +
> +static int fdt_apply_overlay_node(void *dt, void *dto,
> + int target, int overlay)
> +{
> + int property;
> + int node;
> +
> + fdt_for_each_property(dto, overlay, property) {
> + const char *name;
> + const void *prop;
> + int prop_len;
> + int ret;
> +
> + prop = fdt_getprop_by_offset(dto, property, &name,
> + &prop_len);
> + if (!prop)
> + return -FDT_ERR_INTERNAL;
> +
> + ret = fdt_setprop(dt, target, name, prop, prop_len);
> + if (ret) {
> + printf("Couldn't set property %s\n", name);
> + return ret;
> + }
> + }
> +
> + fdt_for_each_subnode(dto, node, overlay) {
> + const char *name = fdt_get_name(dto, node, NULL);
> + int nnode;
> + int ret;
> +
> + nnode = fdt_add_subnode(dt, target, name);
> + if (nnode < 0) {
> + printf("Couldn't add subnode %s (%d)\n", name, nnode);
> + return nnode;
> + }
> +
> + ret = fdt_apply_overlay_node(dt, dto, nnode, node);
> + if (ret) {
> + printf("Couldn't apply sub-overlay (%d)\n", ret);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int fdt_overlay_merge(void *dt, void *dto)
> +{
> + int root, fragment;
> +
> + root = fdt_path_offset(dto, "/");
> + if (root < 0) {
> + printf("Couldn't locate the root of our overlay\n");
> + return root;
> + }
> +
> + fdt_for_each_subnode(dto, fragment, root) {
> + const char *name = fdt_get_name(dto, fragment, NULL);
> + uint32_t target;
> + int overlay;
> + int ret;
> +
> + if (strncmp(name, "fragment", 8))
> + continue;
> +
> + target = fdt_overlay_get_target(dt, dto, fragment);
> + if (target < 0) {
> + printf("Couldn't locate %s target\n", name);
> + return target;
> + }
> +
> + overlay = fdt_subnode_offset(dto, fragment, "__overlay__");
> + if (overlay < 0) {
> + printf("Couldn't locate %s overlay\n", name);
> + return overlay;
> + }
> +
> + ret = fdt_apply_overlay_node(dt, dto, target, overlay);
> + if (ret) {
> + printf("Couldn't apply %s overlay\n", name);
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +int fdt_overlay_apply(void *dt, void *dto)
> +{
> + uint32_t delta = fdt_get_max_phandle(dt) + 1;
> + int ret;
> +
> + ret = fdt_overlay_adjust_local_phandles(dto, delta);
> + if (ret) {
> + printf("Couldn't adjust local phandles\n");
> + return ret;
> + }
> +
> + ret = fdt_overlay_update_local_references(dto, delta);
> + if (ret) {
> + printf("Couldn't update our local references\n");
> + return ret;
> + }
> +
> + ret = fdt_overlay_fixup_phandles(dt, dto);
> + if (ret) {
> + printf("Couldn't resolve the global phandles\n");
> + return ret;
> + }
> +
> + return fdt_overlay_merge(dt, dto);
> +}
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index 296add01f34f..b4184a767e28 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -240,7 +240,7 @@ int arch_fixup_memory_node(void *blob);
>
> int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width,
> u32 height, u32 stride, const char *format);
> -
> +int fdt_overlay_apply(void *fdt, void *overlay);
> #endif /* ifdef CONFIG_OF_LIBFDT */
>
> #ifdef USE_HOSTCC
> --
> 2.8.0
>
In general looks good. Nice job.
Regards
— Pantelis
More information about the U-Boot
mailing list