Hi Ben,
> On May 14, 2015, at 09:46 , Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2015-05-14 at 09:23 +0300, Pantelis Antoniou wrote:
>
>>> A few things that I don't find in the overlay code (but maybe I haven't
>>> looked at it hard enough):
>>>
>>> - Can it remove nodes/properties ?
>>>
>>
>> Yes.
>
> Ok, I've missed that when looking at the overlay code then, I'll have to
> give it a closer look.
>
Ok, let me be more specific. It used to be able to do it ;)
The problem was the format used (a ‘-‘ prefix to the name).
Since I didn’t have clear use for it, I was asked to drop it by Grant.
The removal capability is of-course there for the revert case.
>>> - Can it "commit" a changeset so it's permanently part of the main DT ?
>>> We will never have a concept of "revertable" changesets, if we need a
>>> subsequent update, we will get a new overlay from FW that will remove
>>> what needs to be removed and add what needs to be added.
>>>
>>
>> The overlay when applied is a part of the kernel DT tree.
>> It is trivial to add a mechanism that simply commits everything and
>> tosses away the revert information.
>>
>> Note that in that case you have to make provisions for the unflatten
>> blob to not be freed or for the device tree nodes/properties to be
>> dynamically allocated.
>
> I think it makes sense to do the dynamic thing anyway...
>
>>> IE, our current mechanism without overlay is fairly simple:
>>>
>>> - On PCI unplug, we remove all nodes below the slot (from linux),
>>> the FW does the equivalent internally.
>>>
>>
>> If you use an overlay, you just revert it and everything would
>> be as it was before, without anything hanging below the slot node.
>
> Except that doesn't work for the boot time content which we get
> from the firmware as part of the initial FDT (and we can't change that
> without breaking backward compatibility).
>
OK
>> Note that the ‘remove all nodes below the slot’ does not work for my case.
>>
>> That is because there are devices being instantiated under the slot
>> (i2c busses, i2c devices, FPGAs etc) that need to be removed from the
>> system.
>
> Right while in my case, there isn't, it's just the standard OF PCI
> representation generated by FW, the main thing is that it might have
> some enriched properties for some known cable cards of external drawers
> that are good to have.
>
I see.
>>> - On PCI re-plug, the FW internally builds new nodes and sends a
>>> new subtree as an FDT that we can expand/attach.
>>>
>>
>> You can easily send a DT blob containing an overlay from firmware.
>
> Sending one is easy. Building it is not :-)
>
Heh, true ;)
>> It can be even easy, since you might not have to recreate the full blob
>> each time, but instead using flat device tree methods to populate the
>> few properties that change each time.
>
> No, we basically have our internal tree in the firmware in a format
> similar to Linux, ie, a pointer based tree. We can "flatten" it of
> course, but generating an overlay is trickier. We can, it's just more
> work and we are running out of time (I basically have to cut that FW in
> the next few days, then we'll be stuck with whatever interfaces we
> created, I have a big of time to fix bugs after that but that's about
> it).
>
Hmm, since you just want to transmit a whole subtree things are a bit
simpler.
You don’t need any of the fixups, and your target node is known.
So your overlay is simply:
/ {
fragment@0 {
target-path = “/foo”;
__overlay__ {
/* contents of the slot */
};
};
};
I think it’s possible to just bit-mangle a blob (in pseudo code).
const u8 template_overlay_blob[] = { <compiled blob of the above> };
flatten_slot(slot_blob);
overlay_blob = allocate_new_blob(template_overlay_blob, slot_blob);
overlay_node = find_node(overlay_blob, “/fragment@0/__overlay__);
target_prop = find_prop(overlay_blob, “/fragment@0/target-path”);
inject_slot_blob(overlay_blob, overlay_node, slot_blob);
modify_slot_target(overlay_blob, target_prop, slot_target);
I don’t think you need to re-flatten anything, shuffling bits around with
memmove should work.
>>> Now we could consider that subtree as a changeset that can be undone,
>>> but that wouldn't work for boot time. And subsequent updates wouldn't
>>> have that concept of "undoing" anyway.
>>>
>>
>> I have posted another patch that does boot-time DT quirk which are
>> non-revertable.
>>
>> https://lkml.org/lkml/2015/2/18/258
>
> Not sure how that applies in my case ... I can't change the
> representation of the PCI subtree, this is standard OFW representation,
> I can't change the FW to make it an overlay-like thing at boot time,
> that would break existing kernels.
>
The idea is to append the ‘quirk’ to the already booting device tree blob.
Another idea floating around was to simple concatenate the booting blob with
any overlay blobs you want applied at boot time.
>>> IE. conceptually, what overlays do today is quite rooted around the idea
>>> of having a fixed "base" DT and some pre-compiled DTB overlays that
>>> get added/removed. The design completely ignore the idea of a FW that
>>> maintains a "live" tree which we want to keep in sync, which is what we
>>> want to do here, or what we could do with a "live" open firmware
>>> implementation.
>>>
>>> Now we might be able to reconcile them, but it feels to me that the
>>> overlay/changeset stuff is too rooted in the first concept…
>>>
>>
>> The first DT overlays use case (beaglebone capes) is what got the concept
>> started.
>>
>> Right now is a generic mechanism to apply modifications to the kernel
>> live tree, with the possibility to revert them.
>
> Yes but as I said it's not really thought in term of keeping the kernel
> tree in sync with an external dynamically generated tree. Maybe we can
> fix it, but it's more complex…
>
Yes it is, unfortunately.
> Ben.
>
>>> Ben.
>>>
>>>
>>
>> Regards
>>
>> — Pantelis
>
>
Regards
— Pantelis
--
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
