On Thu, Apr 26, 2018 at 11:00:52AM +0530, poza@xxxxxxxxxxxxxx wrote: > On 2018-04-23 20:53, Oza Pawandeep wrote: > > This patch set brings in error handling support for DPC > > > > The current implementation of AER and error message broadcasting to the > > EP driver is tightly coupled and limited to AER service driver. > > It is important to factor out broadcasting and other link handling > > callbacks. So that not only when AER gets triggered, but also when DPC > > get > > triggered (for e.g. ERR_FATAL), callbacks are handled appropriately. > > > > The goal of the patch-set is: > > DPC should handle the error handling and recovery similar to AER, > > because > > finally both are attempting recovery in some or the other way, > > and for that error handling and recovery framework has to be loosely > > coupled. > > ... > Hi Bjorn, > > I know I need to rebase this whole patch-set to 4.17 now. > > But before I do that, can you please help to comment. My overall comment is that I think the series will be simpler and read better if you first change AER to do remove/re-enumerate, before doing anything with DPC. This could be done by extracting just the AER part of "PCI/AER/DPC: Align FATAL error handling for AER and DPC" (i.e., adding pcie_do_fatal_recovery()) and moving that to be the very first patch. It's a small change in terms of code size, but significant to drivers, and it's really the core of the series, so it would be good to clearly establish the policy of: ERR_NONFATAL => call driver recovery entry points ERR_FATAL => remove and re-enumerate before bringing DPC into the picture. Then the subsequent patches would all be more or less mechanical changes to make DPC follow the same model. Bjorn