This series is part of a larger series which aims at adding fwnode
support in multiple subsystems [1]. The goal of this series was to
add support for software node in various subsystem but in a first
time only the fwnode support had gained consensus and will be added
to multiple subsystems.
For the moment ACPI node support is excluded from the fwnode support
to avoid creating an unspecified ACPI reset device description. With
these modifications, both driver that uses the fwnode_ API or the of_
API to register the reset controller will be usable by consumer
whatever the type of node that is used.
One question raised by this series is that I'm not sure if all reset
drivers should be modified to use the new fwnode support or keep the
existing device-tree support. Maintainer advice on that particular
question will be welcome.
This series was tested on a sparx5 board (pcb134).
[1] https://lore.kernel.org/netdev/[email protected]/T/
---
Changes in V2:
- Updated comment specifying that either one of fwnode_xlate or of_xlate
should provided and add check to return EINVAL if both are set
- Use only fwnode node name in rcdev_name() which also works for of_node
- Remove of_node handling case in __reset_control_get()
- Handle new error values -EOVERFLOW and -ENOMEM (returned by swnode
implementation) for fwnode_property_match_string()
- Add sparx5 driver fwnode conversion
Clément Léger (3):
of: add function to convert fwnode_reference_args to of_phandle_args
reset: add support for fwnode
reset: mchp: sparx5: set fwnode field of reset controller
drivers/reset/core.c | 100 ++++++++++++++++++-------
drivers/reset/reset-microchip-sparx5.c | 3 +-
include/linux/of.h | 18 +++++
include/linux/reset-controller.h | 20 ++++-
include/linux/reset.h | 14 ++++
5 files changed, 123 insertions(+), 32 deletions(-)
--
2.34.1
On Thu, Mar 24, 2022 at 03:12:34PM +0100, Cl?ment L?ger wrote:
> This series is part of a larger series which aims at adding fwnode
> support in multiple subsystems [1]. The goal of this series was to
> add support for software node in various subsystem but in a first
> time only the fwnode support had gained consensus and will be added
> to multiple subsystems.
The goal is describing a solution. What is the problem?
What's the scenario where you have a reset provider not described by
firmware providing resets to devices (consumers) also not described by
firmware.
> For the moment ACPI node support is excluded from the fwnode support
> to avoid creating an unspecified ACPI reset device description. With
> these modifications, both driver that uses the fwnode_ API or the of_
> API to register the reset controller will be usable by consumer
> whatever the type of node that is used.
Good, because controlling reset lines directly isn't how the ACPI device
model works AFAIK.
> One question raised by this series is that I'm not sure if all reset
> drivers should be modified to use the new fwnode support or keep the
> existing device-tree support. Maintainer advice on that particular
> question will be welcome.
That would be pointless churn IMO. Why do we need to convert drivers
which the vast majority will never use anything but DT?
Rob
Le Mon, 4 Apr 2022 12:41:37 -0500,
Rob Herring <[email protected]> a écrit :
> On Thu, Mar 24, 2022 at 03:12:34PM +0100, Clément Léger wrote:
> > This series is part of a larger series which aims at adding fwnode
> > support in multiple subsystems [1]. The goal of this series was to
> > add support for software node in various subsystem but in a first
> > time only the fwnode support had gained consensus and will be added
> > to multiple subsystems.
>
> The goal is describing a solution. What is the problem?
>
> What's the scenario where you have a reset provider not described by
> firmware providing resets to devices (consumers) also not described by
> firmware.
Hi Rob, there was a link attached to this series since there was a
previous one that was sent which described the problem. Here is a link
to the same thread but to a specific message which clarifies the
problem and the solutions that were mentionned by other maintainers
(ACPI overlays, DT overlays, software nodes and so on):
https://lore.kernel.org/netdev/[email protected]/
>
> > For the moment ACPI node support is excluded from the fwnode support
> > to avoid creating an unspecified ACPI reset device description. With
> > these modifications, both driver that uses the fwnode_ API or the of_
> > API to register the reset controller will be usable by consumer
> > whatever the type of node that is used.
>
> Good, because controlling reset lines directly isn't how the ACPI device
> model works AFAIK.
This was based on Mark Brown feedback.
>
> > One question raised by this series is that I'm not sure if all reset
> > drivers should be modified to use the new fwnode support or keep the
> > existing device-tree support. Maintainer advice on that particular
> > question will be welcome.
>
> That would be pointless churn IMO. Why do we need to convert drivers
> which the vast majority will never use anything but DT?
To have a single interface to maintain and to remove duplicated fields
(of_node, fwnode, fwnode_xlate, of_xlate) from reset controller struct.
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
On Tue, Apr 5, 2022 at 10:52 AM Clément Léger <[email protected]> wrote:
>
> Le Tue, 5 Apr 2022 09:47:20 -0500,
> Rob Herring <[email protected]> a écrit :
>
> > + some Xilinx folks
> >
> > On Tue, Apr 05, 2022 at 09:24:34AM +0200, Clément Léger wrote:
> > > Le Mon, 4 Apr 2022 12:41:37 -0500,
> > > Rob Herring <[email protected]> a écrit :
> > >
> > > > On Thu, Mar 24, 2022 at 03:12:34PM +0100, Clément Léger wrote:
> > > > > This series is part of a larger series which aims at adding fwnode
> > > > > support in multiple subsystems [1]. The goal of this series was to
> > > > > add support for software node in various subsystem but in a first
> > > > > time only the fwnode support had gained consensus and will be added
> > > > > to multiple subsystems.
> > > >
> > > > The goal is describing a solution. What is the problem?
> > > >
> > > > What's the scenario where you have a reset provider not described by
> > > > firmware providing resets to devices (consumers) also not described by
> > > > firmware.
> > >
> > > Hi Rob, there was a link attached to this series since there was a
> > > previous one that was sent which described the problem. Here is a link
> > > to the same thread but to a specific message which clarifies the
> > > problem and the solutions that were mentionned by other maintainers
> > > (ACPI overlays, DT overlays, software nodes and so on):
> > >
> > > https://lore.kernel.org/netdev/[email protected]/
> >
> > Thanks, but your commit message should explain the problem. The problem
> > is not subsystems don't support fwnode.
> >
> > This is the exact same problem the Xilinx folks are trying to solve with
> > their PCIe FPGA cards[1] (and that is not really a v1). They need to
> > describe h/w downstream from a 'discoverable' device. Their case is
> > further complicated with the dynamic nature of FPGAs. It's also not just
> > PCIe. Another usecase is describing downstream devices on USB FTDI
> > serial chips which can have GPIO, I2C, SPI downstream. And then you want
> > to plug in 10 of those.
>
> 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.
> "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 don't think swnodes are going to scale for these usecases. We moved
> > h/w description out of the kernel for a reason. Why are we adding that
> > back in a new form? The complexity for what folks want to describe is
> > only going to increase.
> >
> > I think DT overlays is the right (or only) solution here. Of course the
> > DT maintainer would say that. Actually, I would be happier to not have
> > to support overlays in the kernel.
>
> DT overlay might work on DT based system. If I'm going to plug the card
> on an ACPI based platform (x86), I also want that card to work
> seamlessly without requiring the user to create an ACPI overlay.
I agree, it should work the same way for the user.
> If you proposal was to use DT overlays on an ACPI based system, doing
> so would also require to "plug" the PCI subystem when described with
> ACPI to "probe" DT overlays describing PCI devices, not sure this is
> something trivial and it would be PCI centric.
Yes, this is the 2nd part I describe. I don't think there's any way to
avoid this being bus specific because bus specific nodes have to be
created.
> > 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.
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.
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.
> > 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.
> > https://lore.kernel.org/lkml/[email protected]/
>
> 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. I am trying to widen the discussion because it is a
problem that's been on my radar for some time.
Rob
+ some Xilinx folks
On Tue, Apr 05, 2022 at 09:24:34AM +0200, Cl?ment L?ger wrote:
> Le Mon, 4 Apr 2022 12:41:37 -0500,
> Rob Herring <[email protected]> a ?crit :
>
> > On Thu, Mar 24, 2022 at 03:12:34PM +0100, Cl?ment L?ger wrote:
> > > This series is part of a larger series which aims at adding fwnode
> > > support in multiple subsystems [1]. The goal of this series was to
> > > add support for software node in various subsystem but in a first
> > > time only the fwnode support had gained consensus and will be added
> > > to multiple subsystems.
> >
> > The goal is describing a solution. What is the problem?
> >
> > What's the scenario where you have a reset provider not described by
> > firmware providing resets to devices (consumers) also not described by
> > firmware.
>
> Hi Rob, there was a link attached to this series since there was a
> previous one that was sent which described the problem. Here is a link
> to the same thread but to a specific message which clarifies the
> problem and the solutions that were mentionned by other maintainers
> (ACPI overlays, DT overlays, software nodes and so on):
>
> https://lore.kernel.org/netdev/[email protected]/
Thanks, but your commit message should explain the problem. The problem
is not subsystems don't support fwnode.
This is the exact same problem the Xilinx folks are trying to solve with
their PCIe FPGA cards[1] (and that is not really a v1). They need to
describe h/w downstream from a 'discoverable' device. Their case is
further complicated with the dynamic nature of FPGAs. It's also not just
PCIe. Another usecase is describing downstream devices on USB FTDI
serial chips which can have GPIO, I2C, SPI downstream. And then you want
to plug in 10 of those.
I don't think swnodes are going to scale for these usecases. We moved
h/w description out of the kernel for a reason. Why are we adding that
back in a new form? The complexity for what folks want to describe is
only going to increase.
I think DT overlays is the right (or only) solution here. Of course the
DT maintainer would say that. Actually, I would be happier to not have
to support overlays in the kernel.
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. 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.
Rob
https://lore.kernel.org/lkml/[email protected]/
On Tue, Apr 5, 2022 at 12:11 PM Rob Herring <[email protected]> wrote:
>
> On Tue, Apr 5, 2022 at 10:52 AM Clément Léger <[email protected]> wrote:
> >
> > Le Tue, 5 Apr 2022 09:47:20 -0500,
> > Rob Herring <[email protected]> a écrit :
> >
> > > + some Xilinx folks
> > >
> > > On Tue, Apr 05, 2022 at 09:24:34AM +0200, Clément Léger wrote:
> > > > Le Mon, 4 Apr 2022 12:41:37 -0500,
> > > > Rob Herring <[email protected]> a écrit :
> > > >
> > > > > On Thu, Mar 24, 2022 at 03:12:34PM +0100, Clément Léger wrote:
> > > > > > This series is part of a larger series which aims at adding fwnode
> > > > > > support in multiple subsystems [1]. The goal of this series was to
> > > > > > add support for software node in various subsystem but in a first
> > > > > > time only the fwnode support had gained consensus and will be added
> > > > > > to multiple subsystems.
> > > > >
> > > > > The goal is describing a solution. What is the problem?
> > > > >
> > > > > What's the scenario where you have a reset provider not described by
> > > > > firmware providing resets to devices (consumers) also not described by
> > > > > firmware.
> > > >
> > > > Hi Rob, there was a link attached to this series since there was a
> > > > previous one that was sent which described the problem. Here is a link
> > > > to the same thread but to a specific message which clarifies the
> > > > problem and the solutions that were mentionned by other maintainers
> > > > (ACPI overlays, DT overlays, software nodes and so on):
> > > >
> > > > https://lore.kernel.org/netdev/[email protected]/
> > >
> > > Thanks, but your commit message should explain the problem. The problem
> > > is not subsystems don't support fwnode.
> > >
> > > This is the exact same problem the Xilinx folks are trying to solve with
> > > their PCIe FPGA cards[1] (and that is not really a v1). They need to
> > > describe h/w downstream from a 'discoverable' device. Their case is
> > > further complicated with the dynamic nature of FPGAs. It's also not just
> > > PCIe. Another usecase is describing downstream devices on USB FTDI
> > > serial chips which can have GPIO, I2C, SPI downstream. And then you want
> > > to plug in 10 of those.
> >
> > 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.
>
> > "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 don't think swnodes are going to scale for these usecases. We moved
> > > h/w description out of the kernel for a reason. Why are we adding that
> > > back in a new form? The complexity for what folks want to describe is
> > > only going to increase.
> > >
> > > I think DT overlays is the right (or only) solution here. Of course the
> > > DT maintainer would say that. Actually, I would be happier to not have
> > > to support overlays in the kernel.
> >
> > DT overlay might work on DT based system. If I'm going to plug the card
> > on an ACPI based platform (x86), I also want that card to work
> > seamlessly without requiring the user to create an ACPI overlay.
>
> I agree, it should work the same way for the user.
>
> > If you proposal was to use DT overlays on an ACPI based system, doing
> > so would also require to "plug" the PCI subystem when described with
> > ACPI to "probe" DT overlays describing PCI devices, not sure this is
> > something trivial and it would be PCI centric.
>
> Yes, this is the 2nd part I describe. I don't think there's any way to
> avoid this being bus specific because bus specific nodes have to be
> created.
>
>
> > > 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.
>
> 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).
Here's a quick and dirty implementation creating DT nodes for PCI devices:
git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/pop-pci-nodes
Rob
Le Tue, 5 Apr 2022 09:47:20 -0500,
Rob Herring <[email protected]> a écrit :
> + some Xilinx folks
>
> On Tue, Apr 05, 2022 at 09:24:34AM +0200, Clément Léger wrote:
> > Le Mon, 4 Apr 2022 12:41:37 -0500,
> > Rob Herring <[email protected]> a écrit :
> >
> > > On Thu, Mar 24, 2022 at 03:12:34PM +0100, Clément Léger wrote:
> > > > This series is part of a larger series which aims at adding fwnode
> > > > support in multiple subsystems [1]. The goal of this series was to
> > > > add support for software node in various subsystem but in a first
> > > > time only the fwnode support had gained consensus and will be added
> > > > to multiple subsystems.
> > >
> > > The goal is describing a solution. What is the problem?
> > >
> > > What's the scenario where you have a reset provider not described by
> > > firmware providing resets to devices (consumers) also not described by
> > > firmware.
> >
> > Hi Rob, there was a link attached to this series since there was a
> > previous one that was sent which described the problem. Here is a link
> > to the same thread but to a specific message which clarifies the
> > problem and the solutions that were mentionned by other maintainers
> > (ACPI overlays, DT overlays, software nodes and so on):
> >
> > https://lore.kernel.org/netdev/[email protected]/
>
> Thanks, but your commit message should explain the problem. The problem
> is not subsystems don't support fwnode.
>
> This is the exact same problem the Xilinx folks are trying to solve with
> their PCIe FPGA cards[1] (and that is not really a v1). They need to
> describe h/w downstream from a 'discoverable' device. Their case is
> further complicated with the dynamic nature of FPGAs. It's also not just
> PCIe. Another usecase is describing downstream devices on USB FTDI
> serial chips which can have GPIO, I2C, SPI downstream. And then you want
> to plug in 10 of those.
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:
"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.
>
> I don't think swnodes are going to scale for these usecases. We moved
> h/w description out of the kernel for a reason. Why are we adding that
> back in a new form? The complexity for what folks want to describe is
> only going to increase.
>
> I think DT overlays is the right (or only) solution here. Of course the
> DT maintainer would say that. Actually, I would be happier to not have
> to support overlays in the kernel.
DT overlay might work on DT based system. If I'm going to plug the card
on an ACPI based platform (x86), I also want that card to work
seamlessly without requiring the user to create an ACPI overlay.
If you proposal was to use DT overlays on an ACPI based system, doing
so would also require to "plug" the PCI subystem when described with
ACPI to "probe" DT overlays describing PCI devices, not sure this is
something trivial and it would be PCI centric.
>
> 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 ? 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.
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 ? 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.
> 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.
>
>
> Rob
>
>
> https://lore.kernel.org/lkml/[email protected]/
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).
Thanks,
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
Le Tue, 5 Apr 2022 12:11:51 -0500,
Rob Herring <[email protected]> a écrit :
> On Tue, Apr 5, 2022 at 10:52 AM Clément Léger <[email protected]> wrote:
> >
> > Le Tue, 5 Apr 2022 09:47:20 -0500,
> > Rob Herring <[email protected]> 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...
Ok...
[...]
>
>
> > > 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.
Indeed.
>
> > 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.
>
>
> > > https://lore.kernel.org/lkml/[email protected]/
> >
> > 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
https://bootlin.com
Le Tue, 5 Apr 2022 16:28:02 -0500,
Rob Herring <[email protected]> a écrit :
> >
> > 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.
> >
> > 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).
>
> Here's a quick and dirty implementation creating DT nodes for PCI devices:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/pop-pci-nodes
Ok, thanks, after looking at the branch, it appears that you expect the
PCI nodes matching the probed PCI devices should be created by the PCI
subsystem itself. My previous comment saying that the node would be
created by the PCI driver itself is then wrong and I understand what
you meant.
Then, there is still a bit of magic to do to correctly fill the ranges
for translation and then the driver "simply" have to load the dtbo and
apply it with of_overlay_fdt_apply().
>
> Rob
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
On Wed, Apr 06, 2022 at 09:52:13AM +0200, Cl?ment L?ger wrote:
> Le Tue, 5 Apr 2022 16:28:02 -0500,
> Rob Herring <[email protected]> a ?crit :
>
>
> > >
> > > 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.
> > >
> > > 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).
> >
> > Here's a quick and dirty implementation creating DT nodes for PCI devices:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/pop-pci-nodes
>
> Ok, thanks, after looking at the branch, it appears that you expect the
> PCI nodes matching the probed PCI devices should be created by the PCI
> subsystem itself. My previous comment saying that the node would be
> created by the PCI driver itself is then wrong and I understand what
> you meant.
As I said before, the driver could create its node and all the parents.
I went with the other option partly because I didn't have a particular
driver. That has the advantage of only creating necessary nodes and
provides a way to trigger doing so. The issue with it is the timing of
when the node gets set (after parent devices have probed).
> Then, there is still a bit of magic to do to correctly fill the ranges
> for translation and then the driver "simply" have to load the dtbo and
> apply it with of_overlay_fdt_apply().
The host bridge resource list should have all the information needed.
Rob
On Wed, Apr 06, 2022 at 09:40:19AM +0200, Cl?ment L?ger wrote:
> Le Tue, 5 Apr 2022 12:11:51 -0500,
> Rob Herring <[email protected]> a ?crit :
>
> > On Tue, Apr 5, 2022 at 10:52 AM Cl?ment L?ger <[email protected]> wrote:
> > >
> > > Le Tue, 5 Apr 2022 09:47:20 -0500,
> > > Rob Herring <[email protected]> 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...
>
> Ok...
>
> [...]
>
> >
> >
> > > > 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.
Rob
On 06/04/2022 08:19:16-0500, Rob Herring wrote:
> > > > > 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.
>
Last time I tried that, this was not working well because it means that
the ranges property of the device depends on the host machine...
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Le Wed, 6 Apr 2022 15:33:46 +0200,
Alexandre Belloni <[email protected]> a écrit :
> On 06/04/2022 08:19:16-0500, Rob Herring wrote:
> > > > > > 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.
> >
>
> Last time I tried that, this was not working well because it means that
> the ranges property of the device depends on the host machine...
>
Yes, but we can actually resolve that dynamically. The ranges property
that is inserted is inserted in the top node (the PCI device one).
Since this node will be created by "us" (the kernel/driver), we can
insert whatever we want. And most probably, we'll insert a specific BAR
remapping. The underlying nodes will then be loaded from the overlay
and merged in the top PCI device node (At least I guess ;)). These
nodes don't need any "machine dependent" addresses, only realtive
addresses from the top node.
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
Le Wed, 6 Apr 2022 08:19:16 -0500,
Rob Herring <[email protected]> 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
Le Mon, 25 Apr 2022 12:21:15 +0200,
Philipp Zabel <[email protected]> a écrit :
> Hi Clément,
>
> On Fr, 2022-04-08 at 17:48 +0200, Clément Léger wrote:
> [...]
> > > > > > > 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.
>
> Could you show this off somewhere?
>
> From this I take that fwnode support in the reset subsystem is not of
> use to you anymore. I'll postpone taking your patches then, until they
> are needed.
>
> regards
> Philipp
Hi Philip,
Sorry for the lack of asnwer. Indeed, the fwnode support can be left
out. I'm preparing the patches for contribution for this OF overlay
thing, and everything regarding the subsystem have been removed. You
can look at commits [1], [2], [3] and at a driver using them [4]. This
allows the subsystem to be kept entirely without a single modification
(except for the PCI/OF one).
Thanks,
Clément
[1]
https://github.com/clementleger/linux/commit/248d7f25951f90cf5647d295e1c5051cc72ed970
[2]
https://github.com/clementleger/linux/commit/bf3c4c0749e5110b6a58ac9622cafc10872ed17f
[3]
https://github.com/clementleger/linux/commit/8764a2e386fdede73991415277b95e79553622f3
[4]
https://github.com/clementleger/linux/commit/7c66a1ad8f3fadfb538c410f192c9573c66338d5
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
Hi Clément,
On Fr, 2022-04-08 at 17:48 +0200, Clément Léger wrote:
[...]
> > > > > > 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.
Could you show this off somewhere?
From this I take that fwnode support in the reset subsystem is not of
use to you anymore. I'll postpone taking your patches then, until they
are needed.
regards
Philipp