Le Wed, 6 Apr 2022 08:19:16 -0500, Rob Herring <robh@xxxxxxxxxx> a écrit : > > > > > > > > > > > > > I've told the Xilinx folks the same thing, but I would separate this > > > > > into 2 parts. First is just h/w work in a DT based system. Second is > > > > > creating a base tree an overlay can be applied to. The first part should > > > > > be pretty straightforward. We already have PCI bus bindings. The only > > > > > tricky part is getting address translation working from leaf device thru > > > > > the PCI bus to host bus, but support for that should all be in place > > > > > (given we support ISA buses off of PCI bus). The second part will > > > > > require generating PCI DT nodes at runtime. That may be needed for both > > > > > DT and ACPI systems as we don't always describe all the PCI hierarchy > > > > > in DT. > > > > > > > > But then, if the driver generate the nodes, it will most probably > > > > have to describe the nodes by hardcoding them right ? > > > > > > No, the kernel already maintains its own tree of devices. You just > > > need to use that to generate the tree. That's really not much more > > > than nodes with a 'reg' property encoding the device and function > > > numbers. > > > > Just to clarified a point, my PCI device exposes multiple peripherals > > behind one single PCI function. > > Right. I would expect your PCI device DT node to have a 'simple-bus' > child node with all those peripherals. And maybe there's other nodes > like fixed-clocks, etc. > > > To be sure I understood what you are suggesting, you propose to create > > a DT node from the PCI driver that has been probed dynamically > > matching this same PCI device with a 'reg' property. I also think > > this would requires to generate some 'pci-ranges' to remap the > > downstream devices that are described in the DTBO, finally, load the > > overlay to be apply under this newly created node. Is that right ? > > Right. You'll need to take the BAR address(es) for the device and stick > those into 'ranges' to translate offsets to BAR+offset. Hi Rob, I got something working (address translation, probing and so on) using what you started. I switch to using changeset however, I'm not sure that it make sense for property creation since the node has not yet been added to the tree. Attaching the node with changeset however seems to make sense. But I'm no expert here, so any advise is welcome. Based on what we said, I created a PCI driver which uses a builtin overlay. In order to be able to apply the overlay on the correct PCI node -the one on which the card was plugged) and thus be totally plug and play, the 'target-path' property is patched using direct fdt function and replaced the target with the PCI device node path. I don't see any other way to do that before applying the overlay since of_overlay_fdt_apply() takes a fdt blob as input. The driver also insert correct ranges into the PCI device in order to translate the downstream node addresses to BAR addresses. It seems reasonnable to assume that this depends on the driver and thus should not be done by the PCI of core at all. Finally, the driver probes the newly added childs using of_platform_populate(). With all of that, the address translation and the probing works correctly and the platform devices are created. There is still a few things to fix such as the following: [ 2830.324773] OF: overlay: WARNING: memory leak will occur if overlay removed, property: /pci/pci@2,6/dev@0,0/compatible But it seems like this is something that works and would allow to support various use cases. From what I see, it should also work on other platforms. Major advantage of that over fwnode is that the changes are pretty small and relatively contained. However, one point that might be a bit of a problem is enabling CONFIG_OF on x86 for instance. While it seems to work, is there any potential concerns about this ? Moreover, ideally, I would want the driver to "select OF" since without that, the driver won't be visible. Or should it "depends on OF" but thus, it would be almost mandatory to enable CONFIG_OF on x86 kernels if we want to support this driver without the need to recompile a kernel. Thanks, -- Clément Léger, Embedded Linux and Kernel engineer at Bootlin https://bootlin.com