On Thu, Oct 21, 2021 at 10:06:11PM +0530, Naveen Naidu wrote:
> On 20/10, Bjorn Helgaas wrote:
> > On Tue, Oct 05, 2021 at 10:48:11PM +0530, Naveen Naidu wrote:
> > > dpc_process_error() clears both AER fatal and non fatal status
> > > registers. Instead of clearing each status registers via a different
> > > function call use pci_aer_clear_status().
> > >
> > > This helps clean up the code a bit.
> > >
> > > Signed-off-by: Naveen Naidu <naveennaidu479@xxxxxxxxx>
> > > ---
> > > drivers/pci/pcie/dpc.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> > > index df3f3a10f8bc..faf4a1e77fab 100644
> > > --- a/drivers/pci/pcie/dpc.c
> > > +++ b/drivers/pci/pcie/dpc.c
> > > @@ -288,8 +288,7 @@ void dpc_process_error(struct pci_dev *pdev)
> > > dpc_get_aer_uncorrect_severity(pdev, &info) &&
> > > aer_get_device_error_info(pdev, &info)) {
> > > aer_print_error(pdev, &info);
> > > - pci_aer_clear_nonfatal_status(pdev);
> > > - pci_aer_clear_fatal_status(pdev);
> > > + pci_aer_clear_status(pdev);
> >
> > The commit log suggests that this is a simple cleanup that doesn't
> > change any behavior, but that's not quite true:
> >
> > - The new code would clear PCI_ERR_ROOT_STATUS, but the old code
> > does not.
> >
> > - The old code masks the status bits with the severity bits before
> > clearing, but the new code does not.
> >
> > The commit log needs to show why these changes are what we want.
> >
>
> Reading through the code again, I realize how wrong(stupid) I was when
> making this patch. I was thinking that:
>
> pci_aer_clear_status() = pci_aer_clear_fatal_status() + pci_aer_clear_nonfatal_status()
>
> Now I understand, that it is not at all the case. I apologize for the
> mistake. I'll make sure to be meticulous while reading functions and not
> just assume their behaviour just from their function names.
No problem, one could argue that the collection of pci_aer_clear_*()
functions that do slightly different things is itself a defect.
