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 <[email protected]>
---
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);
+
+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;
+}
+
static int __init riscv_intc_init(struct device_node *node,
struct device_node *parent)
{
@@ -119,6 +124,8 @@ static int __init riscv_intc_init(struct device_node *node,
return -ENXIO;
}
+ riscv_set_intc_hwnode_fn(riscv_intc_hwnode);
+
rc = set_handle_irq(&riscv_intc_irq);
if (rc) {
pr_err("failed to set irq handler\n");
--
2.25.1
On Mon, 21 Feb 2022 09:55:05 +0000,
Anup Patel <[email protected]> wrote:
>
> On Mon, Feb 21, 2022 at 3:21 PM Marc Zyngier <[email protected]> wrote:
> >
> > On Sun, 20 Feb 2022 05:08:50 +0000,
> > Anup Patel <[email protected]> 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 <[email protected]>
> > > ---
> > > 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(+)
> > >
[...]
> > > 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.
Then why are you checking for intc_domain being NULL?
> > 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.
Ah, there is this guard that is only valid on the boot CPU. Fair
enough.
M.
--
Without deviation from the norm, progress is not possible.
On Sun, 20 Feb 2022 05:08:50 +0000,
Anup Patel <[email protected]> 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 <[email protected]>
> ---
> 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?
> +
> +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.
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,
You can't have it both ways.
M.
--
Without deviation from the norm, progress is not possible.
On Mon, Feb 21, 2022 at 3:21 PM Marc Zyngier <[email protected]> wrote:
>
> On Sun, 20 Feb 2022 05:08:50 +0000,
> Anup Patel <[email protected]> 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 <[email protected]>
> > ---
> > 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.
On Mon, Feb 21, 2022 at 3:21 PM Marc Zyngier <[email protected]> wrote:
>
> On Sun, 20 Feb 2022 05:08:50 +0000,
> Anup Patel <[email protected]> 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 <[email protected]>
> > ---
> > 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?
>
> > +
> > +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.
Okay, I will drop the check on intc_domain since we are registering
callback after creating a single INTC domain common for all CPUs.
>
> 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,
>
> You can't have it both ways.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Regards,
Anup