> On 24/01/17 10:15, Bharat Kumar Gogada wrote: > >> On 21/01/17 11:11, Bharat Kumar Gogada wrote: > >>> - The legacy status register value for particular INTx becomes low > >>> only after DEASSERT_INTx is received. > >>> - Few End Points take time for sending DEASSERT_INTx, checking > >>> legacy status register in while loop causes invoking of EP handler > >>> continuosly until DEASSERT_INTx is received. > >>> > >>> Signed-off-by: Bharat Kumar Gogada <bharatku@xxxxxxxxxx> > >>> --- > >>> drivers/pci/host/pcie-xilinx-nwl.c | 5 +++-- > >>> 1 files changed, 3 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/pci/host/pcie-xilinx-nwl.c > >>> b/drivers/pci/host/pcie-xilinx-nwl.c > >>> index 43eaa4a..c8b5a33 100644 > >>> --- a/drivers/pci/host/pcie-xilinx-nwl.c > >>> +++ b/drivers/pci/host/pcie-xilinx-nwl.c > >>> @@ -342,9 +342,10 @@ static void nwl_pcie_leg_handler(struct > >>> irq_desc > >>> *desc) > >>> > >>> chained_irq_enter(chip, desc); > >>> pcie = irq_desc_get_handler_data(desc); > >>> + status = nwl_bridge_readl(pcie, MSGF_LEG_STATUS) & > >>> + MSGF_LEG_SR_MASKALL; > >>> > >>> - while ((status = nwl_bridge_readl(pcie, MSGF_LEG_STATUS) & > >>> - MSGF_LEG_SR_MASKALL) != 0) { > >>> + if (status != 0) { > >>> for_each_set_bit(bit, &status, INTX_NUM) { > >>> virq = irq_find_mapping(pcie->legacy_irq_domain, > >>> bit + 1); > >>> > >> > >> But even if you only handle the interrupt once, it is still asserted, > >> right? You exit the low-level exception handler, only to take the > >> interrupt immediately again. So what is the gain here? > >> > > Whenever masking of INTx happens (like as in my next patch[PATCH > > 2/4]), the irq line goes low, but status bit will be set until DEASSERT_INTx > comes. > > In this scenario if it is always in while loop, even though line went > > low it will not be seen until DEASSERT_INTx, unnecessarily invoking EP > > driver. so instead of while loop using if condition so that this change in line is > noticed. > > But what guarantees that you will observe this DEASSERT if you return to the > interrupted context early? From what I understand, your interrupt flow is the > following: > > > read status > mask > handler > unmask > > If the EP takes time to send a deassert after the handler has poked it and the > interrupt is still active, then the only thing that this patch buys you is that by > returning to the interrupted context, you waste a lot of cycles, giving the device > a chance to send its deassert. But that's just luck. Plug this on a fast CPU, and > the same issue will resurface. > Agreed, will leave it as it is. Thanks & Regards, Bharat -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html