Re: [PATCH v2 0/3] add fwnode support to reset subsystem

From: Clément Léger
Date: Wed Apr 06 2022 - 07:19:45 EST

Le Tue, 5 Apr 2022 12:11:51 -0500,
Rob Herring <robh@xxxxxxxxxx> a écrit :

> On Tue, Apr 5, 2022 at 10:52 AM Clément Léger <clement.leger@xxxxxxxxxxx> wrote:
> >
> > Le Tue, 5 Apr 2022 09:47:20 -0500,
> > Rob Herring <robh@xxxxxxxxxx> a écrit :
> >


> >
> > I also tried loading an overlay from a driver on an ACPI based system.
> > Their patch is (I guess) targeting the specific problem that there is
> > no base DT when using ACPI. However, Mark Brown feedback was not to
> > mix OF and ACPI:
> I agree there. I don't think we should use DT bindings in ACPI tables
> which is already happening. In this case, I think what's described by
> ACPI and DT must be completely disjoint. I think that's the case here
> as everything is downstream of the PCIe device.

Yes, there is no references to the host devices (at least in my case).

> > "That seems like it's opening a can of worms that might be best left
> > closed."
> >
> > But I would be interested to know how the Xilinx guys are doing that
> > on x86/ACPI based system.
> They aren't, yet...



> > > 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.

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 ?

> We already support matching a PCI device to a DT node. The PCI
> subsystem checks if there is a corresponding DT node for each PCI
> device created and sets the of_node pointer if there is. For
> OpenFirmware systems (PPC), there always is a node. For FDT, we
> generally don't have a node unless there are additional
> non-discoverable properties. Hikey960 is an example with PCI device
> nodes in the DT as it has a soldered down PCIe switch with downstream
> devices and non-discoverable properties (e.g. reset GPIO for each
> port).
> > Or probably load
> > some dtbo from the FS. If so, I would then have to describe the card
> > for both ACPI and DT. How is that better than using a single software
> > node description for both ACPI/OF based systems ? Or maybe I missed
> > something, but the device description won't come out of thin air I
> > guess.
> What you would have to load is a DT overlay describing all your
> downstream devices.
> We support DTBs (including DTBOs) built into the kernel already, so
> whether it's built into the kernel or in the FS is up to you really.


> > Also, when saying "That may be needed for both DT and ACPI systems", do
> > you actually meant that ACPI overlay should be described for ACPI based
> > systems and DT overlays for DT based ones ?
> No, as I said: "I think DT overlays is the right (or only) solution
> here." ACPI overlays doesn't seem like a workable solution because it
> can't describe your downstream devices.

Ok, so you are actually really suggesting to use OF overlays on ACPI
based systems. If so, I'm ok with that.

> The reason generating nodes may be needed on DT systems as well is
> that all PCI devices are not described in DT systems either.
> > If so, some subsystems do
> > not even support ACPI (reset for instance which is need for my
> > PCI card but that is not the only one). So how to accomodate both ? This
> > would result in having 2 separate descriptions for ACPI and OF and
> > potentially non working with ACPI description.
> >
> > Software nodes have the advantage of being independent from the
> > description systems used (ACPI/OF). If switching susbsystems to use
> > fwnode, this would also allows to accomodate easily for all nodes types
> > and potentially factorize some code.
> It's not independent. You are effectively creating the DT nodes with C
> code. Are these not DT bindings:
> > static const struct property_entry ddr_clk_props[] = {
> > PROPERTY_ENTRY_U32("clock-frequency", 30000000),
> > PROPERTY_ENTRY_U32("#clock-cells", 0),
> > {}
> > };
> Sure looks like DT bindings to me. I don't think moving them into the
> kernel as sw nodes avoids any of the potential pitfalls of mixing ACPI
> and DT. For example, what happens when you have a downstream sw node
> device that wants to do DMA allocations and transfers? I suspect that
> sw nodes can't really handle more than trivial cases.

When integrating with fwnode, the subsystem support will be almost the
same as the one with OF. But I agree that it requires certain level of
subsystem modifications to support fwnode.

> > > That could work either by the PCI subsystem creating nodes as it
> > > populates devices or your driver could make a request to populate nodes
> > > for its hierarchy. That's not a hard problem to solve. That's what
> > > OpenFirmware implementations do already.
> >
> > This would also require to get address translation working with ACPI
> > based systems since the PCI bus isn't described with DT on such
> > systems. I'm not sure how trivial it is. Or it would require to add PCI
> > root complex entries into the device-tree to allow adress translation
> > to work using the existing system probably.
> It would require all that most likely. Maybe there's some shortcuts we
> can take. All the necessary information is maintained by the kernel
> already. Normally it's populated from the firmware into the kernel
> structures. But here we need the opposite direction.
> > >
> >
> > Looking at the feedback of the previous series that I mentionned,
> > absolutely nobody agreed on the solution to be adopted. I asked for a
> > consensus but I only got an answer from Hans de Goede which was ok
> > with the fwnode way. I would be really glad to have some consensus on
> > that in order to implement a final solution (and if the OF overlays is
> > the one to be used, I'll use it).
> Yes, that's a challenge, but buried in some patch series is not going
> to get you there.

Sorry, indeed, you were not on the series were the discussion took
place. I'll think about that next time.

> I am trying to widen the discussion because it is a
> problem that's been on my radar for some time.

Thanks for the proposal, maybe we can achieve something that will suit
everybody and solve the current problems.

Clément Léger,
Embedded Linux and Kernel engineer at Bootlin