Re: [PATCH 2/2] PCI: Add support for PCIe wake interrupt

[Lukas Wunner]

On Tue, Apr 01, 2025 at 10:12:44AM +0530, Krishna Chaitanya Chundru wrote:
> PCIe wake interrupt is needed for bringing back PCIe device state
> from D3cold to D0.
> 
> Implement new functions, of_pci_setup_wake_irq() and
> of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices
> using the Device Tree.
> 
> From the port bus driver call these functions to enable wake support
> for bridges.
[...]
> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -695,6 +695,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
>  	if (type == PCI_EXP_TYPE_RC_EC)
>  		pcie_link_rcec(dev);
>  
> +	status = of_pci_setup_wake_irq(dev);
> +	if (status)
> +		return status;
> +
>  	status = pcie_port_device_register(dev);
>  	if (status)
>  		return status;
> @@ -728,6 +732,8 @@ static void pcie_portdrv_remove(struct pci_dev *dev)
>  		pm_runtime_dont_use_autosuspend(&dev->dev);
>  	}
>  
> +	of_pci_teardown_wake_irq(dev);
> +
>  	pcie_port_device_remove(dev);
>  
>  	pci_disable_device(dev);

Why doesn't the teardown order mirror the probe order, i.e. why is
of_pci_teardown_wake_irq() called *before* pcie_port_device_remove()
instead of after?

(pcie_port_device_remove() is the opposite of pcie_port_device_register().)

Also, why is it safe to bail out of probe on failure of
of_pci_setup_wake_irq() without unwinding whatever pcie_link_rcec()
has done?  I think this needs either an explanation or reordering.

Thanks,

Lukas