On 1/12/2024 10:28 PM, Bjorn Helgaas wrote:
External email: Use caution opening links or attachments On Wed, Jan 10, 2024 at 08:37:25AM +0530, Vidya Sagar wrote:Add support for "preserve-boot-config" property that can be used to selectively (i.e. per host bridge) instruct the kernel to preserve the boot time configuration done by the platform firmware. Reported-by: kernel test robot <lkp@xxxxxxxxx> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx> --- V2: * Addressed issues reported by kernel test robot <lkp@xxxxxxxxx> drivers/pci/controller/pci-host-common.c | 5 ++++- drivers/pci/of.c | 18 ++++++++++++++++++ drivers/pci/probe.c | 2 +- include/linux/of_pci.h | 6 ++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c index 6be3266cd7b5..d3475dc9ec44 100644 --- a/drivers/pci/controller/pci-host-common.c +++ b/drivers/pci/controller/pci-host-common.c @@ -68,13 +68,16 @@ int pci_host_common_probe(struct platform_device *pdev) of_pci_check_probe_only(); + bridge->preserve_config = + of_pci_check_preserve_boot_config(dev->of_node);Thanks for leveraging the existing "preserve_config" support for the ACPI _DSM. Is pci_host_common_probe() the best place for this? I think there are many DT platform drivers that do not use pci_host_common_probe(), so I wonder if there's a more generic place to put this.
My understanding is that pci_host_common_probe() is mainly used in systems where the firmware would have taken care of all the platform specific initialization and giving the ECAM and 'ranges' info through DT for the Linux kernel to go ahead and perform the enumeration. This is similar to ACPI way of handing over the system from firmware to the OS. If PCIe controllers are getting initialized in the kernel itself, then pci_host_probe() is called directly from the respective host controller drivers which is the case with all the DesignWare based implementations including Tegra194 and Tegra234. In those systems, since the controllers themselves have come up and gotten initialized in the kernel, resource assignment has to happen anyway.
I see Rob's concern about adding "preserve-boot-config" vs extending "linux,pci-probe-only" and I don't really have an opinion on that, although I do think the "pci-probe-only" name is not as descriptive as it could be. I guess somebody will argue that "preserve_config" could be more descriptive, too :)
Honestly I would have liked to repurpose of_pci_check_probe_only() API to look for "preserve-boot-config" in the respective PCIe controller's DT node and not "linux,pci-probe-only" in the chosen entry, had it not for the single usage of of_pci_check_probe_only() in arch/powerpc /platforms/pseries/setup.c file. Also FWIW, "linux,pci-probe-only" is not documented anywhere. Since there is at least one user for of_pci_check_probe_only(), and combining with the fact that the scope where "linux,pci-probe-only" and "preserve-boot-config" are used (i.e. chosen entry Vs individual PCIe controller node), I prefer to have it as a separate option. Rob, please let me know if you have any strong objections to that?
Bjorn
