Messages in this thread | | | Date | Tue, 19 Oct 2021 11:18:09 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH V4 1/3] irqchip/sifive-plic: Add thead,c900-plic support |
| |
On Tue, 19 Oct 2021 10:33:49 +0100, Guo Ren <guoren@kernel.org> wrote:
> > If you have an 'automask' behavior and yet the HW doesn't record this > > in a separate bit, then you need to track this by yourself in the > > irq_eoi() callback instead. I guess that you would skip the write to > > the CLAIM register in this case, though I have no idea whether this > > breaks > > the HW interrupt state or not. > The problem is when enable bit is 0 for that irq_number, > "writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM)" wouldn't affect > the hw state machine. Then this irq would enter in ack state and no > continues irqs could come in.
Really? This means that you cannot mask an interrupt while it is being handled? How great...
> > > > There is an example of this in the Apple AIC driver. > Thx for the tip, I think your suggestion is: > +++ b/drivers/irqchip/irq-sifive-plic.c > @@ -163,7 +163,12 @@ static void plic_irq_eoi(struct irq_data *d) > { > struct plic_handler *handler = this_cpu_ptr(&plic_handlers); > > - writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM); > + if (irqd_irq_masked(d)) { > + plic_irq_unmask(d); > + writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM); > + plic_irq_mask(d);
This looks pretty dodgy. You are relying on interrupts being globally masked on the CPU, I guess. It probably works today, but man, what a terrible HW implementation.
You'll definitely have to move this into a c900-specific callback.
M.
-- Without deviation from the norm, progress is not possible.
| |