Re: [PATCH v4 19/21] drivers/of: Support adding sub-tree

Hi Ben,

On May 2, 2015, at 01:57 , Benjamin Herrenschmidt =

[off-list ref] wrote:

=20
On Fri, 2015-05-01 at 13:46 -0500, Rob Herring wrote:

quoted

On Fri, May 1, 2015 at 10:22 AM, Benjamin Herrenschmidt
[off-list ref] wrote:

quoted

On Fri, 2015-05-01 at 07:54 -0500, Rob Herring wrote:
=20

quoted

The difference seems to be whether you allocate space or just point =

to

quoted

quoted

quoted

the FDT for various strings/data. Is that right?
=20

quoted

  * of_fdt_add_subtree() is the introduced API to do the work.

=20
Have you looked at overlays and if so why do they not work for your =

purposes?

quoted

quoted

quoted

=20
Why do you need to do this with the flattened tree?

=20
The basic idea I asked Gavin to implement is that since the FW needs =

to

quoted

quoted

provide a bunch of DT updates to Linux at runtime in the form of new
nodes below an existing one, rather than doing it via some =

new/custom

quoted

quoted

format, instead, have it send a bit of FDT blob to expand under an
existing node.

=20
Overlay =3D an FDT blob to graft into a live running system. Sounds =

like

quoted

the same thing.
=20

quoted

As for the details of Gavin implementation, I haven't looked at it =

in

quoted

quoted

details yet so there might be issues there, however I don't know =

what

quoted

quoted

you mean by "overlays", any pointer ?

=20
CONFIG_OF_OVERLAY
=20
=

http://events.linuxfoundation.org/sites/events/files/slides/dynamic-dt-key=
note-v3.pdf

=20
Well, that looks horrendously complicated, poorly documented and =

totally

unused in-tree outside of the unittest stuff, yay ! It has all sort of
"features" that I don't really care about.
=20

If it was easy to get stuff in, it would get more of the real-use =
drivers
in.

I still don't see what it buys me other than making my FW a lot more
complex having to generate all that additional fixup etc... crap that =

I

don't totally get yet.
=20

You don=E2=80=99t generate any additional fixups. You just compile with =
the option
that generates all the fixups for you.

What's wrong with just unflattening the nodes in place ? The DT comes
from the FW in the first place so all the phandles are already good in
the new added blob. Internally, the FW created new nodes in its =

internal

representation and flattened the subtree and sends that subtree to
Linux.
=20
I don't plan to play "revert" either, if you unplug, I do need to =

remove

what's under the slot but that's true of boot time devices, not just
"new" ones, so the overlay stuff won't do the trick and I certainly
don't want to keep track=E2=80=A6
=20

You get all of the corner cases handled for free. Perhaps it works for =
your
case too.

Perhaps you can educate me on what you need supported and we can make =
sure
it=E2=80=99s included.

Ben.
=20
=20

Regards

=E2=80=94 Pantelis

On Mon, 2015-05-04 at 19:41 +0300, Pantelis Antoniou wrote:

You get all of the corner cases handled for free. Perhaps it works for
your case too.

Perhaps you can educate me on what you need supported and we can make
sure it’s included.

Which corner cases ?

IE, what I want is simply "update" the device-tree below a PCIe slot on
PCI hotplug.

The DT isn't "compiled" from a dts (it's amazing how many people seem to
believe this is the only way you get fdt's nowadays). It's dynamically
(ie programatically) generated by firmware at boot time and contains
whatever PCIe devices happen to be plugged during boot.

When doing PCIe hotplug operations, the kernel does various FW calls
(among others to control slot power), and during these, the FW re-probes
underneath the slot and refreshes its internal representation. So the
phandles remain fully consistent, there is no fixup needed.

We want the kernel to also update his copy as wee in order to avoid
keeping stale nodes that don't match what's there anymore. Also, when
plugging specific kind of IO drawers, the FW can provide additional node
and properties that will be used to control slots inside the drawers.

So what we need is:

  - On PCIe unplug, remove all old nodes below the slot
  - On PCIe plug, get all the new nodes from FW

Note that there is no need to do anything like platform device probing
etc... the PCI layer takes care of that, we will remove the old nodes
after the pci_dev are gone and create the new ones before Linux
re-probes the PCIe bus subtree.

So what we need is very simple: The removal can be handled without FW
help, and the plug case is a matter of just transferring all those new
nodes to Linux to re-expand.

Since the phandle etc... are all consistent with the original tree,
there is no fixups required.

So the "trivial" way to do it (and the way we have implemented the FW
side so far) is to have the FW simply "flatten" the subtree below the
slot and pass it to Linux, with the intent of expanding it back below
the slot node.

This is what Gavin proposed patches do.

The overlay mechanism adds all sorts of features that we don't seen to
need and would make the above more complex.

Cheers,
Ben.

On Tue, 2015-05-05 at 07:14 +1000, Benjamin Herrenschmidt wrote:

So the "trivial" way to do it (and the way we have implemented the FW
side so far) is to have the FW simply "flatten" the subtree below the
slot and pass it to Linux, with the intent of expanding it back below
the slot node.

This is what Gavin proposed patches do.

The overlay mechanism adds all sorts of features that we don't seen to
need and would make the above more complex.

Guys, I never got a final answer from you on this. Are we ok with adding
the way to just expand a subtree or are you insistent we need to use the
overlap mechanism ?

Cheers,
Ben.