Re: [PATCH 2/4] PCI: mvebu: Implement support for interrupts on emulated bridge

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Aug 18, 2022 at 10:07:37PM +0200, Pali Rohár wrote:
> On Thursday 18 August 2022 21:51:21 Andrew Lunn wrote:
> > > -static irqreturn_t mvebu_pcie_irq_handler(int irq, void *arg)
> > > +static irqreturn_t mvebu_pcie_error_irq_handler(int irq, void *arg)
> > > +{
> > > +	struct mvebu_pcie_port *port = arg;
> > > +	struct device *dev = &port->pcie->pdev->dev;
> > > +	u32 cause, unmask, status;
> > > +
> > > +	cause = mvebu_readl(port, PCIE_INT_CAUSE_OFF);
> > > +	unmask = mvebu_readl(port, PCIE_INT_UNMASK_OFF);
> > > +	status = cause & unmask;
> > > +
> > > +	/* "error" interrupt handler does not process INTX interrupts */
> > > +	status &= ~(PCIE_INT_INTX(0) | PCIE_INT_INTX(1) |
> > > +		    PCIE_INT_INTX(2) | PCIE_INT_INTX(3));
> > 
> > Just for my understanding...
> > 
> > There are two interrupts
> 
> yes
> 
> > but the status information what those
> > interrupts actually mean are all packed into one register?
> 
> yes
> 
> for masking individual interrupt events there is just one shared
> register for both "intx" and "error" interrupt source.
> 
> and also there is also just one shared "cause" register which says which
> individual interrupt events happened.
> 
> > I assume reading the clause register does not clear set bits?
> 
> yes, reading does not clear any interrupt event.
> 
> > Otherwise there
> > would be a race condition.
> 
> > Are these actually level interrupts
> 
> yes
> 
> > and in order to clear them you need to poke some other register?
> 
> to clear individual interrupt event you have to write corresponding 1b
> bit into that cause register.
> 
> so if interrupts events BIT(24), BIT(16) and BIT(17) happened and
> BIT(24), BIT(25), BIT(26), BIT(27) and BIT(16) are unmasked then CPU
> receives two interrupts (one for intx:24-27 and one for err:16). kernel
> will call interrupt handlers for both intx and err (possible also in
> parallel if it unmasked on different CPUs) and each handler just clears
> events which process. So writing BIT(16) into cause register clears only
> event 16 and all other (24-27, 17) are still active. And level interrupt
> (the correct one intx or err) is then triggered again.

Thanks for the explanation.

I don't know enough about PCI to be able to give a meaningful
Reviewed-by, so i will leave that to the PCI maintainer. But the DT
bits look good to me.

     Andrew