Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Bharat Kumar Gogada <bharatku@xilinx.com>
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bhelgaas@google.com" <bhelgaas@google.com>,
        Ravikiran Gummaluri <rgummal@xilinx.com>
Subject: RE: [PATCH 4/4] PCI: xilinx-nwl: Add method to
 setup_platform_service_irq hook
Thread-Topic: [PATCH 4/4] PCI: xilinx-nwl: Add method to
 setup_platform_service_irq hook
Thread-Index: AQHUMMEvYQXdyRdHrUqZcZ0+kpU/maTgSo2AgAM9QmA=
Date:   Thu, 6 Sep 2018 15:27:40 +0000
Message-ID: <BN6PR02MB2772814D31D45FB093183458A5010@BN6PR02MB2772.namprd02.prod.outlook.com>
References: <1533915580-31805-1-git-send-email-bharat.kumar.gogada@xilinx.com>
 <1533915580-31805-5-git-send-email-bharat.kumar.gogada@xilinx.com>
 <20180904134859.GC107892@bhelgaas-glaptop.roam.corp.google.com>
In-Reply-To: <20180904134859.GC107892@bhelgaas-glaptop.roam.corp.google.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: xilinx.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: e79b5c5e-e2b9-4b4b-aa80-08d6140d4879
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Sep 2018 15:27:40.0875
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR02MB2180
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> Subject: Re: [PATCH 4/4] PCI: xilinx-nwl: Add method to
> setup_platform_service_irq hook
>=20
> 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@xilinx.com>
> > ---
> >  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 =3D pci_host_bridge_priv(bridge);
> > +	if (plat_mask & PCIE_PORT_SERVICE_AER) {
> > +		irqs[PCIE_PORT_SERVICE_AER_SHIFT] =3D pcie->irq_misc;
> > +		plat_mask &=3D ~(1 << PCIE_PORT_SERVICE_AER_SHIFT);
> > +	}
>=20
> If I understand correctly, this ultimately results in pcie->irq_misc bein=
g
> 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().
>=20
> 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.
>=20
Hi Bjorn, the nwl_pcie_misc_handler() prints all pcie errors along with AER=
 and then clears=20
controller register (MSGF_MISC_STATUS) in which these status bits are set.=
=20
But clearing this controller register will not effect any bits in AER capab=
ilities.
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 differen=
t set of registers.

Thanks,
Bharat