Re: [PATCH 1/2] fdt: Allow stacked overlays phandle references

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

 




On 07/24/17 13:51, Phil Elwell wrote:
> On 24/07/2017 19:06, Frank Rowand wrote:
>> On 07/14/17 00:21, Pantelis Antoniou wrote:
>>
>> Keeping in mind that this thread was originally about libfdt,
>> not the Linux kernel, I am mostly talking about the Linux
>> kernel implementation in this email.
>>
>>
>>> Hi Frank,
>>>
>>> On Thu, 2017-07-13 at 14:40 -0700, Frank Rowand wrote:
>>>> On 07/13/17 14:22, Phil Elwell wrote:
>>>>> On 13/07/2017 21:07, Frank Rowand wrote:
>>>>>> On 07/13/17 12:38, Phil Elwell wrote:
>>>>>>
>>>
>>> [snip]
>>>
>>>>> hope an inability to solve the problem posed by this advanced usage won't
>>>>> prevent a solution to a simpler problem from being accepted.
>>>
>>> I have waited until people started commenting on this patchset before
>>> replying.
>>>
>>> I think we agree on a few things to keep the discussion moving forward.
>>>
>>> 1. Stacked overlays are useful and make overlays easier to use.
>>
>> Stacked overlays are required to handle an add-on board that
>> contains physical connectors to plug in yet more things.
>>
>> I'm not sure what you mean when you say they "make overlays
>> easier to use".  Can you elaborate on that a little bit?
>>
>>
>>> 2. Changing the overlay symbols format now would be unwise.
>>
>> I strongly disagree.  I would say that it is desirable to maintain
>> the current overlay format (not just __symbols__), and that there
>> will be pain (for bootloaders???) if the format changes.  But
>> the Linux implementation is not locked in if there is a good
>> reason to change the format.
> 
> And I gently disagree. Provided changes are made in a way that permits
> overlays written in the old style to be distinguished unambiguously then
> a new format may be the best way to tackle all of the new requirements.
> 
>>> 3. A number of extensions have been put forward/requested.
>>>
>>> 3.1. There should be a method to place a symbol on a node that didn't
>>> have one originally (due to vendor supplying broken DTB or being
>>> generated by firmware at runtime).
>>
>> You saw my reaction of what to do about a broken vendor DTB in that
>> thread.  I do not think this method is a good idea.
>>
>> I don't know why a DTB generated by firmware would be missing a symbol.
>> Was that discussed in that thread, and I'm just forgetting it?
>>
>>
>>> 3.2. Scoping symbol visibility in case of clashes.
>>
>> Yes.  This especially makes sense when a board has
>> multiple identical connectors, and the add-on board
>> should not have to specify different symbols based
>> on which connector it is plugged in to.
>>
>>
>>> This can the ability
>>
>> This can add the ability?
>>
>>
>>> to put multiple path references to a single label/symbol. i.e.
>>> foo = "/path/bar", "/path/bar/baz";
>>> Resolving the ambiguity would require the caller to provide it's
>>> 'location' on the tree. I.e. a device under /path/bar/baz would resolve
>>> to the latter. It is a big change semantically.
>>
>> That is one possible implementation.
>>
>> It would require changes to dtc.
>>
>> For a simple example:
>>
>> / {
>>     target: node@0 {
>>     };
>>     node@1 {
>>         target: subnode@0 {
>>         };
>>     };
>> };
>>
>> The current dtc will detect an error:
>>   <stdout>: ERROR (duplicate_label): Duplicate label 'target' on /node@1/subnode@0 and /node@0
>>
>> To allow the same label at different scopes would lose the
>> ability to detect this type of error.  I think this kind of
>> error detection is critical since we rely so heavily on
>> including a number of dtsi files into a dts file.
>>


========================================================
I think I went off the rails a little bit starting here:


>> Another possible implementation would be for the kernel to
>> associate the contents of the __symbols__ node with the
>> nodes added by the overlay that contained it.  Those
>> symbols would then be visible to a subsequent overlay
>> fragment that had a target that is either (1) the same
>> target as the first overlay or (2) a child of the target
>> of the first overlay, or (3) a descendant of the target
>> of the first overlay.  I haven't thought through the
>> implications of allowing (1) vs (2) vs (3).  For
>> instance, if the connector format was to have a connector
>> node that contained a child node which is where the
>> add-on board was loaded, then the second overlay target
>> would be that child node, and policy (3) would make sense.
>>
>> This would work with a single fragment in an overlay.  If
>> there are multiple fragments in an overlay, maybe the
>> symbols could be split apart by fragment (since the
>> value of the properties in __symbols__ start with
>> "/fragment@XXX/__overlay__/...").  I need to think about
>> the implications of that a bit more.
>>
>> This method also seems like it would work well with the
>> connector / plug architecture.
>>
>> There is another use case where maybe the concept of
>> overlay symbol scoping would cause problems.  And that
>> is the beaglebone architecture, where the add-on board
>> connector does not contain just a "bus" or other I/O
>> interface, but exposes much of the SOC pins.  In that
>> case it might make more sense if overlay symbols were
>> global.
>>
>> I'm sure there are other ways to implement scoping.
>> Suggestions are welcome.

and this is the end of me going off the rails
========================================================

What I should have done at this point in my previous reply was
to go back and look at what we had proposed in the context of
connectors, which deals with this whole issue by providing an
explicit mapping of resources that would be used by an add-on
board that is plugged into a connector.  There can be multiple
physical connectors with the same functionality, which would
all look identical to the node that is plugged in.  So if
you had two identical connectors (eg grove connectors), and
two identical add-on boards that you wanted to plug in, you
could use the same exact overlay (.dtb or .dtbo) for the two
add-on boards (the "add an overlay" request to the overlay
loader would explicitly request a target instead of relying
on a target property located in the overlay), and the mappings
in the two connectors would provide the symbol fixup information
that the overlay loader needs.  So the symbol scope problem
is solved by an explicit map in the connector's dts source
instead of implicitly by dtc providing the scoping.

The connector discussion (which did not come to a final
resolution, but has a lot of good content) is at:

   https://lkml.org/lkml/2016/7/18/332


> If a label is considered to be local to the scope of the containing
> node and its children, is it necessary to permit the same label to be
> redefined while a previous definition is in scope?
> 
> To modify your example, consider:
> 
> / {
>     target0: node@0 {
>         subtarget: subnode@0 {
>         };
>     };
>     target1: node@1 {
>         subtarget: subnode@0 {
>         };
>     };
> };
> 
> If duplicate symbols were restricted in this way then it would still
> be possible to detect many cases of accidental symbol re-use without
> removing much (any?) useful functionality.

Many cases, but not all cases.  The connector approach avoids this
problem totally.

>>> Do you have anything else?
>>
>> There is the issue of tracking what a loaded overlay
>> is dependent upon.  At the moment this is avoided by
>> unloading overlays in the reverse order that they
>> were added.  But it would be nice to be able to
>> unload independent overlays in a different order.
>> That is not something that has to be handled in
>> the first implementation, but it is something to
>> keep in mind.
>>
>> Nothing else at the moment about exposing overlay
>> symbols.  I'll keep thinking.
> 
> Although not a symbol issue, I'd like to repeat my request for
> a way for an overlay to delete properties (necessary for boolean
> properties) and perhaps nodes, so that it can be considered
> during any redesign.

Yes, not a symbol issue.  :-)


> Regards,
> 
> Phil
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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