RE: [PATCH 4/4] PCI: xilinx-nwl: Add method to setup_platform_service_irq hook

[Bharat Kumar Gogada]

> Subject: Re: [PATCH 4/4] PCI: xilinx-nwl: Add method to
> setup_platform_service_irq hook
> 
> On Fri, Aug 10, 2018 at 09:09:40PM +0530, Bharat Kumar Gogada wrote:
> > Add nwl_setup_service_irqs hook to setup_platform_service_irq IRQs to
> > register platform provided IRQ number to kernel AER service.
> >
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xxxxxxxxxx>
> > ---
> >  drivers/pci/controller/pcie-xilinx-nwl.c |   16 ++++++++++++++++
> >  1 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index fb32840..285647b 100644
> > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/irqchip/chained_irq.h>
> >
> >  #include "../pci.h"
> > +#include "../pcie/portdrv.h"
> >
> >  /* Bridge core config registers */
> >  #define BRCFG_PCIE_RX0			0x00000000
> > @@ -819,6 +820,20 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> >  	return 0;
> >  }
> >
> > +int nwl_setup_service_irqs(struct pci_host_bridge *bridge, int *irqs,
> > +			   int plat_mask)
> > +{
> > +	struct nwl_pcie *pcie;
> > +
> > +	pcie = pci_host_bridge_priv(bridge);
> > +	if (plat_mask & PCIE_PORT_SERVICE_AER) {
> > +		irqs[PCIE_PORT_SERVICE_AER_SHIFT] = pcie->irq_misc;
> > +		plat_mask &= ~(1 << PCIE_PORT_SERVICE_AER_SHIFT);
> > +	}
> 
> If I understand correctly, this ultimately results in pcie->irq_misc being
> hooked up to aer_irq() via the aer_probe() path.  We already have pcie-
> >irq_misc being hooked up to nwl_pcie_misc_handler() via
> nwl_pcie_bridge_init().
> 
> We can't rely on the ordering of the two handlers.  Is it safe if
> nwl_pcie_misc_handler() runs first, followed by aer_irq()?  It looks like
> nwl_pcie_misc_handler() might log messages and clear AER-related errors.  If
> that's the case aer_irq() might not find anything to do.
> 
Hi Bjorn, the nwl_pcie_misc_handler() prints all pcie errors along with AER and then clears 
controller register (MSGF_MISC_STATUS) in which these status bits are set. 
But clearing this controller register will not effect any bits in AER capabilities.
So in our case we need  to clear controller register and also handle AER as per spec.
This will not cause any ordering issue as both paths are accessing different set of registers.

Thanks,
Bharat