diff mbox series

[v6,03/14] drivers: irqchip/riscv-intc: Mark all INTC nodes as initialized

Message ID 20230719113542.2293295-4-apatel@ventanamicro.com
State New
Headers
Series Linux RISC-V AIA Support |

Commit Message

Anup Patel July 19, 2023, 11:35 a.m. UTC 
  The RISC-V INTC local interrupts are per-HART (or per-CPU) so
we create INTC IRQ domain only for the INTC node belonging to
the boot HART. This means only the boot HART INTC node will be
marked as initialized and other INTC nodes won't be marked which
results downstream interrupt controllers (such as IMSIC and APLIC
direct-mode) not being probed due to missing device suppliers.

To address this issue, we mark all INTC node for which we don't
create IRQ domain as initialized.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 drivers/irqchip/irq-riscv-intc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Saravana Kannan July 19, 2023, 10:14 p.m. UTC | #1 
On Wed, Jul 19, 2023 at 4:36 AM Anup Patel <apatel@ventanamicro.com> wrote:
>
> The RISC-V INTC local interrupts are per-HART (or per-CPU) so
> we create INTC IRQ domain only for the INTC node belonging to
> the boot HART. This means only the boot HART INTC node will be
> marked as initialized and other INTC nodes won't be marked which
> results downstream interrupt controllers (such as IMSIC and APLIC
> direct-mode) not being probed due to missing device suppliers.
>
> To address this issue, we mark all INTC node for which we don't
> create IRQ domain as initialized.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  drivers/irqchip/irq-riscv-intc.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> index 65f4a2afb381..4e2704bc25fb 100644
> --- a/drivers/irqchip/irq-riscv-intc.c
> +++ b/drivers/irqchip/irq-riscv-intc.c
> @@ -155,8 +155,16 @@ static int __init riscv_intc_init(struct device_node *node,
>          * for each INTC DT node. We only need to do INTC initialization
>          * for the INTC DT node belonging to boot CPU (or boot HART).
>          */
> -       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id())
> +       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) {
> +               /*
> +                * The INTC nodes of each CPU are suppliers for downstream
> +                * interrupt controllers (such as IMSIC and APLIC direct-mode)
> +                * so we should mark an INTC node as initialized if we are
> +                * not creating IRQ domain for it.
> +                */

I'm a bit confused by this. If those non-boot CPUs INTC doesn't have
an IRQ domain, why are the downstream interrupt controllers listing
these non-boot CPU INTCs as an upstream interrupt controller?

This is more of a question of the existing behavior that this patch,
but this patch highlights the existing oddity.

-Saravana

> +               fwnode_dev_initialized(of_fwnode_handle(node), true);
>                 return 0;
> +       }
>
>         return riscv_intc_init_common(of_node_to_fwnode(node));
>  }
> --
> 2.34.1
>
Anup Patel July 20, 2023, 5:21 a.m. UTC | #2 
On Thu, Jul 20, 2023 at 3:45 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Wed, Jul 19, 2023 at 4:36 AM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > The RISC-V INTC local interrupts are per-HART (or per-CPU) so
> > we create INTC IRQ domain only for the INTC node belonging to
> > the boot HART. This means only the boot HART INTC node will be
> > marked as initialized and other INTC nodes won't be marked which
> > results downstream interrupt controllers (such as IMSIC and APLIC
> > direct-mode) not being probed due to missing device suppliers.
> >
> > To address this issue, we mark all INTC node for which we don't
> > create IRQ domain as initialized.
> >
> > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > ---
> >  drivers/irqchip/irq-riscv-intc.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > index 65f4a2afb381..4e2704bc25fb 100644
> > --- a/drivers/irqchip/irq-riscv-intc.c
> > +++ b/drivers/irqchip/irq-riscv-intc.c
> > @@ -155,8 +155,16 @@ static int __init riscv_intc_init(struct device_node *node,
> >          * for each INTC DT node. We only need to do INTC initialization
> >          * for the INTC DT node belonging to boot CPU (or boot HART).
> >          */
> > -       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id())
> > +       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) {
> > +               /*
> > +                * The INTC nodes of each CPU are suppliers for downstream
> > +                * interrupt controllers (such as IMSIC and APLIC direct-mode)
> > +                * so we should mark an INTC node as initialized if we are
> > +                * not creating IRQ domain for it.
> > +                */
>
> I'm a bit confused by this. If those non-boot CPUs INTC doesn't have
> an IRQ domain, why are the downstream interrupt controllers listing
> these non-boot CPU INTCs as an upstream interrupt controller?

Downstream interrupt controllers (such as PLIC, APLIC direct-mode,
or IMSIC) use "interrupts-extended" DT property to associate their
MMIO resources with target CPU.

For example: If we have 4 CPUs then PLIC will typically have 2 PLIC
MMIO contexts for each CPU where one context is for M-mode and
another context is for S-mode. In this case, there are total 8 PLIC
MMIO contexts and the arrangement of these MMIO resources is
platform specific so "interrupts-extended" DT property is used by
PLIC driver to discover the arrangement of PLIC MMIO contexts
and their mapping to target CPU.

Similar to the above example, the APLIC direct-mode has per-CPU
IDC MMIO registers and "interrupts-extended" DT property is used
to discover arrangement of these IDC MMIO registers and their
mapping to the target CPU.

>
> This is more of a question of the existing behavior that this patch,
> but this patch highlights the existing oddity.
>
> -Saravana
>
> > +               fwnode_dev_initialized(of_fwnode_handle(node), true);
> >                 return 0;
> > +       }
> >
> >         return riscv_intc_init_common(of_node_to_fwnode(node));
> >  }
> > --
> > 2.34.1
> >

Regards,
Anup
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index 65f4a2afb381..4e2704bc25fb 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -155,8 +155,16 @@  static int __init riscv_intc_init(struct device_node *node,
 	 * for each INTC DT node. We only need to do INTC initialization
 	 * for the INTC DT node belonging to boot CPU (or boot HART).
 	 */
-	if (riscv_hartid_to_cpuid(hartid) != smp_processor_id())
+	if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) {
+		/*
+		 * The INTC nodes of each CPU are suppliers for downstream
+		 * interrupt controllers (such as IMSIC and APLIC direct-mode)
+		 * so we should mark an INTC node as initialized if we are
+		 * not creating IRQ domain for it.
+		 */
+		fwnode_dev_initialized(of_fwnode_handle(node), true);
 		return 0;
+	}
 
 	return riscv_intc_init_common(of_node_to_fwnode(node));
 }