[PATCH v6 0/4] Add UltraRISC DP1000 PLIC support

Lucas Zampieri posted 4 patches 3 months, 2 weeks ago
Only 3 patches received!
There is a newer version of this series
.../sifive,plic-1.0.0.yaml                    |   3 +
.../devicetree/bindings/vendor-prefixes.yaml  |   2 +
drivers/irqchip/irq-sifive-plic.c             | 126 +++++++++++++++++--
3 files changed, 118 insertions(+), 10 deletions(-)
[PATCH v6 0/4] Add UltraRISC DP1000 PLIC support
Posted by Lucas Zampieri 3 months, 2 weeks ago
This series adds support for the PLIC implementation in the UltraRISC
DP1000 SoC. The UR-CP100 cores used in the DP1000 have a hardware bug in
their PLIC claim register where reading it while multiple interrupts are
pending can return the wrong interrupt ID. The workaround temporarily
disables all interrupts except the first pending one before reading the
claim register, then restores the previous state.

The driver matches on "ultrarisc,cp100-plic" (CPU core compatible), allowing
the quirk to apply to all SoCs using UR-CP100 cores (currently DP1000,
potentially future SoCs).

Charles Mirabile (3):
  dt-bindings: interrupt-controller: add UltraRISC DP1000 PLIC
  irqchip/plic: enable optimization of interrupt enable state
  irqchip/plic: add support for UltraRISC DP1000 PLIC

Lucas Zampieri (1):
  dt-bindings: vendor-prefixes: add UltraRISC

Changes in v6:
- Split enable_save optimization into separate patch 0003
- 0003: New patch - Optimize interrupt enable state tracking by maintaining enable_save
  during normal operation instead of only during suspend/resume
- 0004: Use existing enable_save[] instead of reading enable registers before workaround
- 0004: Return iso_mask from cp100_isolate_pending_irq to use in restore logic
- 0004: Skip writing enable masks that haven't changed during isolation
- 0004: Skip restoring enable masks that haven't changed after claim
- 0004: Skip checking groups with no enabled interrupts in cp100_isolate_pending_irq
- 0004: Updated commit message to clarify dependency on enable_save optimization

Changes in v5:
- 0004: Added brackets around conditional in cp100_isolate_pending_irq (feedback from Thomas Gleixner)
- 0004: Reordered variables in reverse fir tree order in cp100_get_hwirq (feedback from Thomas Gleixner)
- 0004: Replaced raw_spin_lock/unlock with guard(raw_spinlock) (feedback from Thomas Gleixner)
- 0004: Added newline between variable declaration and code in plic_probe (feedback from Thomas Gleixner)
- 0004: Extended generic_handle_domain_irq call to single line (feedback from Thomas Gleixner)

Changes in v4:
- 0002: Simplified commit message to focus on hardware bug (feedback from Conor Dooley)
- 0002: Added Conor's Acked-by
- 0004: Renamed PLIC_QUIRK_CLAIM_REGISTER to PLIC_QUIRK_CP100_CLAIM_REGISTER_ERRATUM
  to be more specific (feedback from Samuel Holland)
- 0004: Added Samuel's Acked-by

Changes in v3:
- 0002: Updated commit message to clarify that DP1000 is an SoC and CP100
  is a core (feedback from Conor Dooley)
- 0004: Renamed dp1000_* functions to cp100_* and updated commit message to
  clarify the hardware bug is in the UR-CP100 core implementation, not
  specific to the DP1000 SoC
- 0004: Moved quirk check out of hot interrupt path by creating separate
  plic_handle_irq_cp100() function and selecting handler at probe time
- 0004: Use existing handler->enable_save[] array instead of stack allocation
- 0004: Use readl_relaxed()/writel_relaxed() for better performance

Changes in v2:
- 0002: Changed compatible string pattern to SoC+core: ultrarisc,dp1000-plic
  with ultrarisc,cp100-plic fallback (suggested by Krzysztof and Vivian)
- 0004: Driver now matches on ultrarisc,cp100-plic (core) instead of dp1000 (SoC)
- All patches: Added submitter Signed-off-by to complete DCO chain

 .../sifive,plic-1.0.0.yaml                    |   3 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/irqchip/irq-sifive-plic.c             | 126 +++++++++++++++++--
 3 files changed, 118 insertions(+), 10 deletions(-)

--
2.51.0
Re: [PATCH v6 0/4] Add UltraRISC DP1000 PLIC support
Posted by Thomas Gleixner 3 months, 2 weeks ago
On Thu, Oct 23 2025 at 15:00, Lucas Zampieri wrote:
> This series adds support for the PLIC implementation in the UltraRISC
> DP1000 SoC. The UR-CP100 cores used in the DP1000 have a hardware bug in
> their PLIC claim register where reading it while multiple interrupts are
> pending can return the wrong interrupt ID. The workaround temporarily
> disables all interrupts except the first pending one before reading the
> claim register, then restores the previous state.
>
> The driver matches on "ultrarisc,cp100-plic" (CPU core compatible), allowing
> the quirk to apply to all SoCs using UR-CP100 cores (currently DP1000,
> potentially future SoCs).
>
> Charles Mirabile (3):
>   dt-bindings: interrupt-controller: add UltraRISC DP1000 PLIC
>   irqchip/plic: enable optimization of interrupt enable state

    That one never showed up. Neither in my inbox nor on lore
Re: [PATCH v6 0/4] Add UltraRISC DP1000 PLIC support
Posted by Charles Mirabile 3 months, 2 weeks ago
Hi Thomas—

On Thu, Oct 23, 2025 at 09:29:44PM +0200, Thomas Gleixner wrote:
> On Thu, Oct 23 2025 at 15:00, Lucas Zampieri wrote:
> > This series adds support for the PLIC implementation in the UltraRISC
> > DP1000 SoC. The UR-CP100 cores used in the DP1000 have a hardware bug in
> > their PLIC claim register where reading it while multiple interrupts are
> > pending can return the wrong interrupt ID. The workaround temporarily
> > disables all interrupts except the first pending one before reading the
> > claim register, then restores the previous state.
> >
> > The driver matches on "ultrarisc,cp100-plic" (CPU core compatible), allowing
> > the quirk to apply to all SoCs using UR-CP100 cores (currently DP1000,
> > potentially future SoCs).
> >
> > Charles Mirabile (3):
> >   dt-bindings: interrupt-controller: add UltraRISC DP1000 PLIC
> >   irqchip/plic: enable optimization of interrupt enable state
> 
>     That one never showed up. Neither in my inbox nor on lore

Looks like the CC list was missing somehow from that patch—I didn't notice because I got it in my inbox because of my Signed-off-by.

The indexing on the patches was slightly wrong anyways, so we will resend tomorrow. Sorry for the noise.

I have attached it here in case you want to take a look.

> 
-- >8 --
From: Charles Mirabile <cmirabil@redhat.com>
Subject: [PATCH v6 3/4] irqchip/plic: enable optimization of interrupt enable state

Optimize the PLIC driver by maintaining the interrupt enable state in
the handler's enable_save array during normal operation rather than only
during suspend/resume. This eliminates the need to read enable registers
during suspend and makes the enable state immediately available for
other optimizations.

Modify __plic_toggle() to take a handler pointer instead of enable_base,
allowing it to update both the hardware registers and the cached
enable_save state atomically within the existing enable_lock protection.

Remove the suspend-time enable register reading since enable_save now
always reflects the current state.

Signed-off-by: Charles Mirabile <cmirabil@redhat.com>

---
 drivers/irqchip/irq-sifive-plic.c | 36 +++++++++++--------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index cbd7697bc1481..d518a8b468742 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -94,15 +94,22 @@ static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
 
 static int plic_irq_set_type(struct irq_data *d, unsigned int type);
 
-static void __plic_toggle(void __iomem *enable_base, int hwirq, int enable)
+static void __plic_toggle(struct plic_handler *handler, int hwirq, int enable)
 {
-	u32 __iomem *reg = enable_base + (hwirq / 32) * sizeof(u32);
+	u32 __iomem *base = handler->enable_base;
 	u32 hwirq_mask = 1 << (hwirq % 32);
+	int group = hwirq / 32;
+	u32 value;
+
+	value = readl(base + group);
 
 	if (enable)
-		writel(readl(reg) | hwirq_mask, reg);
+		value |= hwirq_mask;
 	else
-		writel(readl(reg) & ~hwirq_mask, reg);
+		value &= ~hwirq_mask;
+
+	handler->enable_save[group] = value;
+	writel(value, base + group);
 }
 
 static void plic_toggle(struct plic_handler *handler, int hwirq, int enable)
@@ -110,7 +117,7 @@ static void plic_toggle(struct plic_handler *handler, int hwirq, int enable)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&handler->enable_lock, flags);
-	__plic_toggle(handler->enable_base, hwirq, enable);
+	__plic_toggle(handler, hwirq, enable);
 	raw_spin_unlock_irqrestore(&handler->enable_lock, flags);
 }
 
@@ -247,33 +254,16 @@ static int plic_irq_set_type(struct irq_data *d, unsigned int type)
 
 static int plic_irq_suspend(void)
 {
-	unsigned int i, cpu;
-	unsigned long flags;
-	u32 __iomem *reg;
 	struct plic_priv *priv;
 
 	priv = per_cpu_ptr(&plic_handlers, smp_processor_id())->priv;
 
 	/* irq ID 0 is reserved */
-	for (i = 1; i < priv->nr_irqs; i++) {
+	for (unsigned int i = 1; i < priv->nr_irqs; i++) {
 		__assign_bit(i, priv->prio_save,
 			     readl(priv->regs + PRIORITY_BASE + i * PRIORITY_PER_ID));
 	}
 
-	for_each_present_cpu(cpu) {
-		struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
-
-		if (!handler->present)
-			continue;
-
-		raw_spin_lock_irqsave(&handler->enable_lock, flags);
-		for (i = 0; i < DIV_ROUND_UP(priv->nr_irqs, 32); i++) {
-			reg = handler->enable_base + i * sizeof(u32);
-			handler->enable_save[i] = readl(reg);
-		}
-		raw_spin_unlock_irqrestore(&handler->enable_lock, flags);
-	}
-
 	return 0;
 }

--
2.51.0


Re: [PATCH v6 0/4] Add UltraRISC DP1000 PLIC support
Posted by Lucas Zampieri 3 months, 2 weeks ago
Hi Thomas and Charles,

Yes, missed the cc list to on that one, sending the v6 series again
with the correct headers.
Sorry about that.

Lucas Zampieri
Platform Enablement Team

On Thu, Oct 23, 2025 at 9:17 PM Charles Mirabile <cmirabil@redhat.com> wrote:
>
> Hi Thomas—
>
> On Thu, Oct 23, 2025 at 09:29:44PM +0200, Thomas Gleixner wrote:
> > On Thu, Oct 23 2025 at 15:00, Lucas Zampieri wrote:
> > > This series adds support for the PLIC implementation in the UltraRISC
> > > DP1000 SoC. The UR-CP100 cores used in the DP1000 have a hardware bug in
> > > their PLIC claim register where reading it while multiple interrupts are
> > > pending can return the wrong interrupt ID. The workaround temporarily
> > > disables all interrupts except the first pending one before reading the
> > > claim register, then restores the previous state.
> > >
> > > The driver matches on "ultrarisc,cp100-plic" (CPU core compatible), allowing
> > > the quirk to apply to all SoCs using UR-CP100 cores (currently DP1000,
> > > potentially future SoCs).
> > >
> > > Charles Mirabile (3):
> > >   dt-bindings: interrupt-controller: add UltraRISC DP1000 PLIC
> > >   irqchip/plic: enable optimization of interrupt enable state
> >
> >     That one never showed up. Neither in my inbox nor on lore
>
> Looks like the CC list was missing somehow from that patch—I didn't notice because I got it in my inbox because of my Signed-off-by.
>
> The indexing on the patches was slightly wrong anyways, so we will resend tomorrow. Sorry for the noise.
>
> I have attached it here in case you want to take a look.
>
> >
> -- >8 --
> From: Charles Mirabile <cmirabil@redhat.com>
> Subject: [PATCH v6 3/4] irqchip/plic: enable optimization of interrupt enable state
>
> Optimize the PLIC driver by maintaining the interrupt enable state in
> the handler's enable_save array during normal operation rather than only
> during suspend/resume. This eliminates the need to read enable registers
> during suspend and makes the enable state immediately available for
> other optimizations.
>
> Modify __plic_toggle() to take a handler pointer instead of enable_base,
> allowing it to update both the hardware registers and the cached
> enable_save state atomically within the existing enable_lock protection.
>
> Remove the suspend-time enable register reading since enable_save now
> always reflects the current state.
>
> Signed-off-by: Charles Mirabile <cmirabil@redhat.com>
>
> ---
>  drivers/irqchip/irq-sifive-plic.c | 36 +++++++++++--------------------
>  1 file changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index cbd7697bc1481..d518a8b468742 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -94,15 +94,22 @@ static DEFINE_PER_CPU(struct plic_handler, plic_handlers);
>
>  static int plic_irq_set_type(struct irq_data *d, unsigned int type);
>
> -static void __plic_toggle(void __iomem *enable_base, int hwirq, int enable)
> +static void __plic_toggle(struct plic_handler *handler, int hwirq, int enable)
>  {
> -       u32 __iomem *reg = enable_base + (hwirq / 32) * sizeof(u32);
> +       u32 __iomem *base = handler->enable_base;
>         u32 hwirq_mask = 1 << (hwirq % 32);
> +       int group = hwirq / 32;
> +       u32 value;
> +
> +       value = readl(base + group);
>
>         if (enable)
> -               writel(readl(reg) | hwirq_mask, reg);
> +               value |= hwirq_mask;
>         else
> -               writel(readl(reg) & ~hwirq_mask, reg);
> +               value &= ~hwirq_mask;
> +
> +       handler->enable_save[group] = value;
> +       writel(value, base + group);
>  }
>
>  static void plic_toggle(struct plic_handler *handler, int hwirq, int enable)
> @@ -110,7 +117,7 @@ static void plic_toggle(struct plic_handler *handler, int hwirq, int enable)
>         unsigned long flags;
>
>         raw_spin_lock_irqsave(&handler->enable_lock, flags);
> -       __plic_toggle(handler->enable_base, hwirq, enable);
> +       __plic_toggle(handler, hwirq, enable);
>         raw_spin_unlock_irqrestore(&handler->enable_lock, flags);
>  }
>
> @@ -247,33 +254,16 @@ static int plic_irq_set_type(struct irq_data *d, unsigned int type)
>
>  static int plic_irq_suspend(void)
>  {
> -       unsigned int i, cpu;
> -       unsigned long flags;
> -       u32 __iomem *reg;
>         struct plic_priv *priv;
>
>         priv = per_cpu_ptr(&plic_handlers, smp_processor_id())->priv;
>
>         /* irq ID 0 is reserved */
> -       for (i = 1; i < priv->nr_irqs; i++) {
> +       for (unsigned int i = 1; i < priv->nr_irqs; i++) {
>                 __assign_bit(i, priv->prio_save,
>                              readl(priv->regs + PRIORITY_BASE + i * PRIORITY_PER_ID));
>         }
>
> -       for_each_present_cpu(cpu) {
> -               struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu);
> -
> -               if (!handler->present)
> -                       continue;
> -
> -               raw_spin_lock_irqsave(&handler->enable_lock, flags);
> -               for (i = 0; i < DIV_ROUND_UP(priv->nr_irqs, 32); i++) {
> -                       reg = handler->enable_base + i * sizeof(u32);
> -                       handler->enable_save[i] = readl(reg);
> -               }
> -               raw_spin_unlock_irqrestore(&handler->enable_lock, flags);
> -       }
> -
>         return 0;
>  }
>
> --
> 2.51.0
>
>