Re: [PATCHv3 10/10] PCI: Make link active reporting detection generic

[Sinan Kaya <okaya@kernel.org>]

On 9/19/2018 12:42 PM, Sinan Kaya wrote:
> On 9/18/2018 7:57 PM, Keith Busch wrote:
>> +++ b/drivers/pci/pcie/dpc.c
>> @@ -93,10 +93,12 @@ static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev)
>>       pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
>>                     PCI_EXP_DPC_STATUS_TRIGGER);
>> +    if (!pcie_wait_for_link(pdev, true))
>> +        return PCI_ERS_RESULT_DISCONNECT;
>> +
> 
> We are already doing this in err.c in pcie_do_fatal_recovery() as follows.
> 
>      result = reset_link(udev, service);
>      ...
>      if (result == PCI_ERS_RESULT_RECOVERED) {
>          if (pcie_wait_for_link(udev, true))
>              pci_rescan_bus(udev->bus);
>          pci_info(dev, "Device recovery from fatal error successful\n");
>      }
> 
> This would be an unnecessary duplication.
> 
> You may want to move this piece into err.c and change the return code to 
> PCI_ERS_RESULT_DISCONNECT
> right after return from reset_link().
> 
>>       return PCI_ERS_RESULT_RECOVERED;
>>   }
>> -
> 

Nevermind. I just realized that you unified error recovery handlers into
pcie_do_recovery().

Please disregard this comment.