Register default arch handler via driver instead of directly pointing to
xilinx intc controller. This patch makes architecture code more generic.
Signed-off-by: Michal Simek <[email protected]>
Reviewed-by: Stefan Asserhall <[email protected]>
---
arch/microblaze/Kconfig | 2 ++
arch/microblaze/include/asm/irq.h | 3 ---
arch/microblaze/kernel/irq.c | 16 +---------------
drivers/irqchip/irq-xilinx-intc.c | 22 +++++++++++++++++++++-
4 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 59798e43cdb0..da568e981604 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -45,6 +45,8 @@ config MICROBLAZE
select SET_FS
select ZONE_DMA
select TRACE_IRQFLAGS_SUPPORT
+ select GENERIC_IRQ_MULTI_HANDLER
+ select HANDLE_DOMAIN_IRQ
# Endianness selection
choice
diff --git a/arch/microblaze/include/asm/irq.h b/arch/microblaze/include/asm/irq.h
index 0a28e80bbab0..cb6ab55d1d01 100644
--- a/arch/microblaze/include/asm/irq.h
+++ b/arch/microblaze/include/asm/irq.h
@@ -11,7 +11,4 @@
struct pt_regs;
extern void do_IRQ(struct pt_regs *regs);
-/* should be defined in each interrupt controller driver */
-extern unsigned int xintc_get_irq(void);
-
#endif /* _ASM_MICROBLAZE_IRQ_H */
diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
index 903dad822fad..1f8cb4c4f74f 100644
--- a/arch/microblaze/kernel/irq.c
+++ b/arch/microblaze/kernel/irq.c
@@ -20,27 +20,13 @@
#include <linux/irqchip.h>
#include <linux/of_irq.h>
-static u32 concurrent_irq;
-
void __irq_entry do_IRQ(struct pt_regs *regs)
{
- unsigned int irq;
struct pt_regs *old_regs = set_irq_regs(regs);
trace_hardirqs_off();
irq_enter();
- irq = xintc_get_irq();
-next_irq:
- BUG_ON(!irq);
- generic_handle_irq(irq);
-
- irq = xintc_get_irq();
- if (irq != -1U) {
- pr_debug("next irq: %d\n", irq);
- ++concurrent_irq;
- goto next_irq;
- }
-
+ handle_arch_irq(regs);
irq_exit();
set_irq_regs(old_regs);
trace_hardirqs_on();
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 356a59755d63..c6710190c152 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -110,7 +110,7 @@ static struct irq_chip intc_dev = {
.irq_mask_ack = intc_mask_ack,
};
-unsigned int xintc_get_irq(void)
+static unsigned int xintc_get_irq(void)
{
unsigned int irq = -1;
u32 hwirq;
@@ -164,6 +164,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}
+static u32 concurrent_irq;
+
+static void xil_intc_handle_irq(struct pt_regs *regs)
+{
+ unsigned int irq;
+
+ irq = xintc_get_irq();
+next_irq:
+ BUG_ON(!irq);
+ generic_handle_irq(irq);
+
+ irq = xintc_get_irq();
+ if (irq != -1U) {
+ pr_debug("next irq: %d\n", irq);
+ ++concurrent_irq;
+ goto next_irq;
+ }
+}
+
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
@@ -233,6 +252,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
} else {
primary_intc = irqc;
irq_set_default_host(primary_intc->root_domain);
+ set_handle_irq(xil_intc_handle_irq);
}
return 0;
--
2.35.1
On Thu, 2022-03-03 at 17:11 +0100, Michal Simek wrote:
> Register default arch handler via driver instead of directly pointing to
> xilinx intc controller. This patch makes architecture code more generic.
>
> Signed-off-by: Michal Simek <[email protected]>
> Reviewed-by: Stefan Asserhall <[email protected]>
> ---
>
> arch/microblaze/Kconfig | 2 ++
> arch/microblaze/include/asm/irq.h | 3 ---
> arch/microblaze/kernel/irq.c | 16 +---------------
> drivers/irqchip/irq-xilinx-intc.c | 22 +++++++++++++++++++++-
> 4 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 59798e43cdb0..da568e981604 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -45,6 +45,8 @@ config MICROBLAZE
> select SET_FS
> select ZONE_DMA
> select TRACE_IRQFLAGS_SUPPORT
> + select GENERIC_IRQ_MULTI_HANDLER
> + select HANDLE_DOMAIN_IRQ
>
> # Endianness selection
> choice
> diff --git a/arch/microblaze/include/asm/irq.h
> b/arch/microblaze/include/asm/irq.h
> index 0a28e80bbab0..cb6ab55d1d01 100644
> --- a/arch/microblaze/include/asm/irq.h
> +++ b/arch/microblaze/include/asm/irq.h
> @@ -11,7 +11,4 @@
> struct pt_regs;
> extern void do_IRQ(struct pt_regs *regs);
>
> -/* should be defined in each interrupt controller driver */
> -extern unsigned int xintc_get_irq(void);
> -
> #endif /* _ASM_MICROBLAZE_IRQ_H */
> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
> index 903dad822fad..1f8cb4c4f74f 100644
> --- a/arch/microblaze/kernel/irq.c
> +++ b/arch/microblaze/kernel/irq.c
> @@ -20,27 +20,13 @@
> #include <linux/irqchip.h>
> #include <linux/of_irq.h>
>
> -static u32 concurrent_irq;
> -
> void __irq_entry do_IRQ(struct pt_regs *regs)
> {
> - unsigned int irq;
> struct pt_regs *old_regs = set_irq_regs(regs);
> trace_hardirqs_off();
>
> irq_enter();
> - irq = xintc_get_irq();
> -next_irq:
> - BUG_ON(!irq);
> - generic_handle_irq(irq);
> -
> - irq = xintc_get_irq();
> - if (irq != -1U) {
> - pr_debug("next irq: %d\n", irq);
> - ++concurrent_irq;
> - goto next_irq;
> - }
> -
> + handle_arch_irq(regs);
> irq_exit();
> set_irq_regs(old_regs);
> trace_hardirqs_on();
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-
> intc.c
> index 356a59755d63..c6710190c152 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -110,7 +110,7 @@ static struct irq_chip intc_dev = {
> .irq_mask_ack = intc_mask_ack,
> };
>
> -unsigned int xintc_get_irq(void)
> +static unsigned int xintc_get_irq(void)
> {
> unsigned int irq = -1;
> u32 hwirq;
> @@ -164,6 +164,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> +static u32 concurrent_irq;
Not sure what this variable is for? It seems to be incremented, but never read.
> +
> +static void xil_intc_handle_irq(struct pt_regs *regs)
> +{
> + unsigned int irq;
> +
> + irq = xintc_get_irq();
> +next_irq:
> + BUG_ON(!irq);
> + generic_handle_irq(irq);
> +
> + irq = xintc_get_irq();
> + if (irq != -1U) {
> + pr_debug("next irq: %d\n", irq);
> + ++concurrent_irq;
> + goto next_irq;
> + }
> +}
> +
> static int __init xilinx_intc_of_init(struct device_node *intc,
> struct device_node *parent)
> {
> @@ -233,6 +252,7 @@ static int __init xilinx_intc_of_init(struct device_node
> *intc,
> } else {
> primary_intc = irqc;
> irq_set_default_host(primary_intc->root_domain);
> + set_handle_irq(xil_intc_handle_irq);
> }
>
> return 0;
--
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
http://www.calian.com
On Thu, 03 Mar 2022 16:11:39 +0000,
Michal Simek <[email protected]> wrote:
>
> Register default arch handler via driver instead of directly pointing to
> xilinx intc controller. This patch makes architecture code more generic.
>
> Signed-off-by: Michal Simek <[email protected]>
> Reviewed-by: Stefan Asserhall <[email protected]>
> ---
>
> arch/microblaze/Kconfig | 2 ++
> arch/microblaze/include/asm/irq.h | 3 ---
> arch/microblaze/kernel/irq.c | 16 +---------------
> drivers/irqchip/irq-xilinx-intc.c | 22 +++++++++++++++++++++-
> 4 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 59798e43cdb0..da568e981604 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -45,6 +45,8 @@ config MICROBLAZE
> select SET_FS
> select ZONE_DMA
> select TRACE_IRQFLAGS_SUPPORT
> + select GENERIC_IRQ_MULTI_HANDLER
> + select HANDLE_DOMAIN_IRQ
>
> # Endianness selection
> choice
> diff --git a/arch/microblaze/include/asm/irq.h b/arch/microblaze/include/asm/irq.h
> index 0a28e80bbab0..cb6ab55d1d01 100644
> --- a/arch/microblaze/include/asm/irq.h
> +++ b/arch/microblaze/include/asm/irq.h
> @@ -11,7 +11,4 @@
> struct pt_regs;
> extern void do_IRQ(struct pt_regs *regs);
>
> -/* should be defined in each interrupt controller driver */
> -extern unsigned int xintc_get_irq(void);
> -
> #endif /* _ASM_MICROBLAZE_IRQ_H */
> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
> index 903dad822fad..1f8cb4c4f74f 100644
> --- a/arch/microblaze/kernel/irq.c
> +++ b/arch/microblaze/kernel/irq.c
> @@ -20,27 +20,13 @@
> #include <linux/irqchip.h>
> #include <linux/of_irq.h>
>
> -static u32 concurrent_irq;
> -
> void __irq_entry do_IRQ(struct pt_regs *regs)
> {
> - unsigned int irq;
> struct pt_regs *old_regs = set_irq_regs(regs);
> trace_hardirqs_off();
>
> irq_enter();
> - irq = xintc_get_irq();
> -next_irq:
> - BUG_ON(!irq);
> - generic_handle_irq(irq);
> -
> - irq = xintc_get_irq();
> - if (irq != -1U) {
> - pr_debug("next irq: %d\n", irq);
> - ++concurrent_irq;
> - goto next_irq;
> - }
> -
> + handle_arch_irq(regs);
> irq_exit();
> set_irq_regs(old_regs);
> trace_hardirqs_on();
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 356a59755d63..c6710190c152 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -110,7 +110,7 @@ static struct irq_chip intc_dev = {
> .irq_mask_ack = intc_mask_ack,
> };
>
> -unsigned int xintc_get_irq(void)
> +static unsigned int xintc_get_irq(void)
> {
> unsigned int irq = -1;
> u32 hwirq;
> @@ -164,6 +164,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> +static u32 concurrent_irq;
Please kill this. It serves no purpose at all.
> +
> +static void xil_intc_handle_irq(struct pt_regs *regs)
> +{
> + unsigned int irq;
> +
> + irq = xintc_get_irq();
> +next_irq:
> + BUG_ON(!irq);
> + generic_handle_irq(irq);
> +
> + irq = xintc_get_irq();
> + if (irq != -1U) {
> + pr_debug("next irq: %d\n", irq);
> + ++concurrent_irq;
> + goto next_irq;
> + }
How about writing this in (basic) C code, and use the exiting APIs?
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 356a59755d63..18125bb841b2 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -110,18 +110,19 @@ static struct irq_chip intc_dev = {
.irq_mask_ack = intc_mask_ack,
};
-unsigned int xintc_get_irq(void)
+#define SPURIOUS_IRQ (-1U)
+
+static void xil_intc_handle_irq(struct pt_regs *regs)
{
- unsigned int irq = -1;
u32 hwirq;
- hwirq = xintc_read(primary_intc, IVR);
- if (hwirq != -1U)
- irq = irq_find_mapping(primary_intc->root_domain, hwirq);
-
- pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
+ do {
+ hwirq = xintc_read(primary_intc, IVR);
+ if (unlikely(hwirq == SPURIOUS_IRQ))
+ break;
- return irq;
+ generic_handle_domain_irq(primary_intc->root_domain, hwirq);
+ } while (true);
}
static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
M.
--
Without deviation from the norm, progress is not possible.
On 3/3/22 18:24, Marc Zyngier wrote:
> On Thu, 03 Mar 2022 16:11:39 +0000,
> Michal Simek <[email protected]> wrote:
>>
>> Register default arch handler via driver instead of directly pointing to
>> xilinx intc controller. This patch makes architecture code more generic.
>>
>> Signed-off-by: Michal Simek <[email protected]>
>> Reviewed-by: Stefan Asserhall <[email protected]>
>> ---
>>
>> arch/microblaze/Kconfig | 2 ++
>> arch/microblaze/include/asm/irq.h | 3 ---
>> arch/microblaze/kernel/irq.c | 16 +---------------
>> drivers/irqchip/irq-xilinx-intc.c | 22 +++++++++++++++++++++-
>> 4 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> index 59798e43cdb0..da568e981604 100644
>> --- a/arch/microblaze/Kconfig
>> +++ b/arch/microblaze/Kconfig
>> @@ -45,6 +45,8 @@ config MICROBLAZE
>> select SET_FS
>> select ZONE_DMA
>> select TRACE_IRQFLAGS_SUPPORT
>> + select GENERIC_IRQ_MULTI_HANDLER
>> + select HANDLE_DOMAIN_IRQ
>>
>> # Endianness selection
>> choice
>> diff --git a/arch/microblaze/include/asm/irq.h b/arch/microblaze/include/asm/irq.h
>> index 0a28e80bbab0..cb6ab55d1d01 100644
>> --- a/arch/microblaze/include/asm/irq.h
>> +++ b/arch/microblaze/include/asm/irq.h
>> @@ -11,7 +11,4 @@
>> struct pt_regs;
>> extern void do_IRQ(struct pt_regs *regs);
>>
>> -/* should be defined in each interrupt controller driver */
>> -extern unsigned int xintc_get_irq(void);
>> -
>> #endif /* _ASM_MICROBLAZE_IRQ_H */
>> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
>> index 903dad822fad..1f8cb4c4f74f 100644
>> --- a/arch/microblaze/kernel/irq.c
>> +++ b/arch/microblaze/kernel/irq.c
>> @@ -20,27 +20,13 @@
>> #include <linux/irqchip.h>
>> #include <linux/of_irq.h>
>>
>> -static u32 concurrent_irq;
>> -
>> void __irq_entry do_IRQ(struct pt_regs *regs)
>> {
>> - unsigned int irq;
>> struct pt_regs *old_regs = set_irq_regs(regs);
>> trace_hardirqs_off();
>>
>> irq_enter();
>> - irq = xintc_get_irq();
>> -next_irq:
>> - BUG_ON(!irq);
>> - generic_handle_irq(irq);
>> -
>> - irq = xintc_get_irq();
>> - if (irq != -1U) {
>> - pr_debug("next irq: %d\n", irq);
>> - ++concurrent_irq;
>> - goto next_irq;
>> - }
>> -
>> + handle_arch_irq(regs);
>> irq_exit();
>> set_irq_regs(old_regs);
>> trace_hardirqs_on();
>> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
>> index 356a59755d63..c6710190c152 100644
>> --- a/drivers/irqchip/irq-xilinx-intc.c
>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>> @@ -110,7 +110,7 @@ static struct irq_chip intc_dev = {
>> .irq_mask_ack = intc_mask_ack,
>> };
>>
>> -unsigned int xintc_get_irq(void)
>> +static unsigned int xintc_get_irq(void)
>> {
>> unsigned int irq = -1;
>> u32 hwirq;
>> @@ -164,6 +164,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
>> chained_irq_exit(chip, desc);
>> }
>>
>> +static u32 concurrent_irq;
>
> Please kill this. It serves no purpose at all.
>
>> +
>> +static void xil_intc_handle_irq(struct pt_regs *regs)
>> +{
>> + unsigned int irq;
>> +
>> + irq = xintc_get_irq();
>> +next_irq:
>> + BUG_ON(!irq);
>> + generic_handle_irq(irq);
>> +
>> + irq = xintc_get_irq();
>> + if (irq != -1U) {
>> + pr_debug("next irq: %d\n", irq);
>> + ++concurrent_irq;
>> + goto next_irq;
>> + }
>
> How about writing this in (basic) C code, and use the exiting APIs?
>
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 356a59755d63..18125bb841b2 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -110,18 +110,19 @@ static struct irq_chip intc_dev = {
> .irq_mask_ack = intc_mask_ack,
> };
>
> -unsigned int xintc_get_irq(void)
> +#define SPURIOUS_IRQ (-1U)
> +
> +static void xil_intc_handle_irq(struct pt_regs *regs)
> {
> - unsigned int irq = -1;
> u32 hwirq;
>
> - hwirq = xintc_read(primary_intc, IVR);
> - if (hwirq != -1U)
> - irq = irq_find_mapping(primary_intc->root_domain, hwirq);
> -
> - pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
> + do {
> + hwirq = xintc_read(primary_intc, IVR);
> + if (unlikely(hwirq == SPURIOUS_IRQ))
> + break;
>
> - return irq;
> + generic_handle_domain_irq(primary_intc->root_domain, hwirq);
> + } while (true);
> }
>
> static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
>
> M.
>
That works. Sent v2.
Thanks,
Michal
On 3/3/22 18:01, Robert Hancock wrote:
> On Thu, 2022-03-03 at 17:11 +0100, Michal Simek wrote:
>> Register default arch handler via driver instead of directly pointing to
>> xilinx intc controller. This patch makes architecture code more generic.
>>
>> Signed-off-by: Michal Simek <[email protected]>
>> Reviewed-by: Stefan Asserhall <[email protected]>
>> ---
>>
>> arch/microblaze/Kconfig | 2 ++
>> arch/microblaze/include/asm/irq.h | 3 ---
>> arch/microblaze/kernel/irq.c | 16 +---------------
>> drivers/irqchip/irq-xilinx-intc.c | 22 +++++++++++++++++++++-
>> 4 files changed, 24 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> index 59798e43cdb0..da568e981604 100644
>> --- a/arch/microblaze/Kconfig
>> +++ b/arch/microblaze/Kconfig
>> @@ -45,6 +45,8 @@ config MICROBLAZE
>> select SET_FS
>> select ZONE_DMA
>> select TRACE_IRQFLAGS_SUPPORT
>> + select GENERIC_IRQ_MULTI_HANDLER
>> + select HANDLE_DOMAIN_IRQ
>>
>> # Endianness selection
>> choice
>> diff --git a/arch/microblaze/include/asm/irq.h
>> b/arch/microblaze/include/asm/irq.h
>> index 0a28e80bbab0..cb6ab55d1d01 100644
>> --- a/arch/microblaze/include/asm/irq.h
>> +++ b/arch/microblaze/include/asm/irq.h
>> @@ -11,7 +11,4 @@
>> struct pt_regs;
>> extern void do_IRQ(struct pt_regs *regs);
>>
>> -/* should be defined in each interrupt controller driver */
>> -extern unsigned int xintc_get_irq(void);
>> -
>> #endif /* _ASM_MICROBLAZE_IRQ_H */
>> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
>> index 903dad822fad..1f8cb4c4f74f 100644
>> --- a/arch/microblaze/kernel/irq.c
>> +++ b/arch/microblaze/kernel/irq.c
>> @@ -20,27 +20,13 @@
>> #include <linux/irqchip.h>
>> #include <linux/of_irq.h>
>>
>> -static u32 concurrent_irq;
>> -
>> void __irq_entry do_IRQ(struct pt_regs *regs)
>> {
>> - unsigned int irq;
>> struct pt_regs *old_regs = set_irq_regs(regs);
>> trace_hardirqs_off();
>>
>> irq_enter();
>> - irq = xintc_get_irq();
>> -next_irq:
>> - BUG_ON(!irq);
>> - generic_handle_irq(irq);
>> -
>> - irq = xintc_get_irq();
>> - if (irq != -1U) {
>> - pr_debug("next irq: %d\n", irq);
>> - ++concurrent_irq;
>> - goto next_irq;
>> - }
>> -
>> + handle_arch_irq(regs);
>> irq_exit();
>> set_irq_regs(old_regs);
>> trace_hardirqs_on();
>> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-
>> intc.c
>> index 356a59755d63..c6710190c152 100644
>> --- a/drivers/irqchip/irq-xilinx-intc.c
>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>> @@ -110,7 +110,7 @@ static struct irq_chip intc_dev = {
>> .irq_mask_ack = intc_mask_ack,
>> };
>>
>> -unsigned int xintc_get_irq(void)
>> +static unsigned int xintc_get_irq(void)
>> {
>> unsigned int irq = -1;
>> u32 hwirq;
>> @@ -164,6 +164,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
>> chained_irq_exit(chip, desc);
>> }
>>
>> +static u32 concurrent_irq;
>
> Not sure what this variable is for? It seems to be incremented, but never read.
I used this when I was checking how irqs are working and how many concurrent_irq
I am getting. It was read via debugger not via any linux interface.
Thanks,
Michal