Hi Pantelis,
On Fri, Jun 10, 2016 at 05:28:11PM +0300, Pantelis Antoniou wrote:
Hi Maxime,
On May 27, 2016, at 12:13 , 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, the Raspberry Pi or the CHIP).
Add a new function to merge overlays with a base device tree.
Signed-off-by: Maxime Ripard maxime.ripard@free-electrons.com
include/libfdt.h | 30 ++++ lib/libfdt/Makefile | 2 +- lib/libfdt/fdt_overlay.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 lib/libfdt/fdt_overlay.c
diff --git a/include/libfdt.h b/include/libfdt.h index 1e01b2c7ebdf..783067e841a1 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -1698,6 +1698,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name); */ int fdt_del_node(void *fdt, int nodeoffset);
+/**
- fdt_overlay_apply - Applies a DT overlay on a base DT
- @fdt: pointer to the base device tree blob
- @fdto: pointer to the device tree overlay blob
- fdt_overlay_apply() will apply the given device tree overlay on the
- given base device tree.
- Expect the base device tree to be modified, even if the function
- returns an error.
If the base tree has been modified on an error it is unsafe to use it for booting. A valid strategy would be to scribble over the DT magic number so that that blob is invalidated.
What are the other people’s opinion on this?
That would probably be safer yes, I'll change that.
+static int fdt_overlay_get_target(const void *fdt, const void *fdto,
int fragment)+{
- uint32_t phandle;
- const char *path;
- /* Try first to do a phandle based lookup */
- phandle = fdt_overlay_get_target_phandle(fdto, fragment);
- if (phandle)
return fdt_node_offset_by_phandle(fdt, phandle);- /* And then a path based lookup */
- path = fdt_getprop(fdto, fragment, "target-path", NULL);
- if (!path)
return -FDT_ERR_NOTFOUND;- return fdt_path_offset(fdt, path);
+}
Those targets are fine; beware there are patches with more target options.
Ack.
+static int fdt_overlay_merge(void *dt, void *dto) +{
- int root, fragment;
- root = fdt_path_offset(dto, "/");
- if (root < 0)
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;This is incorrect. The use of “fragment” is a convention only. The real test whether the node is an overlay fragment is that it contains a target property.
target = fdt_overlay_get_target(dt, dto, fragment);if (target < 0)return target;So you could do ‘if (target < 0) continue;’ or handle a more complex error code.
Ok, will change.
I would caution against the liberal use of malloc in libfdt. We’re possibly running in a constrained environment; a custom extents based (non freeing) allocator should be better.
David had the same comments, and I will drop the mallocs entirely.
We need some figures about memory consumption when this is enabled, and a CONFIG option to disable it.
The current code (before and after that patch) adds 18kB to a sandbox_defconfig.
Maxime