Message ID | 20230719113542.2293295-4-apatel@ventanamicro.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Linux RISC-V AIA Support | expand |
Context | Check | Description |
---|---|---|
conchuod/cover_letter | success | Series has a cover letter |
conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 471aba2e4760 |
conchuod/fixes_present | success | Fixes tag not required for -next series |
conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
conchuod/module_param | success | Was 0 now: 0 |
conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
conchuod/build_rv32_defconfig | success | Build OK |
conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
conchuod/header_inline | success | No static functions without inline keyword in header files |
conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 17 lines checked |
conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
conchuod/verify_fixes | success | No Fixes tag |
conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
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 >
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 --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)); }
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(-)