Messages in this thread | | | From | Anup Patel <> | Date | Mon, 21 Feb 2022 15:25:05 +0530 | Subject | Re: [PATCH v3 2/6] irqchip/riscv-intc: Allow drivers to directly discover INTC hwnode |
| |
On Mon, Feb 21, 2022 at 3:21 PM Marc Zyngier <maz@kernel.org> wrote: > > On Sun, 20 Feb 2022 05:08:50 +0000, > Anup Patel <apatel@ventanamicro.com> wrote: > > > > Various RISC-V drivers (such as SBI IPI, SBI Timer, SBI PMU, and > > KVM RISC-V) don't have associated DT node but these drivers need > > standard per-CPU (local) interrupts defined by the RISC-V privileged > > specification. > > > > We add riscv_get_intc_hwnode() in arch/riscv which allows RISC-V > > drivers not having DT node to discover INTC hwnode which in-turn > > helps these drivers to map per-CPU (local) interrupts provided > > by the INTC driver. > > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > arch/riscv/include/asm/irq.h | 4 ++++ > > arch/riscv/kernel/irq.c | 19 +++++++++++++++++++ > > drivers/irqchip/irq-riscv-intc.c | 7 +++++++ > > 3 files changed, 30 insertions(+) > > > > diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h > > index e4c435509983..43b9ebfbd943 100644 > > --- a/arch/riscv/include/asm/irq.h > > +++ b/arch/riscv/include/asm/irq.h > > @@ -12,6 +12,10 @@ > > > > #include <asm-generic/irq.h> > > > > +void riscv_set_intc_hwnode_fn(struct fwnode_handle *(*fn)(void)); > > + > > +struct fwnode_handle *riscv_get_intc_hwnode(void); > > + > > extern void __init init_IRQ(void); > > > > #endif /* _ASM_RISCV_IRQ_H */ > > diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c > > index 7207fa08d78f..ead92432df8c 100644 > > --- a/arch/riscv/kernel/irq.c > > +++ b/arch/riscv/kernel/irq.c > > @@ -7,9 +7,28 @@ > > > > #include <linux/interrupt.h> > > #include <linux/irqchip.h> > > +#include <linux/irqdomain.h> > > +#include <linux/module.h> > > #include <linux/seq_file.h> > > #include <asm/smp.h> > > > > +static struct fwnode_handle *(*__get_intc_node)(void); > > + > > +void riscv_set_intc_hwnode_fn(struct fwnode_handle *(*fn)(void)) > > +{ > > + __get_intc_node = fn; > > +} > > +EXPORT_SYMBOL_GPL(riscv_set_intc_hwnode_fn); > > We're talking about the root interrupt controller here. How can this > ever be implemented as a module?
Okay, I will drop the EXPORT() from here.
> > > + > > +struct fwnode_handle *riscv_get_intc_hwnode(void) > > +{ > > + if (__get_intc_node) > > + return __get_intc_node(); > > + > > + return NULL; > > +} > > +EXPORT_SYMBOL_GPL(riscv_get_intc_hwnode); > > + > > int arch_show_interrupts(struct seq_file *p, int prec) > > { > > show_ipi_stats(p, prec); > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c > > index b65bd8878d4f..fa24ecd01d39 100644 > > --- a/drivers/irqchip/irq-riscv-intc.c > > +++ b/drivers/irqchip/irq-riscv-intc.c > > @@ -92,6 +92,11 @@ static const struct irq_domain_ops riscv_intc_domain_ops = { > > .xlate = irq_domain_xlate_onecell, > > }; > > > > +static struct fwnode_handle *riscv_intc_hwnode(void) > > +{ > > + return (intc_domain) ? intc_domain->fwnode : NULL; > > +} > > This makes no sense. Either you have found the interrupt controller > and allocated the domain, or you haven't. But you don't register a > callback without having found it.
We are registering this callback after creating the INTC domain.
> > And you have totally ignored my previous comments about the multitude > of irq domains for the INTC. Either you get rid of all but one and you > can register a single fwnode, or you stay with what you have today,
Only the INTC DT nodes are per-CPU but we are creating only one INTC domain to manage per-CPU IRQs across all CPUs.
Regards, Anup
> > You can't have it both ways. > > M. > > -- > Without deviation from the norm, progress is not possible.
| |