[U-Boot] [PATCH RESEND 0/2] cmd: fdt: Add device tree overlays support
Hi,
The device tree overlays are a great solution to the issue raised by the bunch expandable boards we find everywhere these days, like the Beaglebone, Raspberry Pi or CHIP.
However, most of the time, the overlays are applied through a mechanism involving the firmware request interface in Linux, that is only fully functional once the userspace has been mounted and is running.
Some expansion boards might need to be enabled before that, because they simply need to patch the DT early on, or need to be initialized early in order to be fully functional, or because they provide access to the root filesystem.
In these cases, having the bootloader applying the overlay before Linux starts seems like the easiest solution.
This implementation doesn't provide all the Linux fancyness though, there's no transactional application, which means that if the overlay cannot be applied for a reason while you're still halfway through the application, you're probably screwed. It also cannot remove an overlay, but I don't think that is currently a use-case.
Let me know what you think, Maxime
Maxime Ripard (2): cmd: fdt: Narrow the check for fdt addr cmd: fdt: add fdt overlay application subcommand
cmd/Makefile | 2 +- cmd/fdt.c | 21 ++- cmd/fdt_overlay.c | 464 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 2 +- 4 files changed, 486 insertions(+), 3 deletions(-) create mode 100644 cmd/fdt_overlay.c
The current code only checks if the fdt subcommand is fdt addr by checking whether it starts with 'a'.
Since this is a pretty widely used letter, narrow down that check a bit.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com --- cmd/fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c index 4c18962d8532..ca6c707dfb48 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -87,7 +87,7 @@ static int do_fdt(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* * Set the address of the fdt */ - if (argv[1][0] == 'a') { + if (strncmp(argv[1], "ad", 2) == 0) { unsigned long addr; int control = 0; struct fdt_header *blob;
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@free-electrons.com wrote:
The current code only checks if the fdt subcommand is fdt addr by checking whether it starts with 'a'.
Since this is a pretty widely used letter, narrow down that check a bit.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
cmd/fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
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.
Signed-off-by: Maxime Ripard maxime.ripard@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 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]; + int index; +}; + +static int fdt_get_max_phandle(const void *dt) +{ + int offset, phandle = 0, new_phandle; + + 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); +} + +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; + + 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; + + for (i = 0; i < table; i++) { + struct fdt_overlay_fixup *fixup = fixups + i; + const char *prop_string = value; + char *sep; + int stlen; + + 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; + + 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
Hi Maxime,
On Apr 4, 2016, at 11:25 , Maxime Ripard maxime.ripard@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@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
On Tue, Apr 5, 2016 at 5:03 PM, Pantelis Antoniou pantelis.antoniou@konsulko.com wrote:
Hi Maxime,
On Apr 4, 2016, at 11:25 , Maxime Ripard maxime.ripard@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.
[...]
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.
+1. It really needs to go into libfdt first to avoid any re-licensing issues.
Another option which Grant has suggested would be to extend the FDT format to include overlays as a whole. Then the kernel can apply them during unflattening. This would simplify things on the bootloader side.
Rob
On Fri, Apr 08, 2016 at 04:29:40PM -0500, Rob Herring wrote:
On Tue, Apr 5, 2016 at 5:03 PM, Pantelis Antoniou pantelis.antoniou@konsulko.com wrote:
Hi Maxime,
On Apr 4, 2016, at 11:25 , Maxime Ripard maxime.ripard@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.
[...]
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.
+1. It really needs to go into libfdt first to avoid any re-licensing issues.
Another option which Grant has suggested would be to extend the FDT format to include overlays as a whole. Then the kernel can apply them during unflattening. This would simplify things on the bootloader side.
Perhaps in some cases? Part of the overall use case here is that something has to dynamically grab the N overlays that apply on the current hardware and make them available. So the bootloader would still need to grab them and pass along to the kernel to apply. And I've already received some pushback on saying it could wait until initrd/initramfs.
Hi Tom,
On Apr 13, 2016, at 22:42 , Tom Rini trini@konsulko.com wrote:
On Fri, Apr 08, 2016 at 04:29:40PM -0500, Rob Herring wrote:
On Tue, Apr 5, 2016 at 5:03 PM, Pantelis Antoniou pantelis.antoniou@konsulko.com wrote:
Hi Maxime,
On Apr 4, 2016, at 11:25 , Maxime Ripard maxime.ripard@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.
[...]
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.
+1. It really needs to go into libfdt first to avoid any re-licensing issues.
Another option which Grant has suggested would be to extend the FDT format to include overlays as a whole. Then the kernel can apply them during unflattening. This would simplify things on the bootloader side.
Perhaps in some cases? Part of the overall use case here is that something has to dynamically grab the N overlays that apply on the current hardware and make them available. So the bootloader would still need to grab them and pass along to the kernel to apply. And I've already received some pushback on saying it could wait until initrd/initramfs.
There are a number of options about what to do.
This patch is made for a specific case of DRM drivers not handling overlays. I expect this is because the DRM device core does not have a reconfig notifier. I plan adding it when I get my CHIP setup.
Grant’s option is to have the kernel do the overlay application by appending the overlay dtbos after the base tree blob.
This patch is orthogonal to both, so let’s review and make up our minds on the updated patch.
-- Tom
Regards
— Pantelis
Hi Maxime,
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@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.
Signed-off-by: Maxime Ripard maxime.ripard@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
I'm happy to take this into the U-Boot tree while you try to get it applied upstream, but please confirm that you will do this and by when. I suggest you sent an email referring to this patch.
Also fdt_overlay.c should go in lib/libfdt/. If this functionality is rejected for libfdt (as was fdtgrep) then we'll move it to lib/.
Finally, please take a look at adding a test for this, with some sample files. See test/py.
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 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];int index;+};
+static int fdt_get_max_phandle(const void *dt) +{
int offset, phandle = 0, new_phandle;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)
Please add function comment here and for others.
+{
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);+}
+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);
Are you sure you are using 80 cols?
if (!val || (len != 4))continue;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)
Can this return an error instead? It seems like failure due to malloc() would be silent.
+{
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;for (i = 0; i < table; i++) {struct fdt_overlay_fixup *fixup = fixups + i;const char *prop_string = value;char *sep;int stlen;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;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);
Does this get freed?
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]));
How would you know what this error refers to? Do you need to print the node path also?
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);
Function comment here.
#endif /* ifdef CONFIG_OF_LIBFDT */
#ifdef USE_HOSTCC
2.8.0
Regards, Simon
Hi Simon,
I'm finally taking the time to address the reviews that have been made here.
On Sat, Apr 09, 2016 at 12:40:14PM -0600, Simon Glass wrote:
Hi Maxime,
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@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.
Signed-off-by: Maxime Ripard maxime.ripard@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
I'm happy to take this into the U-Boot tree while you try to get it applied upstream, but please confirm that you will do this and by when. I suggest you sent an email referring to this patch.
Also fdt_overlay.c should go in lib/libfdt/. If this functionality is rejected for libfdt (as was fdtgrep) then we'll move it to lib/.
I'm definitely ok to send it to libfdt if that's what you meant by upstream. I'll push it at the same time I'm pushing the next version of these patches, just to make sure we progress as fast as we can on this.
Finally, please take a look at adding a test for this, with some sample files. See test/py.
However, that will be a bit difficult. In order to be used, the DT overlays require the overlay support in dtc, which is still not included upstream. So the majority of the users of U-Boot won't have that support, meaning that these tests will likely fail for them (even though the code might be correct), resulting in a lot of false negative.
How do you want me to proceed?
Thanks! Maxime
Hi Maxime,
On 10 May 2016 at 05:45, Maxime Ripard maxime.ripard@free-electrons.com wrote:
Hi Simon,
I'm finally taking the time to address the reviews that have been made here.
On Sat, Apr 09, 2016 at 12:40:14PM -0600, Simon Glass wrote:
Hi Maxime,
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@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.
Signed-off-by: Maxime Ripard maxime.ripard@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
I'm happy to take this into the U-Boot tree while you try to get it applied upstream, but please confirm that you will do this and by when. I suggest you sent an email referring to this patch.
Also fdt_overlay.c should go in lib/libfdt/. If this functionality is rejected for libfdt (as was fdtgrep) then we'll move it to lib/.
I'm definitely ok to send it to libfdt if that's what you meant by upstream. I'll push it at the same time I'm pushing the next version of these patches, just to make sure we progress as fast as we can on this.
OK great.
Finally, please take a look at adding a test for this, with some sample files. See test/py.
However, that will be a bit difficult. In order to be used, the DT overlays require the overlay support in dtc, which is still not included upstream. So the majority of the users of U-Boot won't have that support, meaning that these tests will likely fail for them (even though the code might be correct), resulting in a lot of false negative.
How do you want me to proceed?
Well I suppose you can write the test, but comment it out for now. We can still try it with the out-of-tree dtc.
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Regards, Simon
Hi Maxime,
On May 10, 2016, at 14:45 , Maxime Ripard maxime.ripard@free-electrons.com wrote:
[snip]
How do you want me to proceed?
Thanks! Maxime
-- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
FYI an updated dtc patch has been sent. Hopefully this time will get in.
Regards
— Pantelis
Hi Maxime,
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@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.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Will there be a new version of this patch?
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
Regards, Simon
Hi Simon,
On Sun, May 01, 2016 at 12:55:07PM -0600, Simon Glass wrote:
Hi Maxime,
On 4 April 2016 at 12:25, Maxime Ripard maxime.ripard@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.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
Will there be a new version of this patch?
Yeah, sorry, I haven't had the time to address all the comments you made, but there will definitely be a second version addressing the comments Pantelis, Rob, Stefan and you made.
Probably not before a couple of weeks though :/
Maxime
participants (5)
-
Maxime Ripard -
Pantelis Antoniou -
Rob Herring -
Simon Glass -
Tom Rini