Hi Bjorn,
On 2/22/24 2:15 PM, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> Previous Kconfig allowed the possibility of enabling CONFIG_PCIE_DPC
> without CONFIG_PCIE_EDR. The PCI Firmware Spec, r3.3, sec 4.5.1,
> table 4-5, says an ACPI OS that requests control of DPC must also
> advertise support for EDR.
>
> Remove CONFIG_PCIE_EDR and enable the EDR code with CONFIG_PCIE_DPC so that
> enabling DPC also enables EDR for ACPI systems. Since EDR is an ACPI
> feature, build edr.o only when CONFIG_ACPI is enabled. Stubs cover the
> case when DPC is enabled without ACPI.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
> drivers/acpi/pci_root.c | 2 +-
> drivers/pci/pcie/Kconfig | 14 ++++----------
> drivers/pci/pcie/Makefile | 5 ++++-
> drivers/pci/pcie/dpc.c | 10 ----------
> include/linux/pci-acpi.h | 4 ++--
> 5 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index efc292b6214e..bcaf3d3a5e05 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -448,7 +448,7 @@ static u32 calculate_support(void)
> support |= OSC_PCI_ASPM_SUPPORT | OSC_PCI_CLOCK_PM_SUPPORT;
> if (pci_msi_enabled())
> support |= OSC_PCI_MSI_SUPPORT;
> - if (IS_ENABLED(CONFIG_PCIE_EDR))
> + if (IS_ENABLED(CONFIG_PCIE_DPC)) /* DPC => EDR support */
> support |= OSC_PCI_EDR_SUPPORT;
Since EDR internally touches AER registers, I still think we should make sure
OS enables AER support before advertising the EDR support.
>
> return support;
> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> index 8999fcebde6a..21e98289fbe9 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -137,6 +137,10 @@ config PCIE_DPC
> have this capability or you do not want to use this feature,
> it is safe to answer N.
>
> + On ACPI systems, this includes Error Disconnect Recover support,
> + the hybrid model that uses both firmware and OS to implement DPC,
> + as specified in the PCI Firmware Specification r3.3.
Nit: Include some section reference?
> +
> config PCIE_PTM
> bool "PCI Express Precision Time Measurement support"
> help
> @@ -145,13 +149,3 @@ config PCIE_PTM
>
> This is only useful if you have devices that support PTM, but it
> is safe to enable even if you don't.
> -
> -config PCIE_EDR
> - bool "PCI Express Error Disconnect Recover support"
> - depends on PCIE_DPC && ACPI
> - help
> - This option adds Error Disconnect Recover support as specified
> - in the Downstream Port Containment Related Enhancements ECN to
> - the PCI Firmware Specification r3.2. Enable this if you want to
> - support hybrid DPC model which uses both firmware and OS to
> - implement DPC.
> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index 8de4ed5f98f1..72657f780c33 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -12,4 +12,7 @@ obj-$(CONFIG_PCIEAER_INJECT) += aer_inject.o
> obj-$(CONFIG_PCIE_PME) += pme.o
> obj-$(CONFIG_PCIE_DPC) += dpc.o
> obj-$(CONFIG_PCIE_PTM) += ptm.o
> -obj-$(CONFIG_PCIE_EDR) += edr.o
> +
> +ifdef CONFIG_ACPI
> +obj-$(CONFIG_PCIE_DPC) += edr.o
> +endif
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 94111e438241..0aa79581250b 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -102,19 +102,9 @@ static bool dpc_completed(struct pci_dev *pdev)
> */
> bool pci_dpc_recovered(struct pci_dev *pdev)
> {
> - struct pci_host_bridge *host;
> -
> if (!pdev->dpc_cap)
> return false;
>
> - /*
> - * Synchronization between hotplug and DPC is not supported
> - * if DPC is owned by firmware and EDR is not enabled.
> - */
> - host = pci_find_host_bridge(pdev->bus);
> - if (!host->native_dpc && !IS_ENABLED(CONFIG_PCIE_EDR))
> - return false;
> -
> /*
> * Need a timeout in case DPC never completes due to failure of
> * dpc_wait_rp_inactive(). The spec doesn't mandate a time limit,
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 078225b514d4..92e196ba0249 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -122,13 +122,13 @@ extern const guid_t pci_acpi_dsm_guid;
> #define DSM_PCI_POWER_ON_RESET_DELAY 0x08
> #define DSM_PCI_DEVICE_READINESS_DURATIONS 0x09
>
> -#ifdef CONFIG_PCIE_EDR
> +#ifdef CONFIG_PCIE_DPC
> void pci_acpi_add_edr_notifier(struct pci_dev *pdev);
> void pci_acpi_remove_edr_notifier(struct pci_dev *pdev);
> #else
> static inline void pci_acpi_add_edr_notifier(struct pci_dev *pdev) { }
> static inline void pci_acpi_remove_edr_notifier(struct pci_dev *pdev) { }
> -#endif /* CONFIG_PCIE_EDR */
> +#endif /* CONFIG_PCIE_DPC */
>
> int pci_acpi_set_companion_lookup_hook(struct acpi_device *(*func)(struct pci_dev *));
> void pci_acpi_clear_companion_lookup_hook(void);
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
