Re: [PATCH v2 6/6] libfdt: Add overlay application function

On Tue, Jul 12, 2016 at 3:34 PM, David Gibson
[off-list ref] wrote:

On Mon, Jul 11, 2016 at 09:20:44PM +0100, Phil Elwell wrote:

quoted

On 11/07/2016 20:56, Maxime Ripard wrote:

[snip]

quoted

quoted

+static int overlay_merge(void *fdt, void *fdto)
+{
+   int fragment;
+
+   fdt_for_each_subnode(fragment, fdto, 0) {
+           int overlay;
+           int target;
+           int ret;
+
+           target = overlay_get_target(fdt, fdto, fragment);
+           if (target < 0)
+                   continue;
+
+           overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
+           if (overlay < 0)
+                   return overlay;

quoted

Why does the absence of a target cause a fragment to be ignored but
the absence of an "__overlay__" property cause the merging to be
abandoned with an error? Can't we just ignore fragments that aren't
recognised?

So, I had the same question.  But fragments we can't make sense MUST
cause failures, and not be silently ignored.

An incompletely applied overlay is almost certainly going to cause you
horrible grief at some point, so you absolutely want to know early if
your overlay is in a format your tool doesn't understand.

Cards on the table time - at Raspberry Pi we've found it convenient to
selectively enable or disable fragments within an overlay based on
user-supplied parameters. The way I've implemented this is by using
"__dormant__" as an antonym to "__overlay__" (partly chosen because it
is the same length), and then pre-processing the overlay before
applying it. This scheme works with the kernel overlay support and our
firmware loader (obviously).

Although I could invent a similar scheme that uses antonyms of
"target" and "target-path", I don't see why this is inherently safer.

Phil (phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org)