From: Marc Zyngier <maz@kernel.org> To: Guo Ren <guoren@kernel.org> Cc: "Samuel Holland" <samuel@sholland.org>, "Anup Patel" <anup@brainfault.org>, "Atish Patra" <atish.patra@wdc.com>, "Thomas Gleixner" <tglx@linutronix.de>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Heiko Stübner" <heiko@sntech.de>, "Rob Herring" <robh@kernel.org>, "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org>, "Guo Ren" <guoren@linux.alibaba.com> Subject: Re: [PATCH V4 1/3] irqchip/sifive-plic: Add thead,c900-plic support Date: Tue, 19 Oct 2021 11:18:09 +0100 [thread overview] Message-ID: <8735oxuxlq.wl-maz@kernel.org> (raw) In-Reply-To: <CAJF2gTShT8Tvk0z6B52zKEi0vq_toc-7mAKWFKj3j-zg=OhpYQ@mail.gmail.com> 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.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org> To: Guo Ren <guoren@kernel.org> Cc: "Samuel Holland" <samuel@sholland.org>, "Anup Patel" <anup@brainfault.org>, "Atish Patra" <atish.patra@wdc.com>, "Thomas Gleixner" <tglx@linutronix.de>, "Palmer Dabbelt" <palmer@dabbelt.com>, "Heiko Stübner" <heiko@sntech.de>, "Rob Herring" <robh@kernel.org>, "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>, linux-riscv <linux-riscv@lists.infradead.org>, "Guo Ren" <guoren@linux.alibaba.com> Subject: Re: [PATCH V4 1/3] irqchip/sifive-plic: Add thead,c900-plic support Date: Tue, 19 Oct 2021 11:18:09 +0100 [thread overview] Message-ID: <8735oxuxlq.wl-maz@kernel.org> (raw) In-Reply-To: <CAJF2gTShT8Tvk0z6B52zKEi0vq_toc-7mAKWFKj3j-zg=OhpYQ@mail.gmail.com> 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. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2021-10-19 10:18 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-16 3:21 [PATCH V4 0/3] irqchip: riscv: Add thead,c900-plic support guoren 2021-10-16 3:21 ` guoren 2021-10-16 3:21 ` [PATCH V4 1/3] irqchip/sifive-plic: " guoren 2021-10-16 3:21 ` guoren 2021-10-18 5:17 ` Samuel Holland 2021-10-18 5:17 ` Samuel Holland 2021-10-18 5:40 ` Anup Patel 2021-10-18 5:40 ` Anup Patel 2021-10-18 7:05 ` Guo Ren 2021-10-18 7:05 ` Guo Ren 2021-10-18 7:21 ` Marc Zyngier 2021-10-18 7:21 ` Marc Zyngier 2021-10-19 9:33 ` Guo Ren 2021-10-19 9:33 ` Guo Ren 2021-10-19 10:18 ` Marc Zyngier [this message] 2021-10-19 10:18 ` Marc Zyngier 2021-10-19 13:27 ` Guo Ren 2021-10-19 13:27 ` Guo Ren 2021-10-20 13:34 ` Marc Zyngier 2021-10-20 13:34 ` Marc Zyngier 2021-10-20 14:19 ` Guo Ren 2021-10-20 14:19 ` Guo Ren 2021-10-20 14:59 ` Darius Rad 2021-10-20 14:59 ` Darius Rad 2021-10-20 16:18 ` Anup Patel 2021-10-20 16:18 ` Anup Patel 2021-10-20 18:01 ` Darius Rad 2021-10-20 18:01 ` Darius Rad 2021-10-21 8:47 ` Anup Patel 2021-10-21 8:47 ` Anup Patel 2021-10-20 14:33 ` Anup Patel 2021-10-20 14:33 ` Anup Patel 2021-10-20 15:08 ` Marc Zyngier 2021-10-20 15:08 ` Marc Zyngier 2021-10-20 16:08 ` Anup Patel 2021-10-20 16:08 ` Anup Patel 2021-10-20 16:48 ` Marc Zyngier 2021-10-20 16:48 ` Marc Zyngier 2021-10-21 8:52 ` Anup Patel 2021-10-21 8:52 ` Anup Patel 2021-10-21 1:46 ` Guo Ren 2021-10-21 1:46 ` Guo Ren 2021-10-21 2:00 ` Guo Ren 2021-10-21 2:00 ` Guo Ren 2021-10-21 8:33 ` Marc Zyngier 2021-10-21 8:33 ` Marc Zyngier 2021-10-21 9:43 ` Guo Ren 2021-10-21 9:43 ` Guo Ren 2021-10-16 3:21 ` [PATCH V4 2/3] dt-bindings: update riscv plic compatible string guoren 2021-10-16 3:21 ` guoren 2021-10-16 7:07 ` Andreas Schwab 2021-10-16 7:07 ` Andreas Schwab 2021-10-16 9:16 ` Guo Ren 2021-10-16 9:16 ` Guo Ren 2021-10-16 10:34 ` Heiko Stuebner 2021-10-16 10:34 ` Heiko Stuebner 2021-10-16 12:56 ` Guo Ren 2021-10-16 12:56 ` Guo Ren 2021-10-16 16:31 ` Heiko Stuebner 2021-10-16 16:31 ` Heiko Stuebner 2021-10-20 12:15 ` Guo Ren 2021-10-20 12:15 ` Guo Ren 2021-10-18 12:02 ` Rob Herring 2021-10-18 12:02 ` Rob Herring 2021-10-19 0:55 ` Guo Ren 2021-10-19 0:55 ` Guo Ren 2021-10-16 3:22 ` [PATCH V4 3/3] dt-bindings: vendor-prefixes: add T-Head Semiconductor guoren 2021-10-16 3:22 ` guoren
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=8735oxuxlq.wl-maz@kernel.org \ --to=maz@kernel.org \ --cc=anup@brainfault.org \ --cc=atish.patra@wdc.com \ --cc=guoren@kernel.org \ --cc=guoren@linux.alibaba.com \ --cc=heiko@sntech.de \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=robh@kernel.org \ --cc=samuel@sholland.org \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.