| Message ID | 20220427094502.456111-1-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | add dynamic PCI device of_node creation for overlay | expand |
On 4/27/22 04:44, Clément Léger wrote: > This series adds foundation work to support the lan9662 PCIe card. This > card is meant to be used an ethernet switch with 2 x RJ45 ports and > 2 x 2.5G SFPs. The lan966x SoCs can be used in two different ways: > > - It can run Linux by itself, on ARM64 cores included in the SoC. This > use-case of the lan966x is currently being upstreamed, using a > traditional Device Tree representation of the lan996x HW blocks [1] > A number of drivers for the different IPs of the SoC have already > been merged in upstream Linux. > > - It can be used as a PCIe endpoint, connected to a separate platform > that acts as the PCIe root complex. In this case, all the devices > that are embedded on this SoC are exposed through PCIe BARs and the > ARM64 cores of the SoC are not used. Since this is a PCIe card, it > can be plugged on any platform, of any architecture supporting PCIe. > > The problem that arose is that we want to reuse all the existing OF > compatible drivers that are used when in SoC mode to instantiate the > PCI device when in PCIe endpoint mode. > > A previous attempt to tackle this problem was made using fwnode [1]. > However, this proved being way too invasive and it required > modifications in both subsystems and drivers to support fwnode. First > series did not lead to a consensus and multiple ideas to support this > use-case were mentioned (ACPI overlay, fwnode, device-tree overlay). > Since it only seemed that fwnode was not a totally silly idea, we > continued on this way. > > However, on the series that added fwnode support to the reset subsystem, > Rob Herring mentioned the fact that OF overlay might actually be the > best way to probe PCI devices and populate platform drivers using this > overlay. He also provided a branch containing some commits that helped I need to go look at the various email threads mentioned above before I continue reading this patch series. I do have serious concerns with this approach. I need to investigate more fully before I can determine whether the concerns are addressed sufficiently. To give some background to my longstanding response to similar proposals, here is my old statement from https://elinux.org/Device_Tree_Reference: Overlays Mainline Linux Support Run time overlay apply and run time overlay remove from user space are not supported in the mainline kernel. There are out of tree patches to implement this feature via an overlay manager. The overlay manager is used successfully by many users for specific overlays on specific boards with specific environments and use cases. However, there are many issues with the Linux kernel overlay implementation due to incomplete and incorrect code. The overlay manager has not been accepted in mainline due to these issues. Once these issues are resolved, it is expected that some method of run time overlay apply and overlay removal from user space will be supported by the Linux kernel. There is a possibility that overlay apply and overlay remove support could be phased in slowly, feature by feature, as specific issues are resolved. Those are my words, not Rob's, but I thought that Rob was somewhat in agreement with those ideas. Apparently either I misunderstood his thoughts, or his thoughts have evolved, since you say that he suggested overlays in one of the above email threads, and you list him as a co-developer. In the next line of the elinux info above, I provide a link to more detailed information: Frank's thoughts on what is needed to complete basic overlay support The link goes to: https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts That page provides an incomplete list of issues to be resolved, and a list of "what has been completed". Please read through the elinux.org page to understand the basis of my concerns. If after reading through the related email threads, and this thread, I agree that overlays are a good approach, I am already aware of areas that I will have specific comments about on the patches in this thread. -Frank > to implement this idea on a x86 setup. Due to the dynamic nature of PCI > bus enumeration, some other modifications needs to be applied on the > overlay to apply it correctly. Indeed, it is necessary to modify the > target node of the fragments to apply them correctly on the PCI device > that was probed. Moreover, the 'ranges' must be set according to the > BAR addresses in order to remap devices to the correct PCI addresses. > These modifications are the located into the driver since the remapping > is something that is specific to each driver. > > After modifications, this proves to be successful and a full support of > the aforementioned lan966x PCI card was added. The modifications to > support that (apply an overlay on a dynamically created PCI of_node) are > actually minimal and only touches a few places (pci/of.c). This series > contains the 3 commits that are necessary to do that: > > - First commit creates the root node if not present on a x86 setup > without a firmware provided device-tree. > - Second one dynamically creates the PCI bus/device device-tree node > hierarchy using changeset API. > - Finally a last commit allows to apply an overlay by targeting a > specific device-tree node. > > Other problems that might be considered with this series is the fact > that CONFIG_OF is not enabled by default on x86 configuration and thus > the driver can't be used without rebuilding a complete kernel with > CONFIG_OF=y. In order to fully support this PCIe card and allow lambda > user to use this driver, it would be almost mandatory to enable > CONFIG_OF by default on such setup. > > A driver using this support was added and can be seen at [3]. This > driver embeds a builtin overlay and applies it to the live tree using > of_overlay_fdt_apply_to_node(). An interrupt driver is also included and > associated to a node that is added by the overlay. The driver also > insert a specific "ranges" property based on the BAR values which allows > to remap the device-tree node to BAR addresses dynamically. This is > needed to allow applying the overlay without depending on specific > enumeration BAR addresses. > > This series was tested on a x86 kernel using CONFIG_OF under a virtual > machine using PCI passthrough. > > Link: [1] https://lore.kernel.org/lkml/YhQHqDJvahgriDZK@lunn.ch/t/ > Link: [2] https://lore.kernel.org/lkml/20220408174841.34458529@fixe.home/T/ > Link: [3] https://github.com/clementleger/linux/tree/lan966x/of_support > > Clément Léger (3): > of: always populate a root node > PCI: of: create DT nodes for PCI devices if they do not exists > of: overlay: add of_overlay_fdt_apply_to_node() > > drivers/of/base.c | 16 +++- > drivers/of/overlay.c | 21 +++-- > drivers/pci/of.c | 184 +++++++++++++++++++++++++++++++++++++++++++ > include/linux/of.h | 17 +++- > 4 files changed, 224 insertions(+), 14 deletions(-) >
Le Fri, 6 May 2022 13:33:22 -0500, Frank Rowand <frowand.list@gmail.com> a écrit : > On 4/27/22 04:44, Clément Léger wrote: > > This series adds foundation work to support the lan9662 PCIe card. > > This card is meant to be used an ethernet switch with 2 x RJ45 > > ports and 2 x 2.5G SFPs. The lan966x SoCs can be used in two > > different ways: > > > > - It can run Linux by itself, on ARM64 cores included in the SoC. > > This use-case of the lan966x is currently being upstreamed, using a > > traditional Device Tree representation of the lan996x HW blocks > > [1] A number of drivers for the different IPs of the SoC have > > already been merged in upstream Linux. > > > > - It can be used as a PCIe endpoint, connected to a separate > > platform that acts as the PCIe root complex. In this case, all the > > devices that are embedded on this SoC are exposed through PCIe BARs > > and the ARM64 cores of the SoC are not used. Since this is a PCIe > > card, it can be plugged on any platform, of any architecture > > supporting PCIe. > > > > The problem that arose is that we want to reuse all the existing OF > > compatible drivers that are used when in SoC mode to instantiate the > > PCI device when in PCIe endpoint mode. > > > > A previous attempt to tackle this problem was made using fwnode [1]. > > However, this proved being way too invasive and it required > > modifications in both subsystems and drivers to support fwnode. > > First series did not lead to a consensus and multiple ideas to > > support this use-case were mentioned (ACPI overlay, fwnode, > > device-tree overlay). Since it only seemed that fwnode was not a > > totally silly idea, we continued on this way. > > > > However, on the series that added fwnode support to the reset > > subsystem, Rob Herring mentioned the fact that OF overlay might > > actually be the best way to probe PCI devices and populate platform > > drivers using this overlay. He also provided a branch containing > > some commits that helped > > I need to go look at the various email threads mentioned above before > I continue reading this patch series. > > I do have serious concerns with this approach. I need to investigate > more fully before I can determine whether the concerns are addressed > sufficiently. > > To give some background to my longstanding response to similar > proposals, here is my old statement from > https://elinux.org/Device_Tree_Reference: > > Overlays > Mainline Linux Support > Run time overlay apply and run time overlay remove from user space > are not supported in the mainline kernel. There are out of tree > patches to implement this feature via an overlay manager. The overlay > manager is used successfully by many users for specific overlays on > specific boards with specific environments and use cases. However, > there are many issues with the Linux kernel overlay implementation > due to incomplete and incorrect code. The overlay manager has not > been accepted in mainline due to these issues. Once these issues are > resolved, it is expected that some method of run time overlay apply > and overlay removal from user space will be supported by the Linux > kernel. > > There is a possibility that overlay apply and overlay remove > support could be phased in slowly, feature by feature, as specific > issues are resolved. Hi Frank, This work uses the kernel space interface (of_overlay_fdt_apply()) and the device tree overlay is builtin the driver. This interface was used until recently by rcu-dcar driver. While the only user (sic), this seems to work pretty well and I was able to use it successfully. Moreover, this support targets at using this with PCI devices. This devices are really well contained and do not interfere with other devices. This actually consists in adding a complete subtree into the existing device-tree and thus it limits the interactions between potentially platform provided devices and PCI ones. Clément > > Those are my words, not Rob's, but I thought that Rob was somewhat in > agreement with those ideas. Apparently either I misunderstood his > thoughts, or his thoughts have evolved, since you say that he > suggested overlays in one of the above email threads, and you list > him as a co-developer. > > In the next line of the elinux info above, I provide a link to more > detailed information: > > Frank's thoughts on what is needed to complete basic overlay > support > > The link goes to: > > https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts > > That page provides an incomplete list of issues to be resolved, and > a list of "what has been completed". > > Please read through the elinux.org page to understand the basis of > my concerns. > > If after reading through the related email threads, and this thread, > I agree that overlays are a good approach, I am already aware of areas > that I will have specific comments about on the patches in this > thread. > > -Frank > > > to implement this idea on a x86 setup. Due to the dynamic nature of > > PCI bus enumeration, some other modifications needs to be applied > > on the overlay to apply it correctly. Indeed, it is necessary to > > modify the target node of the fragments to apply them correctly on > > the PCI device that was probed. Moreover, the 'ranges' must be set > > according to the BAR addresses in order to remap devices to the > > correct PCI addresses. These modifications are the located into the > > driver since the remapping is something that is specific to each > > driver. > > > > After modifications, this proves to be successful and a full > > support of the aforementioned lan966x PCI card was added. The > > modifications to support that (apply an overlay on a dynamically > > created PCI of_node) are actually minimal and only touches a few > > places (pci/of.c). This series contains the 3 commits that are > > necessary to do that: > > > > - First commit creates the root node if not present on a x86 setup > > without a firmware provided device-tree. > > - Second one dynamically creates the PCI bus/device device-tree node > > hierarchy using changeset API. > > - Finally a last commit allows to apply an overlay by targeting a > > specific device-tree node. > > > > Other problems that might be considered with this series is the fact > > that CONFIG_OF is not enabled by default on x86 configuration and > > thus the driver can't be used without rebuilding a complete kernel > > with CONFIG_OF=y. In order to fully support this PCIe card and > > allow lambda user to use this driver, it would be almost mandatory > > to enable CONFIG_OF by default on such setup. > > > > A driver using this support was added and can be seen at [3]. This > > driver embeds a builtin overlay and applies it to the live tree > > using of_overlay_fdt_apply_to_node(). An interrupt driver is also > > included and associated to a node that is added by the overlay. The > > driver also insert a specific "ranges" property based on the BAR > > values which allows to remap the device-tree node to BAR addresses > > dynamically. This is needed to allow applying the overlay without > > depending on specific enumeration BAR addresses. > > > > This series was tested on a x86 kernel using CONFIG_OF under a > > virtual machine using PCI passthrough. > > > > Link: [1] https://lore.kernel.org/lkml/YhQHqDJvahgriDZK@lunn.ch/t/ > > Link: [2] > > https://lore.kernel.org/lkml/20220408174841.34458529@fixe.home/T/ > > Link: [3] > > https://github.com/clementleger/linux/tree/lan966x/of_support > > > > Clément Léger (3): > > of: always populate a root node > > PCI: of: create DT nodes for PCI devices if they do not exists > > of: overlay: add of_overlay_fdt_apply_to_node() > > > > drivers/of/base.c | 16 +++- > > drivers/of/overlay.c | 21 +++-- > > drivers/pci/of.c | 184 > > +++++++++++++++++++++++++++++++++++++++++++ include/linux/of.h | > > 17 +++- 4 files changed, 224 insertions(+), 14 deletions(-) > > >
On 5/9/22 07:16, Clément Léger wrote: > Le Fri, 6 May 2022 13:33:22 -0500, > Frank Rowand <frowand.list@gmail.com> a écrit : > >> On 4/27/22 04:44, Clément Léger wrote: >>> This series adds foundation work to support the lan9662 PCIe card. >>> This card is meant to be used an ethernet switch with 2 x RJ45 >>> ports and 2 x 2.5G SFPs. The lan966x SoCs can be used in two >>> different ways: >>> >>> - It can run Linux by itself, on ARM64 cores included in the SoC. >>> This use-case of the lan966x is currently being upstreamed, using a >>> traditional Device Tree representation of the lan996x HW blocks >>> [1] A number of drivers for the different IPs of the SoC have >>> already been merged in upstream Linux. >>> >>> - It can be used as a PCIe endpoint, connected to a separate >>> platform that acts as the PCIe root complex. In this case, all the >>> devices that are embedded on this SoC are exposed through PCIe BARs >>> and the ARM64 cores of the SoC are not used. Since this is a PCIe >>> card, it can be plugged on any platform, of any architecture >>> supporting PCIe. >>> >>> The problem that arose is that we want to reuse all the existing OF >>> compatible drivers that are used when in SoC mode to instantiate the >>> PCI device when in PCIe endpoint mode. >>> >>> A previous attempt to tackle this problem was made using fwnode [1]. >>> However, this proved being way too invasive and it required >>> modifications in both subsystems and drivers to support fwnode. >>> First series did not lead to a consensus and multiple ideas to >>> support this use-case were mentioned (ACPI overlay, fwnode, >>> device-tree overlay). Since it only seemed that fwnode was not a >>> totally silly idea, we continued on this way. >>> >>> However, on the series that added fwnode support to the reset >>> subsystem, Rob Herring mentioned the fact that OF overlay might >>> actually be the best way to probe PCI devices and populate platform >>> drivers using this overlay. He also provided a branch containing >>> some commits that helped >> >> I need to go look at the various email threads mentioned above before >> I continue reading this patch series. >> >> I do have serious concerns with this approach. I need to investigate >> more fully before I can determine whether the concerns are addressed >> sufficiently. >> >> To give some background to my longstanding response to similar >> proposals, here is my old statement from >> https://elinux.org/Device_Tree_Reference: >> >> Overlays >> Mainline Linux Support >> Run time overlay apply and run time overlay remove from user space >> are not supported in the mainline kernel. There are out of tree >> patches to implement this feature via an overlay manager. The overlay >> manager is used successfully by many users for specific overlays on >> specific boards with specific environments and use cases. However, >> there are many issues with the Linux kernel overlay implementation >> due to incomplete and incorrect code. The overlay manager has not >> been accepted in mainline due to these issues. Once these issues are >> resolved, it is expected that some method of run time overlay apply >> and overlay removal from user space will be supported by the Linux >> kernel. >> >> There is a possibility that overlay apply and overlay remove >> support could be phased in slowly, feature by feature, as specific >> issues are resolved. > > Hi Frank, > > This work uses the kernel space interface (of_overlay_fdt_apply()) > and the device tree overlay is builtin the driver. This interface was > used until recently by rcu-dcar driver. While the only user (sic), > this seems to work pretty well and I was able to use it successfully. Yes, of_overlay_fdt_apply() was used by one driver. But that driver was explicitly recognized as a grandfathered exception, and not an example for other users. It was finally removed in 5.18-rc1. You may have used of_overlay_fdt_apply() in a specific use case at a specific kernel version, but if you read through the references I provided you will find that applying overlays after the kernel boots is a fragile endeavor, with expectations of bugs and problems being exposed as usage is changed (simple example is that my adding some overlay notifier unittests exposed yet another memory leak). The reference that I provided also shows how the overlay code is being improved over time. Even with improvements, it will remain fragile. > > Moreover, this support targets at using this with PCI devices. This > devices are really well contained and do not interfere with other > devices. This actually consists in adding a complete subtree into the > existing device-tree and thus it limits the interactions between > potentially platform provided devices and PCI ones. Yes, that it is very important that you have described this fact, both here and in other emails. Thank you for that information, it does help understanding the alternatives. I've hesitated in recommending a specific solution before better understanding the architecture of your pcie board and drivers, but I've delayed too long, so I am going to go ahead and mention one possibility at the risk of not yet fully understanding the situation. On the surface, it appears that your need might be well met by having a base devicetree that describes all of the pcie nodes, but with each node having a status of "disabled" so that they will not be used. Have a devicetree overlay describing the pcie card (as you proposed), where the overlay also includes a status of "ok" for the pcie node. Applying the overlay, with a method of redirecting the target to a specific pcie node would change the status of the pcie node to enable its use. (You have already proposed a patch to modify of_overlay_fdt_apply() to allow a modified target, so not a new concept from me.) My suggestion is to apply the overlay devicetree to the base devicetree before the combined FDT devicetree is passed to the kernel at boot. The overlay apply could be done by several different entities. It could be before the bootloader executes, it could be done by the bootloader, it could be done by a shim between the bootloader and the kernel. This method avoids all of the issues of applying an overlay to a running system that I find problematic. It is also a method used by the U-boot bootloader, as an example. The other big issue is mixing ACPI and devicetree on a single system. Historically, the Linux devicetree community has not been receptive to the ides of that mixture. Your example might be a specific case where the two can be isolated from each other, or maybe not. (For disclosure, I am essentially ACPI ignorant.) I suspect that mixing ACPI and devicetree is a recipe for disaster in the general case. More to come later as I finish reading through the various threads. -Frank > > Clément > >> >> Those are my words, not Rob's, but I thought that Rob was somewhat in >> agreement with those ideas. Apparently either I misunderstood his >> thoughts, or his thoughts have evolved, since you say that he >> suggested overlays in one of the above email threads, and you list >> him as a co-developer. >> >> In the next line of the elinux info above, I provide a link to more >> detailed information: >> >> Frank's thoughts on what is needed to complete basic overlay >> support >> >> The link goes to: >> >> https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts >> >> That page provides an incomplete list of issues to be resolved, and >> a list of "what has been completed". >> >> Please read through the elinux.org page to understand the basis of >> my concerns. >> >> If after reading through the related email threads, and this thread, >> I agree that overlays are a good approach, I am already aware of areas >> that I will have specific comments about on the patches in this >> thread. >> >> -Frank >> >>> to implement this idea on a x86 setup. Due to the dynamic nature of >>> PCI bus enumeration, some other modifications needs to be applied >>> on the overlay to apply it correctly. Indeed, it is necessary to >>> modify the target node of the fragments to apply them correctly on >>> the PCI device that was probed. Moreover, the 'ranges' must be set >>> according to the BAR addresses in order to remap devices to the >>> correct PCI addresses. These modifications are the located into the >>> driver since the remapping is something that is specific to each >>> driver. >>> >>> After modifications, this proves to be successful and a full >>> support of the aforementioned lan966x PCI card was added. The >>> modifications to support that (apply an overlay on a dynamically >>> created PCI of_node) are actually minimal and only touches a few >>> places (pci/of.c). This series contains the 3 commits that are >>> necessary to do that: >>> >>> - First commit creates the root node if not present on a x86 setup >>> without a firmware provided device-tree. >>> - Second one dynamically creates the PCI bus/device device-tree node >>> hierarchy using changeset API. >>> - Finally a last commit allows to apply an overlay by targeting a >>> specific device-tree node. >>> >>> Other problems that might be considered with this series is the fact >>> that CONFIG_OF is not enabled by default on x86 configuration and >>> thus the driver can't be used without rebuilding a complete kernel >>> with CONFIG_OF=y. In order to fully support this PCIe card and >>> allow lambda user to use this driver, it would be almost mandatory >>> to enable CONFIG_OF by default on such setup. >>> >>> A driver using this support was added and can be seen at [3]. This >>> driver embeds a builtin overlay and applies it to the live tree >>> using of_overlay_fdt_apply_to_node(). An interrupt driver is also >>> included and associated to a node that is added by the overlay. The >>> driver also insert a specific "ranges" property based on the BAR >>> values which allows to remap the device-tree node to BAR addresses >>> dynamically. This is needed to allow applying the overlay without >>> depending on specific enumeration BAR addresses. >>> >>> This series was tested on a x86 kernel using CONFIG_OF under a >>> virtual machine using PCI passthrough. >>> >>> Link: [1] https://lore.kernel.org/lkml/YhQHqDJvahgriDZK@lunn.ch/t/ >>> Link: [2] >>> https://lore.kernel.org/lkml/20220408174841.34458529@fixe.home/T/ >>> Link: [3] >>> https://github.com/clementleger/linux/tree/lan966x/of_support >>> >>> Clément Léger (3): >>> of: always populate a root node >>> PCI: of: create DT nodes for PCI devices if they do not exists >>> of: overlay: add of_overlay_fdt_apply_to_node() >>> >>> drivers/of/base.c | 16 +++- >>> drivers/of/overlay.c | 21 +++-- >>> drivers/pci/of.c | 184 >>> +++++++++++++++++++++++++++++++++++++++++++ include/linux/of.h | >>> 17 +++- 4 files changed, 224 insertions(+), 14 deletions(-) >>> >> >
Le Mon, 9 May 2022 10:56:36 -0500, Frank Rowand <frowand.list@gmail.com> a écrit : > > Hi Frank, > > > > This work uses the kernel space interface (of_overlay_fdt_apply()) > > and the device tree overlay is builtin the driver. This interface > > was used until recently by rcu-dcar driver. While the only user > > (sic), this seems to work pretty well and I was able to use it > > successfully. > > Yes, of_overlay_fdt_apply() was used by one driver. But that driver > was explicitly recognized as a grandfathered exception, and not an > example for other users. It was finally removed in 5.18-rc1. I noticed that unfortunately. > > You may have used of_overlay_fdt_apply() in a specific use case at > a specific kernel version, but if you read through the references > I provided you will find that applying overlays after the kernel > boots is a fragile endeavor, with expectations of bugs and problems > being exposed as usage is changed (simple example is that my adding > some overlay notifier unittests exposed yet another memory leak). > > The reference that I provided also shows how the overlay code is > being improved over time. Even with improvements, it will remain > fragile. Acked. > > > > > Moreover, this support targets at using this with PCI devices. This > > devices are really well contained and do not interfere with other > > devices. This actually consists in adding a complete subtree into > > the existing device-tree and thus it limits the interactions between > > potentially platform provided devices and PCI ones. > > Yes, that it is very important that you have described this fact, both > here and in other emails. Thank you for that information, it does > help understanding the alternatives. > > I've hesitated in recommending a specific solution before better > understanding the architecture of your pcie board and drivers, but > I've delayed too long, so I am going to go ahead and mention one > possibility at the risk of not yet fully understanding the situation. > > On the surface, it appears that your need might be well met by having > a base devicetree that describes all of the pcie nodes, but with each > node having a status of "disabled" so that they will not be used. > Have a devicetree overlay describing the pcie card (as you proposed), > where the overlay also includes a status of "ok" for the pcie node. > Applying the overlay, with a method of redirecting the target to a > specific pcie node would change the status of the pcie node to enable > its use. (You have already proposed a patch to modify > of_overlay_fdt_apply() to allow a modified target, so not a new > concept from me.) My suggestion is to apply the overlay devicetree > to the base devicetree before the combined FDT devicetree is passed > to the kernel at boot. The overlay apply could be done by several > different entities. It could be before the bootloader executes, it > could be done by the bootloader, it could be done by a shim between > the bootloader and the kernel. This method avoids all of the issues > of applying an overlay to a running system that I find problematic. > It is also a method used by the U-boot bootloader, as an example. Ok, that is actually possible on a system that is given a device-tree by the bootloader. But on a system that is desrcibed using ACPI (such as the x86), this is much more difficult (at least to my knowledge)... We want this feature to be easy to use for the end user. Adding such configuration which also differs between various architecture is clearly not so easy to setup. Moreover, since the PCI is meant to be "Plug and Play", such configuration would completely break that. If the user switches the PCIe card from one slot to another, the bootloader configuration will need to be modified. This seems a big no way for me (and for the user). > > The other big issue is mixing ACPI and devicetree on a single system. > Historically, the Linux devicetree community has not been receptive > to the ides of that mixture. Your example might be a specific case > where the two can be isolated from each other, or maybe not. (For > disclosure, I am essentially ACPI ignorant.) I suspect that mixing > ACPI and devicetree is a recipe for disaster in the general case. Agreed, on that fact, it did raised some eyebrows, and it was for that specific concern that initially, I proposed the fwnode solution. Honestly, the fwnode conversion represent a lot of work (hundreds of lines easily) + requires a conversion of all the subsystem that are not fwnode ready (spoiler: almost all of them are not ready). After implementing Rob's solution, the device-tree overlay really seems the cleaner to me and requires much less modifications. > > More to come later as I finish reading through the various threads. Ok, thanks for your time ! Clément > > -Frank
On Mon, May 09, 2022 at 06:09:17PM +0200, Clément Léger wrote: > Le Mon, 9 May 2022 10:56:36 -0500, > Frank Rowand <frowand.list@gmail.com> a écrit : ... > > On the surface, it appears that your need might be well met by having > > a base devicetree that describes all of the pcie nodes, but with each > > node having a status of "disabled" so that they will not be used. > > Have a devicetree overlay describing the pcie card (as you proposed), > > where the overlay also includes a status of "ok" for the pcie node. > > Applying the overlay, with a method of redirecting the target to a > > specific pcie node would change the status of the pcie node to enable > > its use. (You have already proposed a patch to modify > > of_overlay_fdt_apply() to allow a modified target, so not a new > > concept from me.) My suggestion is to apply the overlay devicetree > > to the base devicetree before the combined FDT devicetree is passed > > to the kernel at boot. The overlay apply could be done by several > > different entities. It could be before the bootloader executes, it > > could be done by the bootloader, it could be done by a shim between > > the bootloader and the kernel. This method avoids all of the issues > > of applying an overlay to a running system that I find problematic. > > It is also a method used by the U-boot bootloader, as an example. > > Ok, that is actually possible on a system that is given a device-tree > by the bootloader. But on a system that is desrcibed using ACPI (such > as the x86), this is much more difficult (at least to my knowledge)... > We want this feature to be easy to use for the end user. Adding such > configuration which also differs between various architecture is > clearly not so easy to setup. > > Moreover, since the PCI is meant to be "Plug and Play", such > configuration would completely break that. If the user switches the > PCIe card from one slot to another, the bootloader configuration will > need to be modified. This seems a big no way for me (and for the user). The main problem here is that Linux does not support hotplugging for the devices behind non-hotpluggable buses. You need to develop something to say that the device tree (in terms of hardware) can morph at run-time transparently to the user. I think the closest one is what FPGA does, or at least should do. > > The other big issue is mixing ACPI and devicetree on a single system. > > Historically, the Linux devicetree community has not been receptive > > to the ides of that mixture. Your example might be a specific case > > where the two can be isolated from each other, or maybe not. (For > > disclosure, I am essentially ACPI ignorant.) I suspect that mixing > > ACPI and devicetree is a recipe for disaster in the general case. > > Agreed, on that fact, it did raised some eyebrows, and it was for that > specific concern that initially, I proposed the fwnode solution. > Honestly, the fwnode conversion represent a lot of work (hundreds of > lines easily) + requires a conversion of all the subsystem that are not > fwnode ready (spoiler: almost all of them are not ready). In either case you need to provide a format that would be suitable for DT-based as well as ACPI-based platforms.
On Mon, May 9, 2022 at 10:56 AM Frank Rowand <frowand.list@gmail.com> wrote: > > On 5/9/22 07:16, Clément Léger wrote: > > Le Fri, 6 May 2022 13:33:22 -0500, > > Frank Rowand <frowand.list@gmail.com> a écrit : > > > >> On 4/27/22 04:44, Clément Léger wrote: > >>> This series adds foundation work to support the lan9662 PCIe card. > >>> This card is meant to be used an ethernet switch with 2 x RJ45 > >>> ports and 2 x 2.5G SFPs. The lan966x SoCs can be used in two > >>> different ways: > >>> > >>> - It can run Linux by itself, on ARM64 cores included in the SoC. > >>> This use-case of the lan966x is currently being upstreamed, using a > >>> traditional Device Tree representation of the lan996x HW blocks > >>> [1] A number of drivers for the different IPs of the SoC have > >>> already been merged in upstream Linux. > >>> > >>> - It can be used as a PCIe endpoint, connected to a separate > >>> platform that acts as the PCIe root complex. In this case, all the > >>> devices that are embedded on this SoC are exposed through PCIe BARs > >>> and the ARM64 cores of the SoC are not used. Since this is a PCIe > >>> card, it can be plugged on any platform, of any architecture > >>> supporting PCIe. > >>> > >>> The problem that arose is that we want to reuse all the existing OF > >>> compatible drivers that are used when in SoC mode to instantiate the > >>> PCI device when in PCIe endpoint mode. > >>> > >>> A previous attempt to tackle this problem was made using fwnode [1]. > >>> However, this proved being way too invasive and it required > >>> modifications in both subsystems and drivers to support fwnode. > >>> First series did not lead to a consensus and multiple ideas to > >>> support this use-case were mentioned (ACPI overlay, fwnode, > >>> device-tree overlay). Since it only seemed that fwnode was not a > >>> totally silly idea, we continued on this way. > >>> > >>> However, on the series that added fwnode support to the reset > >>> subsystem, Rob Herring mentioned the fact that OF overlay might > >>> actually be the best way to probe PCI devices and populate platform > >>> drivers using this overlay. He also provided a branch containing > >>> some commits that helped > >> > >> I need to go look at the various email threads mentioned above before > >> I continue reading this patch series. > >> > >> I do have serious concerns with this approach. I need to investigate > >> more fully before I can determine whether the concerns are addressed > >> sufficiently. > >> > >> To give some background to my longstanding response to similar > >> proposals, here is my old statement from > >> https://elinux.org/Device_Tree_Reference: > >> > >> Overlays > >> Mainline Linux Support > >> Run time overlay apply and run time overlay remove from user space > >> are not supported in the mainline kernel. There are out of tree > >> patches to implement this feature via an overlay manager. The overlay > >> manager is used successfully by many users for specific overlays on > >> specific boards with specific environments and use cases. However, > >> there are many issues with the Linux kernel overlay implementation > >> due to incomplete and incorrect code. The overlay manager has not > >> been accepted in mainline due to these issues. Once these issues are > >> resolved, it is expected that some method of run time overlay apply > >> and overlay removal from user space will be supported by the Linux > >> kernel. > >> > >> There is a possibility that overlay apply and overlay remove > >> support could be phased in slowly, feature by feature, as specific > >> issues are resolved. > > > > Hi Frank, > > > > This work uses the kernel space interface (of_overlay_fdt_apply()) > > and the device tree overlay is builtin the driver. This interface was > > used until recently by rcu-dcar driver. While the only user (sic), > > this seems to work pretty well and I was able to use it successfully. > > Yes, of_overlay_fdt_apply() was used by one driver. But that driver > was explicitly recognized as a grandfathered exception, and not an > example for other users. It was finally removed in 5.18-rc1. What API are folks supposed to use exactly? That's the only API to apply an overlay. I thought the FPGA mgr code was using it too, but it's not. It doesn't look to me like the upstream code there even works as nothing applies the overlays AFAICT. If there are no in kernel users applying overlays, then let's remove the overlay code. I hear it has lots of problems. I am *way* more comfortable with driver specific applying of overlays than any generic mechanism. I don't think we'll ever have a generic mechanism. At least not one that doesn't end up with the same usage constraints driver specific cases would have. > You may have used of_overlay_fdt_apply() in a specific use case at > a specific kernel version, but if you read through the references > I provided you will find that applying overlays after the kernel > boots is a fragile endeavor, with expectations of bugs and problems > being exposed as usage is changed (simple example is that my adding > some overlay notifier unittests exposed yet another memory leak). The exception being specific drivers that are only applying overlays isolated to their device as this usecase is. The usecase here is entirely self-contained. The only base tree is only what's needed to represent the PCI device. > The reference that I provided also shows how the overlay code is > being improved over time. Even with improvements, it will remain > fragile. > > > > > Moreover, this support targets at using this with PCI devices. This > > devices are really well contained and do not interfere with other > > devices. This actually consists in adding a complete subtree into the > > existing device-tree and thus it limits the interactions between > > potentially platform provided devices and PCI ones. > > Yes, that it is very important that you have described this fact, both > here and in other emails. Thank you for that information, it does help > understanding the alternatives. > > I've hesitated in recommending a specific solution before better > understanding the architecture of your pcie board and drivers, but > I've delayed too long, so I am going to go ahead and mention one > possibility at the risk of not yet fully understanding the situation. > > On the surface, it appears that your need might be well met by having > a base devicetree that describes all of the pcie nodes, but with each > node having a status of "disabled" so that they will not be used. > Have a devicetree overlay describing the pcie card (as you proposed), > where the overlay also includes a status of "ok" for the pcie node. > Applying the overlay, with a method of redirecting the target to a > specific pcie node would change the status of the pcie node to enable > its use. (You have already proposed a patch to modify of_overlay_fdt_apply() > to allow a modified target, so not a new concept from me.) My suggestion > is to apply the overlay devicetree to the base devicetree before the > combined FDT devicetree is passed to the kernel at boot. The overlay > apply could be done by several different entities. It could be before > the bootloader executes, it could be done by the bootloader, it could > be done by a shim between the bootloader and the kernel. This method > avoids all of the issues of applying an overlay to a running system > that I find problematic. It is also a method used by the U-boot > bootloader, as an example. Adding a layer, the solution to all problems... I don't think that's a workable solution unless all the components are in one party's control. Given the desire to work on ACPI and DT based systems, that doesn't sound like the case here. > The other big issue is mixing ACPI and devicetree on a single system. > Historically, the Linux devicetree community has not been receptive > to the ides of that mixture. Your example might be a specific case > where the two can be isolated from each other, or maybe not. (For > disclosure, I am essentially ACPI ignorant.) I suspect that mixing > ACPI and devicetree is a recipe for disaster in the general case. The idea here is what is described by ACPI and DT are disjoint which I think we can enforce. Enforcement comes from fwnode assuming it has either an ACPI or a DT handle, but not both. > More to come later as I finish reading through the various threads. There is also the Xilinx folks wanting to support their PCI FPGA card with DT for the FPGA contents on both ACPI and DT systems. Rob
On 5/9/22 11:09, Clément Léger wrote: > Le Mon, 9 May 2022 10:56:36 -0500, > Frank Rowand <frowand.list@gmail.com> a écrit : > >>> Hi Frank, >>> >>> This work uses the kernel space interface (of_overlay_fdt_apply()) >>> and the device tree overlay is builtin the driver. This interface >>> was used until recently by rcu-dcar driver. While the only user >>> (sic), this seems to work pretty well and I was able to use it >>> successfully. >> >> Yes, of_overlay_fdt_apply() was used by one driver. But that driver >> was explicitly recognized as a grandfathered exception, and not an >> example for other users. It was finally removed in 5.18-rc1. > > I noticed that unfortunately. > >> >> You may have used of_overlay_fdt_apply() in a specific use case at >> a specific kernel version, but if you read through the references >> I provided you will find that applying overlays after the kernel >> boots is a fragile endeavor, with expectations of bugs and problems >> being exposed as usage is changed (simple example is that my adding >> some overlay notifier unittests exposed yet another memory leak). >> >> The reference that I provided also shows how the overlay code is >> being improved over time. Even with improvements, it will remain >> fragile. > > Acked. > >> >>> >>> Moreover, this support targets at using this with PCI devices. This >>> devices are really well contained and do not interfere with other >>> devices. This actually consists in adding a complete subtree into >>> the existing device-tree and thus it limits the interactions between >>> potentially platform provided devices and PCI ones. >> >> Yes, that it is very important that you have described this fact, both >> here and in other emails. Thank you for that information, it does >> help understanding the alternatives. >> >> I've hesitated in recommending a specific solution before better >> understanding the architecture of your pcie board and drivers, but >> I've delayed too long, so I am going to go ahead and mention one >> possibility at the risk of not yet fully understanding the situation. >> >> On the surface, it appears that your need might be well met by having >> a base devicetree that describes all of the pcie nodes, but with each >> node having a status of "disabled" so that they will not be used. >> Have a devicetree overlay describing the pcie card (as you proposed), >> where the overlay also includes a status of "ok" for the pcie node. >> Applying the overlay, with a method of redirecting the target to a >> specific pcie node would change the status of the pcie node to enable >> its use. (You have already proposed a patch to modify >> of_overlay_fdt_apply() to allow a modified target, so not a new >> concept from me.) My suggestion is to apply the overlay devicetree >> to the base devicetree before the combined FDT devicetree is passed >> to the kernel at boot. The overlay apply could be done by several >> different entities. It could be before the bootloader executes, it >> could be done by the bootloader, it could be done by a shim between >> the bootloader and the kernel. This method avoids all of the issues >> of applying an overlay to a running system that I find problematic. >> It is also a method used by the U-boot bootloader, as an example. > Apologies if my following questions have already been answered in the other threads... > Ok, that is actually possible on a system that is given a device-tree > by the bootloader. But on a system that is desrcibed using ACPI (such > as the x86), this is much more difficult (at least to my knowledge)... > We want this feature to be easy to use for the end user. Adding such > configuration which also differs between various architecture is > clearly not so easy to setup. Are you trying to make your card work on any ACPI based system (x86, x86-64, etc)? Or do you have a specific model of computer that you want to make this work on for a specific customer or appliance? If for many arbitrary systems, can you limit it to one architecture or sub-architecture? > > Moreover, since the PCI is meant to be "Plug and Play", such > configuration would completely break that. If the user switches the > PCIe card from one slot to another, the bootloader configuration will > need to be modified. This seems a big no way for me (and for the user). Yes. I was envisioning the pre-bootloader, bootloader, or Linux pre-boot shim dynamically determining the slot containing the card, and applying the overlay devicetree to the base devicetree, retargeting the overlay to the proper location, before the Linux boot. The base devicetree would be for a specific type of machine or family of machines, just as is the case for all devicetree based systems. > >> >> The other big issue is mixing ACPI and devicetree on a single system. >> Historically, the Linux devicetree community has not been receptive >> to the ides of that mixture. Your example might be a specific case >> where the two can be isolated from each other, or maybe not. (For >> disclosure, I am essentially ACPI ignorant.) I suspect that mixing >> ACPI and devicetree is a recipe for disaster in the general case. > > Agreed, on that fact, it did raised some eyebrows, and it was for that > specific concern that initially, I proposed the fwnode solution. > Honestly, the fwnode conversion represent a lot of work (hundreds of > lines easily) + requires a conversion of all the subsystem that are not > fwnode ready (spoiler: almost all of them are not ready). > > After implementing Rob's solution, the device-tree overlay really seems > the cleaner to me and requires much less modifications. > >> >> More to come later as I finish reading through the various threads. > > Ok, thanks for your time ! Your welcome. I'll keep looking deeper into the previous threads. -Frank > > Clément > >> >> -Frank > > .
On 5/9/22 12:00, Andy Shevchenko wrote: > On Mon, May 09, 2022 at 06:09:17PM +0200, Clément Léger wrote: >> Le Mon, 9 May 2022 10:56:36 -0500, >> Frank Rowand <frowand.list@gmail.com> a écrit : > > ... > >>> On the surface, it appears that your need might be well met by having >>> a base devicetree that describes all of the pcie nodes, but with each >>> node having a status of "disabled" so that they will not be used. >>> Have a devicetree overlay describing the pcie card (as you proposed), >>> where the overlay also includes a status of "ok" for the pcie node. >>> Applying the overlay, with a method of redirecting the target to a >>> specific pcie node would change the status of the pcie node to enable >>> its use. (You have already proposed a patch to modify >>> of_overlay_fdt_apply() to allow a modified target, so not a new >>> concept from me.) My suggestion is to apply the overlay devicetree >>> to the base devicetree before the combined FDT devicetree is passed >>> to the kernel at boot. The overlay apply could be done by several >>> different entities. It could be before the bootloader executes, it >>> could be done by the bootloader, it could be done by a shim between >>> the bootloader and the kernel. This method avoids all of the issues >>> of applying an overlay to a running system that I find problematic. >>> It is also a method used by the U-boot bootloader, as an example. >> >> Ok, that is actually possible on a system that is given a device-tree >> by the bootloader. But on a system that is desrcibed using ACPI (such >> as the x86), this is much more difficult (at least to my knowledge)... >> We want this feature to be easy to use for the end user. Adding such >> configuration which also differs between various architecture is >> clearly not so easy to setup. >> >> Moreover, since the PCI is meant to be "Plug and Play", such >> configuration would completely break that. If the user switches the >> PCIe card from one slot to another, the bootloader configuration will >> need to be modified. This seems a big no way for me (and for the user). > > The main problem here is that Linux does not support hotplugging for the > devices behind non-hotpluggable buses. You need to develop something to > say that the device tree (in terms of hardware) can morph at run-time > transparently to the user. I think the closest one is what FPGA does, > or at least should do. That is something I was not aware of yet. Is the card in question a hotpluggable card? Do the systems that you anticipate plugging the card into support hotplug? -Frank > >>> The other big issue is mixing ACPI and devicetree on a single system. >>> Historically, the Linux devicetree community has not been receptive >>> to the ides of that mixture. Your example might be a specific case >>> where the two can be isolated from each other, or maybe not. (For >>> disclosure, I am essentially ACPI ignorant.) I suspect that mixing >>> ACPI and devicetree is a recipe for disaster in the general case. >> >> Agreed, on that fact, it did raised some eyebrows, and it was for that >> specific concern that initially, I proposed the fwnode solution. >> Honestly, the fwnode conversion represent a lot of work (hundreds of >> lines easily) + requires a conversion of all the subsystem that are not >> fwnode ready (spoiler: almost all of them are not ready). > > In either case you need to provide a format that would be suitable for > DT-based as well as ACPI-based platforms. >
On 5/9/22 13:36, Rob Herring wrote: > On Mon, May 9, 2022 at 10:56 AM Frank Rowand <frowand.list@gmail.com> wrote: >> >> On 5/9/22 07:16, Clément Léger wrote: >>> Le Fri, 6 May 2022 13:33:22 -0500, >>> Frank Rowand <frowand.list@gmail.com> a écrit : >>> >>>> On 4/27/22 04:44, Clément Léger wrote: >>>>> This series adds foundation work to support the lan9662 PCIe card. >>>>> This card is meant to be used an ethernet switch with 2 x RJ45 >>>>> ports and 2 x 2.5G SFPs. The lan966x SoCs can be used in two >>>>> different ways: >>>>> >>>>> - It can run Linux by itself, on ARM64 cores included in the SoC. >>>>> This use-case of the lan966x is currently being upstreamed, using a >>>>> traditional Device Tree representation of the lan996x HW blocks >>>>> [1] A number of drivers for the different IPs of the SoC have >>>>> already been merged in upstream Linux. >>>>> >>>>> - It can be used as a PCIe endpoint, connected to a separate >>>>> platform that acts as the PCIe root complex. In this case, all the >>>>> devices that are embedded on this SoC are exposed through PCIe BARs >>>>> and the ARM64 cores of the SoC are not used. Since this is a PCIe >>>>> card, it can be plugged on any platform, of any architecture >>>>> supporting PCIe. >>>>> >>>>> The problem that arose is that we want to reuse all the existing OF >>>>> compatible drivers that are used when in SoC mode to instantiate the >>>>> PCI device when in PCIe endpoint mode. >>>>> >>>>> A previous attempt to tackle this problem was made using fwnode [1]. >>>>> However, this proved being way too invasive and it required >>>>> modifications in both subsystems and drivers to support fwnode. >>>>> First series did not lead to a consensus and multiple ideas to >>>>> support this use-case were mentioned (ACPI overlay, fwnode, >>>>> device-tree overlay). Since it only seemed that fwnode was not a >>>>> totally silly idea, we continued on this way. >>>>> >>>>> However, on the series that added fwnode support to the reset >>>>> subsystem, Rob Herring mentioned the fact that OF overlay might >>>>> actually be the best way to probe PCI devices and populate platform >>>>> drivers using this overlay. He also provided a branch containing >>>>> some commits that helped >>>> >>>> I need to go look at the various email threads mentioned above before >>>> I continue reading this patch series. >>>> >>>> I do have serious concerns with this approach. I need to investigate >>>> more fully before I can determine whether the concerns are addressed >>>> sufficiently. >>>> >>>> To give some background to my longstanding response to similar >>>> proposals, here is my old statement from >>>> https://elinux.org/Device_Tree_Reference: >>>> >>>> Overlays >>>> Mainline Linux Support >>>> Run time overlay apply and run time overlay remove from user space >>>> are not supported in the mainline kernel. There are out of tree >>>> patches to implement this feature via an overlay manager. The overlay >>>> manager is used successfully by many users for specific overlays on >>>> specific boards with specific environments and use cases. However, >>>> there are many issues with the Linux kernel overlay implementation >>>> due to incomplete and incorrect code. The overlay manager has not >>>> been accepted in mainline due to these issues. Once these issues are >>>> resolved, it is expected that some method of run time overlay apply >>>> and overlay removal from user space will be supported by the Linux >>>> kernel. >>>> >>>> There is a possibility that overlay apply and overlay remove >>>> support could be phased in slowly, feature by feature, as specific >>>> issues are resolved. >>> >>> Hi Frank, >>> >>> This work uses the kernel space interface (of_overlay_fdt_apply()) >>> and the device tree overlay is builtin the driver. This interface was >>> used until recently by rcu-dcar driver. While the only user (sic), >>> this seems to work pretty well and I was able to use it successfully. >> >> Yes, of_overlay_fdt_apply() was used by one driver. But that driver >> was explicitly recognized as a grandfathered exception, and not an >> example for other users. It was finally removed in 5.18-rc1. > > What API are folks supposed to use exactly? That's the only API to > apply an overlay. Yes, that is the API to designed to be used if overlays are applied after the kernel has booted. > I thought the FPGA mgr code was using it too, but That was my understanding too. > it's not. It doesn't look to me like the upstream code there even > works as nothing applies the overlays AFAICT. If there are no in > kernel users applying overlays, then let's remove the overlay code. I > hear it has lots of problems. I would not object to doing that. But I suspect there are other people who might. I much prefer that overlays be applied before boot. > > I am *way* more comfortable with driver specific applying of overlays > than any generic mechanism. I don't think we'll ever have a generic > mechanism. At least not one that doesn't end up with the same usage > constraints driver specific cases would have. Yes, generic use leads to a lot of the complications of implementing runtime overlays. > > >> You may have used of_overlay_fdt_apply() in a specific use case at >> a specific kernel version, but if you read through the references >> I provided you will find that applying overlays after the kernel >> boots is a fragile endeavor, with expectations of bugs and problems >> being exposed as usage is changed (simple example is that my adding >> some overlay notifier unittests exposed yet another memory leak). > > The exception being specific drivers that are only applying overlays > isolated to their device as this usecase is. The usecase here is > entirely self-contained. The only base tree is only what's needed to > represent the PCI device. This example might be the best example of such a use case. As it is claimed that the drivers for the devices on the board do not access anything on the host other than the pcie slot. > >> The reference that I provided also shows how the overlay code is >> being improved over time. Even with improvements, it will remain >> fragile. >> >>> >>> Moreover, this support targets at using this with PCI devices. This >>> devices are really well contained and do not interfere with other >>> devices. This actually consists in adding a complete subtree into the >>> existing device-tree and thus it limits the interactions between >>> potentially platform provided devices and PCI ones. >> >> Yes, that it is very important that you have described this fact, both >> here and in other emails. Thank you for that information, it does help >> understanding the alternatives. >> >> I've hesitated in recommending a specific solution before better >> understanding the architecture of your pcie board and drivers, but >> I've delayed too long, so I am going to go ahead and mention one >> possibility at the risk of not yet fully understanding the situation. >> >> On the surface, it appears that your need might be well met by having >> a base devicetree that describes all of the pcie nodes, but with each >> node having a status of "disabled" so that they will not be used. >> Have a devicetree overlay describing the pcie card (as you proposed), >> where the overlay also includes a status of "ok" for the pcie node. >> Applying the overlay, with a method of redirecting the target to a >> specific pcie node would change the status of the pcie node to enable >> its use. (You have already proposed a patch to modify of_overlay_fdt_apply() >> to allow a modified target, so not a new concept from me.) My suggestion >> is to apply the overlay devicetree to the base devicetree before the >> combined FDT devicetree is passed to the kernel at boot. The overlay >> apply could be done by several different entities. It could be before >> the bootloader executes, it could be done by the bootloader, it could >> be done by a shim between the bootloader and the kernel. This method >> avoids all of the issues of applying an overlay to a running system >> that I find problematic. It is also a method used by the U-boot >> bootloader, as an example. > > Adding a layer, the solution to all problems... < insert xkcd reference here > :-) > > I don't think that's a workable solution unless all the components are > in one party's control. Given the desire to work on ACPI and DT based > systems, that doesn't sound like the case here. That is the motivation behind my questions of how generic or targeted the use of this one specific card is. A pre-boot shim that discovers the card and applies the overlay could be somewhat generic to ACPI systems. Is the overlay approach also being proposed for DT based systems? > >> The other big issue is mixing ACPI and devicetree on a single system. >> Historically, the Linux devicetree community has not been receptive >> to the ides of that mixture. Your example might be a specific case >> where the two can be isolated from each other, or maybe not. (For >> disclosure, I am essentially ACPI ignorant.) I suspect that mixing >> ACPI and devicetree is a recipe for disaster in the general case. > > The idea here is what is described by ACPI and DT are disjoint which I > think we can enforce. Enforcement comes from fwnode assuming it has > either an ACPI or a DT handle, but not both. I thought the intent was to use DT API drivers, not fwnode API drivers. And I thought the card was not to be described by ACPI, so there would not be any ACPI fwnode info for the card _and_ fwnode is somewhat irrelevant since the drivers are using the DT API. So the enforcement you mention could be implemented by the overlay apply code verifying that there is no ACPI fwnode for any DT node in the overlay. So not an _assumption_, but a testable rule. That is a long way of essentially agreeing with you. > >> More to come later as I finish reading through the various threads. > > There is also the Xilinx folks wanting to support their PCI FPGA card > with DT for the FPGA contents on both ACPI and DT systems. Is that the XRT Alveo driver infrastructure thread? I was not cc:ed on that thread, and just recently stumbled upon it. Yet another thread that is on my to-read list. > > Rob
On Mon, May 9, 2022 at 10:36 PM Frank Rowand <frowand.list@gmail.com> wrote: > > On 5/9/22 12:00, Andy Shevchenko wrote: > > On Mon, May 09, 2022 at 06:09:17PM +0200, Clément Léger wrote: > >> Le Mon, 9 May 2022 10:56:36 -0500, > >> Frank Rowand <frowand.list@gmail.com> a écrit : > > > > ... > > > >>> On the surface, it appears that your need might be well met by having > >>> a base devicetree that describes all of the pcie nodes, but with each > >>> node having a status of "disabled" so that they will not be used. > >>> Have a devicetree overlay describing the pcie card (as you proposed), > >>> where the overlay also includes a status of "ok" for the pcie node. > >>> Applying the overlay, with a method of redirecting the target to a > >>> specific pcie node would change the status of the pcie node to enable > >>> its use. (You have already proposed a patch to modify > >>> of_overlay_fdt_apply() to allow a modified target, so not a new > >>> concept from me.) My suggestion is to apply the overlay devicetree > >>> to the base devicetree before the combined FDT devicetree is passed > >>> to the kernel at boot. The overlay apply could be done by several > >>> different entities. It could be before the bootloader executes, it > >>> could be done by the bootloader, it could be done by a shim between > >>> the bootloader and the kernel. This method avoids all of the issues > >>> of applying an overlay to a running system that I find problematic. > >>> It is also a method used by the U-boot bootloader, as an example. > >> > >> Ok, that is actually possible on a system that is given a device-tree > >> by the bootloader. But on a system that is desrcibed using ACPI (such > >> as the x86), this is much more difficult (at least to my knowledge)... > >> We want this feature to be easy to use for the end user. Adding such > >> configuration which also differs between various architecture is > >> clearly not so easy to setup. > >> > >> Moreover, since the PCI is meant to be "Plug and Play", such > >> configuration would completely break that. If the user switches the > >> PCIe card from one slot to another, the bootloader configuration will > >> need to be modified. This seems a big no way for me (and for the user). > > > > The main problem here is that Linux does not support hotplugging for the > > devices behind non-hotpluggable buses. You need to develop something to > > say that the device tree (in terms of hardware) can morph at run-time > > transparently to the user. I think the closest one is what FPGA does, > > or at least should do. > > That is something I was not aware of yet. Is the card in question a > hotpluggable card? Do the systems that you anticipate plugging the > card into support hotplug? Any PCIe card is potentially hotpluggable (seems nobody actually cares in 90%+ drivers in the Linux kernel). But what I have heard in a thread (not this one IIRC) is that the card may have pluggable modules and it would be nice to change configuration and notify OS somehow. I might be mistaken if it's the case here or not.
Le Mon, 9 May 2022 15:07:18 -0500, Frank Rowand <frowand.list@gmail.com> a écrit : > > Ok, that is actually possible on a system that is given a > > device-tree by the bootloader. But on a system that is desrcibed > > using ACPI (such as the x86), this is much more difficult (at least > > to my knowledge)... We want this feature to be easy to use for the > > end user. Adding such configuration which also differs between > > various architecture is clearly not so easy to setup. > > Are you trying to make your card work on any ACPI based system (x86, > x86-64, etc)? Or do you have a specific model of computer that you > want to make this work on for a specific customer or appliance? There is no particular appliance/architecture targeted. This card should work with any system that can support PCIe. > > If for many arbitrary systems, can you limit it to one architecture > or sub-architecture? Previous answer does rule out this one. > > > > > Moreover, since the PCI is meant to be "Plug and Play", such > > configuration would completely break that. If the user switches the > > PCIe card from one slot to another, the bootloader configuration > > will need to be modified. This seems a big no way for me (and for > > the user). > > Yes. I was envisioning the pre-bootloader, bootloader, or Linux > pre-boot shim dynamically determining the slot containing the card, > and applying the overlay devicetree to the base devicetree, > retargeting the overlay to the proper location, before the Linux boot. Ok, this is however not doable on many already existing platforms since it would require to adapt this system for whatever bootloader is present on the platform. Moreover, AFAIK some platforms bootchain (x86) are hardly modifiable. > > The base devicetree would be for a specific type of machine or family > of machines, just as is the case for all devicetree based systems. > > > > >> > >> The other big issue is mixing ACPI and devicetree on a single > >> system. Historically, the Linux devicetree community has not been > >> receptive to the ides of that mixture. Your example might be a > >> specific case where the two can be isolated from each other, or > >> maybe not. (For disclosure, I am essentially ACPI ignorant.) I > >> suspect that mixing ACPI and devicetree is a recipe for disaster > >> in the general case. > > > > Agreed, on that fact, it did raised some eyebrows, and it was for > > that specific concern that initially, I proposed the fwnode > > solution. Honestly, the fwnode conversion represent a lot of work > > (hundreds of lines easily) + requires a conversion of all the > > subsystem that are not fwnode ready (spoiler: almost all of them > > are not ready). > > > > After implementing Rob's solution, the device-tree overlay really > > seems the cleaner to me and requires much less modifications. > > > >> > >> More to come later as I finish reading through the various > >> threads. > > > > Ok, thanks for your time ! > > Your welcome. I'll keep looking deeper into the previous threads. > > -Frank > > > > > Clément > > > >> > >> -Frank > > > > . >
On Mon, May 09, 2022 at 10:40:12PM +0200, Andy Shevchenko wrote: > > hotpluggable card? Do the systems that you anticipate plugging the > > card into support hotplug? > > Any PCIe card is potentially hotpluggable (seems nobody actually cares > in 90%+ drivers in the Linux kernel). But what I have heard in a > thread (not this one IIRC) is that the card may have pluggable modules > and it would be nice to change configuration and notify OS somehow. I > might be mistaken if it's the case here or not. Well. M.2 for example is not hotpluggable, as are soldered on BGA devices or a lot of not quite PCIe devices that actually sit on CPUs or shipset components. But for all but the last category an upstream bridge could still be hot plugged, so not supporting it in drivers is indeed generally speaking a bad idea.
On Mon, May 09, 2022 at 03:35:45PM -0500, Frank Rowand wrote: > On 5/9/22 13:36, Rob Herring wrote: > > On Mon, May 9, 2022 at 10:56 AM Frank Rowand <frowand.list@gmail.com> wrote: > >> > >> On 5/9/22 07:16, Clément Léger wrote: > >>> Le Fri, 6 May 2022 13:33:22 -0500, > >>> Frank Rowand <frowand.list@gmail.com> a écrit : > >>> > >>>> On 4/27/22 04:44, Clément Léger wrote: > >>>>> This series adds foundation work to support the lan9662 PCIe card. > >>>>> This card is meant to be used an ethernet switch with 2 x RJ45 > >>>>> ports and 2 x 2.5G SFPs. The lan966x SoCs can be used in two > >>>>> different ways: [...] > >>> This work uses the kernel space interface (of_overlay_fdt_apply()) > >>> and the device tree overlay is builtin the driver. This interface was > >>> used until recently by rcu-dcar driver. While the only user (sic), > >>> this seems to work pretty well and I was able to use it successfully. > >> > >> Yes, of_overlay_fdt_apply() was used by one driver. But that driver > >> was explicitly recognized as a grandfathered exception, and not an > >> example for other users. It was finally removed in 5.18-rc1. > > > > What API are folks supposed to use exactly? That's the only API to > > apply an overlay. > > Yes, that is the API to designed to be used if overlays are applied > after the kernel has booted. > > > I thought the FPGA mgr code was using it too, but > > That was my understanding too. > > > it's not. It doesn't look to me like the upstream code there even > > works as nothing applies the overlays AFAICT. If there are no in > > kernel users applying overlays, then let's remove the overlay code. I > > hear it has lots of problems. > > I would not object to doing that. But I suspect there are other people > who might. I much prefer that overlays be applied before boot. Strictly speaking, we don't keep APIs with no users. Perhaps removing or threatening to remove would spur on some work in your overlay issues list. :) > >>> Moreover, this support targets at using this with PCI devices. This > >>> devices are really well contained and do not interfere with other > >>> devices. This actually consists in adding a complete subtree into the > >>> existing device-tree and thus it limits the interactions between > >>> potentially platform provided devices and PCI ones. > >> > >> Yes, that it is very important that you have described this fact, both > >> here and in other emails. Thank you for that information, it does help > >> understanding the alternatives. > >> > >> I've hesitated in recommending a specific solution before better > >> understanding the architecture of your pcie board and drivers, but > >> I've delayed too long, so I am going to go ahead and mention one > >> possibility at the risk of not yet fully understanding the situation. > >> > >> On the surface, it appears that your need might be well met by having > >> a base devicetree that describes all of the pcie nodes, but with each > >> node having a status of "disabled" so that they will not be used. > >> Have a devicetree overlay describing the pcie card (as you proposed), > >> where the overlay also includes a status of "ok" for the pcie node. > >> Applying the overlay, with a method of redirecting the target to a > >> specific pcie node would change the status of the pcie node to enable > >> its use. (You have already proposed a patch to modify of_overlay_fdt_apply() > >> to allow a modified target, so not a new concept from me.) My suggestion > >> is to apply the overlay devicetree to the base devicetree before the > >> combined FDT devicetree is passed to the kernel at boot. The overlay > >> apply could be done by several different entities. It could be before > >> the bootloader executes, it could be done by the bootloader, it could > >> be done by a shim between the bootloader and the kernel. This method > >> avoids all of the issues of applying an overlay to a running system > >> that I find problematic. It is also a method used by the U-boot > >> bootloader, as an example. > > > > Adding a layer, the solution to all problems... > > < insert xkcd reference here > :-) > > > > > I don't think that's a workable solution unless all the components are > > in one party's control. Given the desire to work on ACPI and DT based > > systems, that doesn't sound like the case here. > > That is the motivation behind my questions of how generic or targeted > the use of this one specific card is. > > A pre-boot shim that discovers the card and applies the overlay could > be somewhat generic to ACPI systems. > > Is the overlay approach also being proposed for DT based systems? Yes, the intent is it would work for either. Another usecase I have in mind are USB to serial chips with downstream GPIOs, I2C, SPI, etc. where you want to describe downstream devices. And then you plug in more than 1... > >> The other big issue is mixing ACPI and devicetree on a single system. > >> Historically, the Linux devicetree community has not been receptive > >> to the ides of that mixture. Your example might be a specific case > >> where the two can be isolated from each other, or maybe not. (For > >> disclosure, I am essentially ACPI ignorant.) I suspect that mixing > >> ACPI and devicetree is a recipe for disaster in the general case. > > > > The idea here is what is described by ACPI and DT are disjoint which I > > think we can enforce. Enforcement comes from fwnode assuming it has > > either an ACPI or a DT handle, but not both. > > I thought the intent was to use DT API drivers, not fwnode API drivers. Yes. > And I thought the card was not to be described by ACPI, so there would > not be any ACPI fwnode info for the card _and_ fwnode is somewhat > irrelevant since the drivers are using the DT API. The card would not be described, but the PCI host bridge would likely be and that may be where the conflict is. Though I think PCI hosts are described differently from devices that end up with a fwnode handle in ACPI. My current thinking is the whole PCI bus structure from the card device up the tree needs to be created in the base DT. Maybe we could create just a fake parent for the device and avoid the above issue, but I'd guess that would just create other issues. > So the enforcement > you mention could be implemented by the overlay apply code verifying > that there is no ACPI fwnode for any DT node in the overlay. So not > an _assumption_, but a testable rule. I think we'd need something where ever device fwnode ptrs are getting set. Not entirely sure yet. > That is a long way of essentially agreeing with you. > > > > >> More to come later as I finish reading through the various threads. > > > > There is also the Xilinx folks wanting to support their PCI FPGA card > > with DT for the FPGA contents on both ACPI and DT systems. > > Is that the XRT Alveo driver infrastructure thread? I was not cc:ed > on that thread, and just recently stumbled upon it. Yet another > thread that is on my to-read list. Yes, and I've had a call about it in the 'system DT' call. Rob