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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Tue, Jul 12, 2016 at 04:07:49PM +0100, Phil Elwell wrote:
> On Tue, Jul 12, 2016 at 3:34 PM, David Gibson
> <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > On Mon, Jul 11, 2016 at 09:20:44PM +0100, Phil Elwell wrote:
> >> On 11/07/2016 20:56, Maxime Ripard wrote:
> > [snip]
> >
> >> > +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;
> >
> >> 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).

Ok, if you want that feature, let's invent a mechanism specifically
for conditionally disabling chunks, rather than using a side-effect
which will break horribly as soon as we extend the overlay format.

Alternatively you could just use fdt_del_node() or fdt_nop_node() to
remove the fragments you don't wish to apply.

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



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux