Re: [PATCH 2/4] of: DT quirks infrastructure
From: Frank Rowand
Date: Thu Feb 19 2015 - 11:51:25 EST
On 2/19/2015 8:40 AM, Frank Rowand wrote:
> On 2/19/2015 6:41 AM, Pantelis Antoniou wrote:
>> Hi Frank,
>>> On Feb 19, 2015, at 04:08 , Frank Rowand <frowand.list@xxxxxxxxx> wrote:
>>> On 2/18/2015 6:59 AM, Pantelis Antoniou wrote:
>>>> Implement a method of applying DT quirks early in the boot sequence.
>>>> A DT quirk is a subtree of the boot DT that can be applied to
>>>> a target in the base DT resulting in a modification of the live
>>>> tree. The format of the quirk nodes is that of a device tree overlay.
>>> The use of the word "quirk" is a different mental model for me than what
>>> this patch series appears to be addressing. I would suggest totally
>>> removing the word "quirk" from this proposal to avoid confusing the
>>> mental models of future generations of kernel folks.
>> Naming things is hard to do. Suggestions?
> You are inviting me to bikeshed. I'll leave that to you.
>>> What this patch series seems to be proposing is a method to apply DT
>>> overlays as soon as unflatten_device_tree() completes. In other words,
>>> making the device tree a dynamic object, that is partially defined by
>>> the kernel during boot. Well, to be fair, the kernel chooses among
>>> several possible alternatives encoded in the DT blob. So the device
>>> tree is no longer a static object that describes the hardware of the
>>> system. It may not sound like a big deal, but it seems to me to be
>>> a fundamental shift in what the device tree blob is. Something that
>>> should be thought about carefully and not just applied as a patch to
>>> solve a point problem.
>> There is a fundamental shift going on about what hardware is. It is nowhere
>> as static as it used to be. It is time for the kernel to keep up.
> Run time overlays do that.
> The problem you seem to be dealing with here is that you want a single
> DT blob that can be installed on many different _physical_ boards.
>>> The stated use of this proposal is to create dynamic DT blobs that can
>>> describe many similar variants of a given system instead of creating
>>> unique DT blobs for each different system.
>>> I obviously have not thought through the architectural implications yet,
>>> but just a quick example. One of the issues we have been trying to fix
>>> is device tree validation. The not yet existent (except as a few proof
>>> of concept attempts) validator would need to validate a device tree
>>> for each dynamic variant. Probably not a big deal, but an example of
>>> the ripple effects this conceptual change implies.
>> I don’t see what the big problem with the validator is. The ‘quirk’
>> are easily identified by the presence of the __overlay__ nodes and
>> the validator can apply each overlay and perform the validation check
>> at each resultant tree.
> I said "not a big deal". I was trying to make people think about the
> bigger picture. Defending that this is a non-issue for the validator
> is totally missing my point. Step back and think architecturally
> about the big picture. I do _not_ know if this is a problem, but
> they will be ripples from this proposal.
oops: there will be ripples from this proposal.
>>> A second function that this patch is proposing is a method to enable
>>> or disable devices via command line options. If I understand
>>> correctly, this is meant to solve a problem with run time overlays
>>> that require disabling a device previously enabled by the DT blob.
>>> If so, it seems like it could easily be implemented in a simpler
>>> generic way than in the board specific code in this patch series.
>> Disabling a device is the most common case, but other options are desired
>> too. For instance changing OPPs by a command line option, etc.
> The device tree is supposed to describe what the hardware is. Why would
> you want a command line option to change what OPPs are possible for the
>>> I share the concerns that Mark Rutland has expressed in his comments
>>> about this series.
>>> < snip >
>>> I have read through the patches and will have comments on the code
>>> later if this proposal is seen as a good idea.
>> — Pantelis
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/