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

[Krishna Chaitanya Chundru]

On 4/1/2025 12:31 PM, Lukas Wunner wrote:
> 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?
ack, in the next patch I will move teardown after
pcie_port_device_remove()
> (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.
if there is a failure in port_device_register also we are not unwinding
so I taught it is already taken care. Looks like it is not.

In the next patch I will move of_pci_setup_wake_irq() above 
pcie_link_rcec() to avoid all this.

- Krishna Chaitanya.

> Thanks,
>
> Lukas