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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

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