Re: [PATCH v2 3/6] PCI/EDR: Remove CONFIG_PCIE_EDR

[Yazen Ghannam]

On Tue, Oct 20, 2026 at 11:19:20AM -0700, Kuppuswamy Sathyanarayanan wrote:
> Error Disconnect Recover (EDR) is the ACPI half of the hybrid DPC model,
> where firmware owns DPC and notifies the OS of containment events.
> 
> CONFIG_PCIE_EDR made EDR optional on top of CONFIG_PCIE_DPC.  This adds
> another knob to the decision to request DPC control in _OSC.  The OS
> requests DPC control only if it also supports EDR, so with
> CONFIG_PCIE_EDR=n it never requests DPC control.  There is little reason
> to build DPC without EDR on ACPI systems.
> 
> Remove the option and build edr.c whenever CONFIG_PCIE_DPC and
> CONFIG_ACPI are both enabled.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> ---
>  drivers/acpi/pci_root.c   | 6 +++---
>  drivers/pci/pcie/Kconfig  | 9 ---------
>  drivers/pci/pcie/Makefile | 5 ++++-
>  drivers/pci/pcie/dpc.c    | 5 +++--
>  include/linux/pci-acpi.h  | 4 ++--
>  5 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index a1a9c06012f8..b0e0b33e3254 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -438,7 +438,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))
>  		support |= OSC_PCI_EDR_SUPPORT;
>  
>  	return support;
> @@ -515,9 +515,9 @@ static u32 calculate_control(void)
>  	 * Per the Downstream Port Containment Related Enhancements ECN to
>  	 * the PCI Firmware Spec, r3.2, sec 4.5.1, table 4-5,
>  	 * OSC_PCI_EXPRESS_DPC_CONTROL indicates the OS supports both DPC
> -	 * and EDR.
> +	 * and EDR.  EDR is always built with DPC on ACPI systems.
>  	 */
> -	if (IS_ENABLED(CONFIG_PCIE_DPC) && IS_ENABLED(CONFIG_PCIE_EDR))
> +	if (IS_ENABLED(CONFIG_PCIE_DPC))
>  		control |= OSC_PCI_EXPRESS_DPC_CONTROL;
>  
>  	return control;
> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> index 207c2deae35f..de4ea33b4524 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -137,12 +137,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.

What about keeping 'PCIE_EDR' and using it like 'PCIE_PME'?

IOW, remove the user-visible parts have it default to (PCIE_DPC &&
ACPI)?

I understand EDR as its own feature on top of DPC. So I think there's
value in having the EDR bits clearly distinct from the DPC bits.

Also, it would reduce the changes in this patch.

> diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
> index b0b43a18c304..46b24311f659 100644
> --- a/drivers/pci/pcie/Makefile
> +++ b/drivers/pci/pcie/Makefile
> @@ -13,4 +13,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 104ff2b91f1f..71adc0e94bca 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -109,10 +109,11 @@ bool pci_dpc_recovered(struct pci_dev *pdev)
>  
>  	/*
>  	 * Synchronization between hotplug and DPC is not supported
> -	 * if DPC is owned by firmware and EDR is not enabled.
> +	 * if DPC is owned by firmware and there is no EDR, i.e., on
> +	 * non-ACPI systems.
>  	 */
>  	host = pci_find_host_bridge(pdev->bus);
> -	if (!host->native_dpc && !IS_ENABLED(CONFIG_PCIE_EDR))
> +	if (!host->native_dpc && !IS_ENABLED(CONFIG_ACPI))
>  		return false;
>  

This seems like dead code, and it was deleted in an older version.

The conditions seem to be mutually exclusive.

Maybe it should just be removed in a pre-patch?

Thanks,
Yazen