2020-03-17 12:57:47

by Mubin Usman Sayyed

[permalink] [raw]
Subject: [PATCH v5 0/4] irqchip: xilinx: Add support for multiple instances

Created series by rebasing inter-dependent patches
from Michal (https://lkml.org/lkml/2020/3/8/164) on top
of v3 (https://lkml.org/lkml/2020/2/14/2680)

Changes for v5:
- Fixed review comments from Marc - fixed intc_dev
related handling in 1/4 and added 4/4 to remove
unnecessary call to irq_set_default_host()
Changes for v4:
- Fixed review comments from Thomas - updated commit
message, variable declarations changed to reverse
fir tree and cleaned up some code.
- Added inter-dependendent patches 2/3 and 3/3 from Michal
https://lkml.org/lkml/2020/3/8/164

Changes for v3:
- Modified prototype of xintc_write/xintc_read
- Fixed review comments regarding indentation/variable
names, used BIT
- Modified xintc_get_irq_local to return 0
in case of failure/no pending interrupts
- Fixed return type of xintc_read
- Reverted changes related to device name and
kept intc_dev as static

Changes for v2:
- Removed write_fn/read_fn hooks, used xintc_write/
xintc_read directly
- Moved primary_intc declaration after xintc_irq_chip



Michal Simek (2):
irqchip: xilinx: Fill error code when irq domain registration fails
irqchip: xilinx: Enable generic irq multi handler

Mubin Sayyed (2):
irqchip: xilinx: Add support for multiple instances
irqchip: xilinx: Do not call irq_set_default_host()

arch/microblaze/Kconfig | 2 +
arch/microblaze/include/asm/irq.h | 3 -
arch/microblaze/kernel/irq.c | 21 +----
drivers/irqchip/irq-xilinx-intc.c | 123 ++++++++++++++++++------------
4 files changed, 77 insertions(+), 72 deletions(-)

--
2.25.0


2020-03-17 12:57:55

by Mubin Usman Sayyed

[permalink] [raw]
Subject: [PATCH v5 1/4] irqchip: xilinx: Add support for multiple instances

From: Mubin Sayyed <[email protected]>

Added support for cascaded interrupt controllers.

Following cascaded configurations have been tested,

- peripheral->xilinx-intc->xilinx-intc->gic->Cortexa53 processor
on zcu102 board
- peripheral->xilinx-intc->xilinx-intc->microblaze processor
on kcu105 board

Signed-off-by: Mubin Sayyed <[email protected]>
Signed-off-by: Anirudha Sarangi <[email protected]>
---
Changes for v5:
- Fixed review comments from Marc - removed intc_dev
from xintc_irq_chip structure
Changes for v4:
- Fixed review comments from Thomas - updated commit
message, variable declarations changed to reverse
fir tree and cleaned up some code.

Changes for v3:
- Modified prototype of xintc_write/xintc_read
- Fixed review comments regarding indentation/variable
names, used BIT
- Modified xintc_get_irq_local to return 0
in case of failure/no pending interrupts
- Fixed return type of xintc_read
- Reverted changes related to device name and
kept intc_dev as static

Changes for v2:
- Removed write_fn/read_fn hooks, used xintc_write/
xintc_read directly
- Moved primary_intc declaration after xintc_irq_chip
---
drivers/irqchip/irq-xilinx-intc.c | 115 +++++++++++++++++-------------
1 file changed, 67 insertions(+), 48 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index e3043ded8973..34593fa34c68 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -38,29 +38,31 @@ struct xintc_irq_chip {
void __iomem *base;
struct irq_domain *root_domain;
u32 intr_mask;
+ u32 nr_irq;
};

-static struct xintc_irq_chip *xintc_irqc;
+static struct xintc_irq_chip *primary_intc;

-static void xintc_write(int reg, u32 data)
+static void xintc_write(struct xintc_irq_chip *irqc, int reg, u32 data)
{
if (static_branch_unlikely(&xintc_is_be))
- iowrite32be(data, xintc_irqc->base + reg);
+ iowrite32be(data, irqc->base + reg);
else
- iowrite32(data, xintc_irqc->base + reg);
+ iowrite32(data, irqc->base + reg);
}

-static unsigned int xintc_read(int reg)
+static u32 xintc_read(struct xintc_irq_chip *irqc, int reg)
{
if (static_branch_unlikely(&xintc_is_be))
- return ioread32be(xintc_irqc->base + reg);
+ return ioread32be(irqc->base + reg);
else
- return ioread32(xintc_irqc->base + reg);
+ return ioread32(irqc->base + reg);
}

static void intc_enable_or_unmask(struct irq_data *d)
{
- unsigned long mask = 1 << d->hwirq;
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+ unsigned long mask = BIT(d->hwirq);

pr_debug("irq-xilinx: enable_or_unmask: %ld\n", d->hwirq);

@@ -69,30 +71,35 @@ static void intc_enable_or_unmask(struct irq_data *d)
* acks the irq before calling the interrupt handler
*/
if (irqd_is_level_type(d))
- xintc_write(IAR, mask);
+ xintc_write(irqc, IAR, mask);

- xintc_write(SIE, mask);
+ xintc_write(irqc, SIE, mask);
}

static void intc_disable_or_mask(struct irq_data *d)
{
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
- xintc_write(CIE, 1 << d->hwirq);
+ xintc_write(irqc, CIE, BIT(d->hwirq));
}

static void intc_ack(struct irq_data *d)
{
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
- xintc_write(IAR, 1 << d->hwirq);
+ xintc_write(irqc, IAR, BIT(d->hwirq));
}

static void intc_mask_ack(struct irq_data *d)
{
- unsigned long mask = 1 << d->hwirq;
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+ unsigned long mask = BIT(d->hwirq);

pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
- xintc_write(CIE, mask);
- xintc_write(IAR, mask);
+ xintc_write(irqc, CIE, mask);
+ xintc_write(irqc, IAR, mask);
}

static struct irq_chip intc_dev = {
@@ -103,13 +110,28 @@ static struct irq_chip intc_dev = {
.irq_mask_ack = intc_mask_ack,
};

+static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
+{
+ unsigned int irq = 0;
+ u32 hwirq;
+
+ hwirq = xintc_read(irqc, IVR);
+ if (hwirq != -1U)
+ irq = irq_find_mapping(irqc->root_domain, hwirq);
+
+ pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
+
+ return irq;
+}
+
unsigned int xintc_get_irq(void)
{
- unsigned int hwirq, irq = -1;
+ unsigned int irq = -1;
+ u32 hwirq;

- hwirq = xintc_read(IVR);
+ hwirq = xintc_read(primary_intc, IVR);
if (hwirq != -1U)
- irq = irq_find_mapping(xintc_irqc->root_domain, hwirq);
+ irq = irq_find_mapping(primary_intc->root_domain, hwirq);

pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);

@@ -118,15 +140,18 @@ unsigned int xintc_get_irq(void)

static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
{
- if (xintc_irqc->intr_mask & (1 << hw)) {
+ struct xintc_irq_chip *irqc = d->host_data;
+
+ if (irqc->intr_mask & BIT(hw)) {
irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_edge_irq, "edge");
+ handle_edge_irq, "edge");
irq_clear_status_flags(irq, IRQ_LEVEL);
} else {
irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_level_irq, "level");
+ handle_level_irq, "level");
irq_set_status_flags(irq, IRQ_LEVEL);
}
+ irq_set_chip_data(irq, irqc);
return 0;
}

@@ -138,12 +163,14 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
static void xil_intc_irq_handler(struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct xintc_irq_chip *irqc;
u32 pending;

+ irqc = irq_data_get_irq_handler_data(&desc->irq_data);
chained_irq_enter(chip, desc);
do {
- pending = xintc_get_irq();
- if (pending == -1U)
+ pending = xintc_get_irq_local(irqc);
+ if (pending == 0)
break;
generic_handle_irq(pending);
} while (true);
@@ -153,28 +180,19 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
- u32 nr_irq;
- int ret, irq;
struct xintc_irq_chip *irqc;
-
- if (xintc_irqc) {
- pr_err("irq-xilinx: Multiple instances aren't supported\n");
- return -EINVAL;
- }
+ int ret, irq;

irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
if (!irqc)
return -ENOMEM;
-
- xintc_irqc = irqc;
-
irqc->base = of_iomap(intc, 0);
BUG_ON(!irqc->base);

- ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &nr_irq);
+ ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &irqc->nr_irq);
if (ret < 0) {
pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
- goto err_alloc;
+ goto error;
}

ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
@@ -183,34 +201,34 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
irqc->intr_mask = 0;
}

- if (irqc->intr_mask >> nr_irq)
+ if (irqc->intr_mask >> irqc->nr_irq)
pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");

pr_info("irq-xilinx: %pOF: num_irq=%d, edge=0x%x\n",
- intc, nr_irq, irqc->intr_mask);
+ intc, irqc->nr_irq, irqc->intr_mask);


/*
* Disable all external interrupts until they are
* explicity requested.
*/
- xintc_write(IER, 0);
+ xintc_write(irqc, IER, 0);

/* Acknowledge any pending interrupts just in case. */
- xintc_write(IAR, 0xffffffff);
+ xintc_write(irqc, IAR, 0xffffffff);

/* Turn on the Master Enable. */
- xintc_write(MER, MER_HIE | MER_ME);
- if (!(xintc_read(MER) & (MER_HIE | MER_ME))) {
+ xintc_write(irqc, MER, MER_HIE | MER_ME);
+ if (xintc_read(irqc, MER) != (MER_HIE | MER_ME)) {
static_branch_enable(&xintc_is_be);
- xintc_write(MER, MER_HIE | MER_ME);
+ xintc_write(irqc, MER, MER_HIE | MER_ME);
}

- irqc->root_domain = irq_domain_add_linear(intc, nr_irq,
+ irqc->root_domain = irq_domain_add_linear(intc, irqc->nr_irq,
&xintc_irq_domain_ops, irqc);
if (!irqc->root_domain) {
pr_err("irq-xilinx: Unable to create IRQ domain\n");
- goto err_alloc;
+ goto error;
}

if (parent) {
@@ -222,16 +240,17 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
} else {
pr_err("irq-xilinx: interrupts property not in DT\n");
ret = -EINVAL;
- goto err_alloc;
+ goto error;
}
} else {
- irq_set_default_host(irqc->root_domain);
+ primary_intc = irqc;
+ irq_set_default_host(primary_intc->root_domain);
}

return 0;

-err_alloc:
- xintc_irqc = NULL;
+error:
+ iounmap(irqc->base);
kfree(irqc);
return ret;

--
2.25.0

2020-03-17 12:58:10

by Mubin Usman Sayyed

[permalink] [raw]
Subject: [PATCH v5 2/4] irqchip: xilinx: Fill error code when irq domain registration fails

From: Michal Simek <[email protected]>

There is no ret filled in case of irq_domain_add_linear() failure.

Signed-off-by: Michal Simek <[email protected]>
Reviewed-by: Stefan Asserhall <[email protected]>
---
drivers/irqchip/irq-xilinx-intc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 34593fa34c68..1d3d273309bd 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -228,6 +228,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
&xintc_irq_domain_ops, irqc);
if (!irqc->root_domain) {
pr_err("irq-xilinx: Unable to create IRQ domain\n");
+ ret = -EINVAL;
goto error;
}

--
2.25.0

2020-03-17 12:58:25

by Mubin Usman Sayyed

[permalink] [raw]
Subject: [PATCH v5 3/4] irqchip: xilinx: Enable generic irq multi handler

From: Michal Simek <[email protected]>

Register default arch handler via driver instead of directly pointing to
xilinx intc controller. This patch makes architecture code more generic.

Driver calls generic domain specific irq handler which does the most of
things self. Also get rid of concurrent_irq counting which hasn't been
exported anywhere.
Based on this loop was also optimized by using do/while loop instead of
goto loop.

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 | 21 +------------------
drivers/irqchip/irq-xilinx-intc.c | 34 ++++++++++++++++++-------------
4 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index a75896f18e58..b5a87c294925 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -47,6 +47,8 @@ config MICROBLAZE
select CPU_NO_EFFICIENT_FFS
select MMU_GATHER_NO_RANGE if MMU
select SPARSE_IRQ
+ 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 eac2fb4b3fb9..5166f0893e2b 100644
--- a/arch/microblaze/include/asm/irq.h
+++ b/arch/microblaze/include/asm/irq.h
@@ -14,7 +14,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..0b37dde60a1e 100644
--- a/arch/microblaze/kernel/irq.c
+++ b/arch/microblaze/kernel/irq.c
@@ -20,29 +20,10 @@
#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;
- }
-
- irq_exit();
- set_irq_regs(old_regs);
+ handle_arch_irq(regs);
trace_hardirqs_on();
}

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1d3d273309bd..ea741818a1ce 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
return irq;
}

-unsigned int xintc_get_irq(void)
-{
- 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);
-
- return irq;
-}
-
static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
{
struct xintc_irq_chip *irqc = d->host_data;
@@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

+static void xil_intc_handle_irq(struct pt_regs *regs)
+{
+ u32 hwirq;
+ struct xintc_irq_chip *irqc = primary_intc;
+
+ do {
+ hwirq = xintc_read(irqc, IVR);
+ if (likely(hwirq != -1U)) {
+ int ret;
+
+ ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
+ WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
+ continue;
+ }
+
+ break;
+ } while (1);
+}
+
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
@@ -246,6 +251,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.25.0

2020-03-17 12:59:33

by Mubin Usman Sayyed

[permalink] [raw]
Subject: [PATCH v5 4/4] irqchip: xilinx: Do not call irq_set_default_host()

From: Mubin Sayyed <[email protected]>

Using a default domain on DT based platforms is unnecessary.

Signed-off-by: Mubin Sayyed <[email protected]>
---
drivers/irqchip/irq-xilinx-intc.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index ea741818a1ce..7f811fe5bf69 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -250,7 +250,6 @@ 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);
}

--
2.25.0

2020-03-21 15:21:37

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 0/4] irqchip: xilinx: Add support for multiple instances

On Tue, 17 Mar 2020 12:55:56 +0000,
Mubin Usman Sayyed <[email protected]> wrote:
>
> Created series by rebasing inter-dependent patches
> from Michal (https://lkml.org/lkml/2020/3/8/164) on top
> of v3 (https://lkml.org/lkml/2020/2/14/2680)
>
> Changes for v5:
> - Fixed review comments from Marc - fixed intc_dev
> related handling in 1/4 and added 4/4 to remove
> unnecessary call to irq_set_default_host()
> Changes for v4:
> - Fixed review comments from Thomas - updated commit
> message, variable declarations changed to reverse
> fir tree and cleaned up some code.
> - Added inter-dependendent patches 2/3 and 3/3 from Michal
> https://lkml.org/lkml/2020/3/8/164
>
> Changes for v3:
> - Modified prototype of xintc_write/xintc_read
> - Fixed review comments regarding indentation/variable
> names, used BIT
> - Modified xintc_get_irq_local to return 0
> in case of failure/no pending interrupts
> - Fixed return type of xintc_read
> - Reverted changes related to device name and
> kept intc_dev as static
>
> Changes for v2:
> - Removed write_fn/read_fn hooks, used xintc_write/
> xintc_read directly
> - Moved primary_intc declaration after xintc_irq_chip
>
>
>
> Michal Simek (2):
> irqchip: xilinx: Fill error code when irq domain registration fails
> irqchip: xilinx: Enable generic irq multi handler
>
> Mubin Sayyed (2):
> irqchip: xilinx: Add support for multiple instances
> irqchip: xilinx: Do not call irq_set_default_host()
>
> arch/microblaze/Kconfig | 2 +
> arch/microblaze/include/asm/irq.h | 3 -
> arch/microblaze/kernel/irq.c | 21 +----
> drivers/irqchip/irq-xilinx-intc.c | 123 ++++++++++++++++++------------
> 4 files changed, 77 insertions(+), 72 deletions(-)

Queued for 5.7.

Thanks,

M.

--
Jazz is not dead, it just smells funny.

Subject: [tip: irq/core] irqchip/xilinx: Do not call irq_set_default_host()

The following commit has been merged into the irq/core branch of tip:

Commit-ID: 9c2d4f525c002591f4e0c14a37663663aaba1656
Gitweb: https://git.kernel.org/tip/9c2d4f525c002591f4e0c14a37663663aaba1656
Author: Mubin Sayyed <[email protected]>
AuthorDate: Tue, 17 Mar 2020 18:26:00 +05:30
Committer: Marc Zyngier <[email protected]>
CommitterDate: Sun, 22 Mar 2020 11:52:53

irqchip/xilinx: Do not call irq_set_default_host()

Using a default domain on DT based platforms is unnecessary.

Signed-off-by: Mubin Sayyed <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-xilinx-intc.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index ea74181..7f811fe 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -250,7 +250,6 @@ 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);
}

Subject: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

The following commit has been merged into the irq/core branch of tip:

Commit-ID: a0789993bf8266e62fea6b4613945ba081c71e7d
Gitweb: https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
Author: Michal Simek <[email protected]>
AuthorDate: Tue, 17 Mar 2020 18:25:59 +05:30
Committer: Marc Zyngier <[email protected]>
CommitterDate: Sun, 22 Mar 2020 11:52:53

irqchip/xilinx: Enable generic irq multi handler

Register default arch handler via driver instead of directly pointing to
xilinx intc controller. This patch makes architecture code more generic.

Driver calls generic domain specific irq handler which does the most of
things self. Also get rid of concurrent_irq counting which hasn't been
exported anywhere.
Based on this loop was also optimized by using do/while loop instead of
goto loop.

Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Stefan Asserhall <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/microblaze/Kconfig | 2 ++-
arch/microblaze/include/asm/irq.h | 3 +---
arch/microblaze/kernel/irq.c | 21 +-------------------
drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
4 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 6a331bd..242f58e 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -47,6 +47,8 @@ config MICROBLAZE
select CPU_NO_EFFICIENT_FFS
select MMU_GATHER_NO_RANGE if MMU
select SPARSE_IRQ
+ 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 eac2fb4..5166f08 100644
--- a/arch/microblaze/include/asm/irq.h
+++ b/arch/microblaze/include/asm/irq.h
@@ -14,7 +14,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 903dad8..0b37dde 100644
--- a/arch/microblaze/kernel/irq.c
+++ b/arch/microblaze/kernel/irq.c
@@ -20,29 +20,10 @@
#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;
- }
-
- irq_exit();
- set_irq_regs(old_regs);
+ handle_arch_irq(regs);
trace_hardirqs_on();
}

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1d3d273..ea74181 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
return irq;
}

-unsigned int xintc_get_irq(void)
-{
- 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);
-
- return irq;
-}
-
static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
{
struct xintc_irq_chip *irqc = d->host_data;
@@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

+static void xil_intc_handle_irq(struct pt_regs *regs)
+{
+ u32 hwirq;
+ struct xintc_irq_chip *irqc = primary_intc;
+
+ do {
+ hwirq = xintc_read(irqc, IVR);
+ if (likely(hwirq != -1U)) {
+ int ret;
+
+ ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
+ WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
+ continue;
+ }
+
+ break;
+ } while (1);
+}
+
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
@@ -246,6 +251,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;

Subject: [tip: irq/core] irqchip/xilinx: Fill error code when irq domain registration fails

The following commit has been merged into the irq/core branch of tip:

Commit-ID: c74038baa9bccac76344b7215a55be136c81dfc3
Gitweb: https://git.kernel.org/tip/c74038baa9bccac76344b7215a55be136c81dfc3
Author: Michal Simek <[email protected]>
AuthorDate: Tue, 17 Mar 2020 18:25:58 +05:30
Committer: Marc Zyngier <[email protected]>
CommitterDate: Sun, 22 Mar 2020 11:52:53

irqchip/xilinx: Fill error code when irq domain registration fails

There is no ret filled in case of irq_domain_add_linear() failure.

Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Reviewed-by: Stefan Asserhall <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-xilinx-intc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 34593fa..1d3d273 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -228,6 +228,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
&xintc_irq_domain_ops, irqc);
if (!irqc->root_domain) {
pr_err("irq-xilinx: Unable to create IRQ domain\n");
+ ret = -EINVAL;
goto error;
}

Subject: [tip: irq/core] irqchip/xilinx: Add support for multiple instances

The following commit has been merged into the irq/core branch of tip:

Commit-ID: 67862a3c47fcfc9330d153436c7a6604ae3daec9
Gitweb: https://git.kernel.org/tip/67862a3c47fcfc9330d153436c7a6604ae3daec9
Author: Mubin Sayyed <[email protected]>
AuthorDate: Tue, 17 Mar 2020 18:25:57 +05:30
Committer: Marc Zyngier <[email protected]>
CommitterDate: Sun, 22 Mar 2020 11:52:52

irqchip/xilinx: Add support for multiple instances

Added support for cascaded interrupt controllers.

Following cascaded configurations have been tested,

- peripheral->xilinx-intc->xilinx-intc->gic->Cortexa53 processor
on zcu102 board
- peripheral->xilinx-intc->xilinx-intc->microblaze processor
on kcu105 board

Signed-off-by: Mubin Sayyed <[email protected]>
Signed-off-by: Anirudha Sarangi <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-xilinx-intc.c | 115 ++++++++++++++++-------------
1 file changed, 67 insertions(+), 48 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index e3043de..34593fa 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -38,29 +38,31 @@ struct xintc_irq_chip {
void __iomem *base;
struct irq_domain *root_domain;
u32 intr_mask;
+ u32 nr_irq;
};

-static struct xintc_irq_chip *xintc_irqc;
+static struct xintc_irq_chip *primary_intc;

-static void xintc_write(int reg, u32 data)
+static void xintc_write(struct xintc_irq_chip *irqc, int reg, u32 data)
{
if (static_branch_unlikely(&xintc_is_be))
- iowrite32be(data, xintc_irqc->base + reg);
+ iowrite32be(data, irqc->base + reg);
else
- iowrite32(data, xintc_irqc->base + reg);
+ iowrite32(data, irqc->base + reg);
}

-static unsigned int xintc_read(int reg)
+static u32 xintc_read(struct xintc_irq_chip *irqc, int reg)
{
if (static_branch_unlikely(&xintc_is_be))
- return ioread32be(xintc_irqc->base + reg);
+ return ioread32be(irqc->base + reg);
else
- return ioread32(xintc_irqc->base + reg);
+ return ioread32(irqc->base + reg);
}

static void intc_enable_or_unmask(struct irq_data *d)
{
- unsigned long mask = 1 << d->hwirq;
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+ unsigned long mask = BIT(d->hwirq);

pr_debug("irq-xilinx: enable_or_unmask: %ld\n", d->hwirq);

@@ -69,30 +71,35 @@ static void intc_enable_or_unmask(struct irq_data *d)
* acks the irq before calling the interrupt handler
*/
if (irqd_is_level_type(d))
- xintc_write(IAR, mask);
+ xintc_write(irqc, IAR, mask);

- xintc_write(SIE, mask);
+ xintc_write(irqc, SIE, mask);
}

static void intc_disable_or_mask(struct irq_data *d)
{
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
- xintc_write(CIE, 1 << d->hwirq);
+ xintc_write(irqc, CIE, BIT(d->hwirq));
}

static void intc_ack(struct irq_data *d)
{
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+
pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
- xintc_write(IAR, 1 << d->hwirq);
+ xintc_write(irqc, IAR, BIT(d->hwirq));
}

static void intc_mask_ack(struct irq_data *d)
{
- unsigned long mask = 1 << d->hwirq;
+ struct xintc_irq_chip *irqc = irq_data_get_irq_chip_data(d);
+ unsigned long mask = BIT(d->hwirq);

pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
- xintc_write(CIE, mask);
- xintc_write(IAR, mask);
+ xintc_write(irqc, CIE, mask);
+ xintc_write(irqc, IAR, mask);
}

static struct irq_chip intc_dev = {
@@ -103,13 +110,28 @@ static struct irq_chip intc_dev = {
.irq_mask_ack = intc_mask_ack,
};

+static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
+{
+ unsigned int irq = 0;
+ u32 hwirq;
+
+ hwirq = xintc_read(irqc, IVR);
+ if (hwirq != -1U)
+ irq = irq_find_mapping(irqc->root_domain, hwirq);
+
+ pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
+
+ return irq;
+}
+
unsigned int xintc_get_irq(void)
{
- unsigned int hwirq, irq = -1;
+ unsigned int irq = -1;
+ u32 hwirq;

- hwirq = xintc_read(IVR);
+ hwirq = xintc_read(primary_intc, IVR);
if (hwirq != -1U)
- irq = irq_find_mapping(xintc_irqc->root_domain, hwirq);
+ irq = irq_find_mapping(primary_intc->root_domain, hwirq);

pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);

@@ -118,15 +140,18 @@ unsigned int xintc_get_irq(void)

static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
{
- if (xintc_irqc->intr_mask & (1 << hw)) {
+ struct xintc_irq_chip *irqc = d->host_data;
+
+ if (irqc->intr_mask & BIT(hw)) {
irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_edge_irq, "edge");
+ handle_edge_irq, "edge");
irq_clear_status_flags(irq, IRQ_LEVEL);
} else {
irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_level_irq, "level");
+ handle_level_irq, "level");
irq_set_status_flags(irq, IRQ_LEVEL);
}
+ irq_set_chip_data(irq, irqc);
return 0;
}

@@ -138,12 +163,14 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
static void xil_intc_irq_handler(struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct xintc_irq_chip *irqc;
u32 pending;

+ irqc = irq_data_get_irq_handler_data(&desc->irq_data);
chained_irq_enter(chip, desc);
do {
- pending = xintc_get_irq();
- if (pending == -1U)
+ pending = xintc_get_irq_local(irqc);
+ if (pending == 0)
break;
generic_handle_irq(pending);
} while (true);
@@ -153,28 +180,19 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
- u32 nr_irq;
- int ret, irq;
struct xintc_irq_chip *irqc;
-
- if (xintc_irqc) {
- pr_err("irq-xilinx: Multiple instances aren't supported\n");
- return -EINVAL;
- }
+ int ret, irq;

irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
if (!irqc)
return -ENOMEM;
-
- xintc_irqc = irqc;
-
irqc->base = of_iomap(intc, 0);
BUG_ON(!irqc->base);

- ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &nr_irq);
+ ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &irqc->nr_irq);
if (ret < 0) {
pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
- goto err_alloc;
+ goto error;
}

ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
@@ -183,34 +201,34 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
irqc->intr_mask = 0;
}

- if (irqc->intr_mask >> nr_irq)
+ if (irqc->intr_mask >> irqc->nr_irq)
pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");

pr_info("irq-xilinx: %pOF: num_irq=%d, edge=0x%x\n",
- intc, nr_irq, irqc->intr_mask);
+ intc, irqc->nr_irq, irqc->intr_mask);


/*
* Disable all external interrupts until they are
* explicity requested.
*/
- xintc_write(IER, 0);
+ xintc_write(irqc, IER, 0);

/* Acknowledge any pending interrupts just in case. */
- xintc_write(IAR, 0xffffffff);
+ xintc_write(irqc, IAR, 0xffffffff);

/* Turn on the Master Enable. */
- xintc_write(MER, MER_HIE | MER_ME);
- if (!(xintc_read(MER) & (MER_HIE | MER_ME))) {
+ xintc_write(irqc, MER, MER_HIE | MER_ME);
+ if (xintc_read(irqc, MER) != (MER_HIE | MER_ME)) {
static_branch_enable(&xintc_is_be);
- xintc_write(MER, MER_HIE | MER_ME);
+ xintc_write(irqc, MER, MER_HIE | MER_ME);
}

- irqc->root_domain = irq_domain_add_linear(intc, nr_irq,
+ irqc->root_domain = irq_domain_add_linear(intc, irqc->nr_irq,
&xintc_irq_domain_ops, irqc);
if (!irqc->root_domain) {
pr_err("irq-xilinx: Unable to create IRQ domain\n");
- goto err_alloc;
+ goto error;
}

if (parent) {
@@ -222,16 +240,17 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
} else {
pr_err("irq-xilinx: interrupts property not in DT\n");
ret = -EINVAL;
- goto err_alloc;
+ goto error;
}
} else {
- irq_set_default_host(irqc->root_domain);
+ primary_intc = irqc;
+ irq_set_default_host(primary_intc->root_domain);
}

return 0;

-err_alloc:
- xintc_irqc = NULL;
+error:
+ iounmap(irqc->base);
kfree(irqc);
return ret;

2020-03-30 08:34:29

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

Hi Thomas and Marc,

On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
> The following commit has been merged into the irq/core branch of tip:
>
> Commit-ID: a0789993bf8266e62fea6b4613945ba081c71e7d
> Gitweb: https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
> Author: Michal Simek <[email protected]>
> AuthorDate: Tue, 17 Mar 2020 18:25:59 +05:30
> Committer: Marc Zyngier <[email protected]>
> CommitterDate: Sun, 22 Mar 2020 11:52:53
>
> irqchip/xilinx: Enable generic irq multi handler
>
> Register default arch handler via driver instead of directly pointing to
> xilinx intc controller. This patch makes architecture code more generic.
>
> Driver calls generic domain specific irq handler which does the most of
> things self. Also get rid of concurrent_irq counting which hasn't been
> exported anywhere.
> Based on this loop was also optimized by using do/while loop instead of
> goto loop.
>
> Signed-off-by: Michal Simek <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>
> Reviewed-by: Stefan Asserhall <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> ---
> arch/microblaze/Kconfig | 2 ++-
> arch/microblaze/include/asm/irq.h | 3 +---
> arch/microblaze/kernel/irq.c | 21 +-------------------
> drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
> 4 files changed, 23 insertions(+), 37 deletions(-)
>
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 6a331bd..242f58e 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -47,6 +47,8 @@ config MICROBLAZE
> select CPU_NO_EFFICIENT_FFS
> select MMU_GATHER_NO_RANGE if MMU
> select SPARSE_IRQ
> + 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 eac2fb4..5166f08 100644
> --- a/arch/microblaze/include/asm/irq.h
> +++ b/arch/microblaze/include/asm/irq.h
> @@ -14,7 +14,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 903dad8..0b37dde 100644
> --- a/arch/microblaze/kernel/irq.c
> +++ b/arch/microblaze/kernel/irq.c
> @@ -20,29 +20,10 @@
> #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;
> - }
> -
> - irq_exit();
> - set_irq_regs(old_regs);
> + handle_arch_irq(regs);
> trace_hardirqs_on();
> }
>
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 1d3d273..ea74181 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
> return irq;
> }
>
> -unsigned int xintc_get_irq(void)
> -{
> - 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);
> -
> - return irq;
> -}
> -
> static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> {
> struct xintc_irq_chip *irqc = d->host_data;
> @@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> +static void xil_intc_handle_irq(struct pt_regs *regs)
> +{
> + u32 hwirq;
> + struct xintc_irq_chip *irqc = primary_intc;
> +
> + do {
> + hwirq = xintc_read(irqc, IVR);
> + if (likely(hwirq != -1U)) {
> + int ret;
> +
> + ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
> + WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
> + continue;
> + }
> +
> + break;
> + } while (1);
> +}
> +
> static int __init xilinx_intc_of_init(struct device_node *intc,
> struct device_node *parent)
> {
> @@ -246,6 +251,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;
>

Stephen reported compilation issue when this patch is applied because of
removal of xintc_get_irq() which is also used by ancient ppc405/ppc440
xilinx platforms. I have reported this twice to Marc already last week.

On Friday I have also send v1 of removal of that platforms (need to send v2)
https://lore.kernel.org/alsa-devel/[email protected]/

That's why please really consider next steps and let us know.

Thanks,
Michal


2020-03-30 08:46:09

by Marc Zyngier

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 2020-03-30 09:32, Michal Simek wrote:
> Hi Thomas and Marc,
>
> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>> The following commit has been merged into the irq/core branch of tip:
>>
>> Commit-ID: a0789993bf8266e62fea6b4613945ba081c71e7d
>> Gitweb:
>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>> Author: Michal Simek <[email protected]>
>> AuthorDate: Tue, 17 Mar 2020 18:25:59 +05:30
>> Committer: Marc Zyngier <[email protected]>
>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>
>> irqchip/xilinx: Enable generic irq multi handler
>>
>> Register default arch handler via driver instead of directly pointing
>> to
>> xilinx intc controller. This patch makes architecture code more
>> generic.
>>
>> Driver calls generic domain specific irq handler which does the most
>> of
>> things self. Also get rid of concurrent_irq counting which hasn't been
>> exported anywhere.
>> Based on this loop was also optimized by using do/while loop instead
>> of
>> goto loop.
>>
>> Signed-off-by: Michal Simek <[email protected]>
>> Signed-off-by: Marc Zyngier <[email protected]>
>> Reviewed-by: Stefan Asserhall <[email protected]>
>> Link:
>> https://lore.kernel.org/r/[email protected]
>> ---
>> arch/microblaze/Kconfig | 2 ++-
>> arch/microblaze/include/asm/irq.h | 3 +---
>> arch/microblaze/kernel/irq.c | 21 +-------------------
>> drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
>> 4 files changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> index 6a331bd..242f58e 100644
>> --- a/arch/microblaze/Kconfig
>> +++ b/arch/microblaze/Kconfig
>> @@ -47,6 +47,8 @@ config MICROBLAZE
>> select CPU_NO_EFFICIENT_FFS
>> select MMU_GATHER_NO_RANGE if MMU
>> select SPARSE_IRQ
>> + 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 eac2fb4..5166f08 100644
>> --- a/arch/microblaze/include/asm/irq.h
>> +++ b/arch/microblaze/include/asm/irq.h
>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>> --- a/arch/microblaze/kernel/irq.c
>> +++ b/arch/microblaze/kernel/irq.c
>> @@ -20,29 +20,10 @@
>> #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;
>> - }
>> -
>> - irq_exit();
>> - set_irq_regs(old_regs);
>> + handle_arch_irq(regs);
>> trace_hardirqs_on();
>> }
>>
>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>> b/drivers/irqchip/irq-xilinx-intc.c
>> index 1d3d273..ea74181 100644
>> --- a/drivers/irqchip/irq-xilinx-intc.c
>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>> xintc_irq_chip *irqc)
>> return irq;
>> }
>>
>> -unsigned int xintc_get_irq(void)
>> -{
>> - 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);
>> -
>> - return irq;
>> -}
>> -
>> static int xintc_map(struct irq_domain *d, unsigned int irq,
>> irq_hw_number_t hw)
>> {
>> struct xintc_irq_chip *irqc = d->host_data;
>> @@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc
>> *desc)
>> chained_irq_exit(chip, desc);
>> }
>>
>> +static void xil_intc_handle_irq(struct pt_regs *regs)
>> +{
>> + u32 hwirq;
>> + struct xintc_irq_chip *irqc = primary_intc;
>> +
>> + do {
>> + hwirq = xintc_read(irqc, IVR);
>> + if (likely(hwirq != -1U)) {
>> + int ret;
>> +
>> + ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
>> + WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
>> + continue;
>> + }
>> +
>> + break;
>> + } while (1);
>> +}
>> +
>> static int __init xilinx_intc_of_init(struct device_node *intc,
>> struct device_node *parent)
>> {
>> @@ -246,6 +251,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;
>>
>
> Stephen reported compilation issue when this patch is applied because
> of
> removal of xintc_get_irq() which is also used by ancient ppc405/ppc440
> xilinx platforms. I have reported this twice to Marc already last week.

Did you? I can't possibly find these emails.

> On Friday I have also send v1 of removal of that platforms (need to
> send v2)
> https://lore.kernel.org/alsa-devel/[email protected]/

You want to remove exising platforms two days before the start of the
merge window?
I don't think this is acceptable with such short notice.

> That's why please really consider next steps and let us know.

I think the only option is to revert (at least partially) the Xilinx
series.

M.
--
Jazz is not dead. It just smells funny...

2020-03-30 09:05:44

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 30. 03. 20 10:45, Marc Zyngier wrote:
> On 2020-03-30 09:32, Michal Simek wrote:
>> Hi Thomas and Marc,
>>
>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>> The following commit has been merged into the irq/core branch of tip:
>>>
>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>> Gitweb:       
>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>> Author:        Michal Simek <[email protected]>
>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>> Committer:     Marc Zyngier <[email protected]>
>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>
>>> irqchip/xilinx: Enable generic irq multi handler
>>>
>>> Register default arch handler via driver instead of directly pointing to
>>> xilinx intc controller. This patch makes architecture code more generic.
>>>
>>> Driver calls generic domain specific irq handler which does the most of
>>> things self. Also get rid of concurrent_irq counting which hasn't been
>>> exported anywhere.
>>> Based on this loop was also optimized by using do/while loop instead of
>>> goto loop.
>>>
>>> Signed-off-by: Michal Simek <[email protected]>
>>> Signed-off-by: Marc Zyngier <[email protected]>
>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>> Link:
>>> https://lore.kernel.org/r/[email protected]
>>>
>>> ---
>>>  arch/microblaze/Kconfig           |  2 ++-
>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>  drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>> index 6a331bd..242f58e 100644
>>> --- a/arch/microblaze/Kconfig
>>> +++ b/arch/microblaze/Kconfig
>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>      select CPU_NO_EFFICIENT_FFS
>>>      select MMU_GATHER_NO_RANGE if MMU
>>>      select SPARSE_IRQ
>>> +    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 eac2fb4..5166f08 100644
>>> --- a/arch/microblaze/include/asm/irq.h
>>> +++ b/arch/microblaze/include/asm/irq.h
>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>> --- a/arch/microblaze/kernel/irq.c
>>> +++ b/arch/microblaze/kernel/irq.c
>>> @@ -20,29 +20,10 @@
>>>  #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;
>>> -    }
>>> -
>>> -    irq_exit();
>>> -    set_irq_regs(old_regs);
>>> +    handle_arch_irq(regs);
>>>      trace_hardirqs_on();
>>>  }
>>>
>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>> b/drivers/irqchip/irq-xilinx-intc.c
>>> index 1d3d273..ea74181 100644
>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>> xintc_irq_chip *irqc)
>>>      return irq;
>>>  }
>>>
>>> -unsigned int xintc_get_irq(void)
>>> -{
>>> -    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);
>>> -
>>> -    return irq;
>>> -}
>>> -
>>>  static int xintc_map(struct irq_domain *d, unsigned int irq,
>>> irq_hw_number_t hw)
>>>  {
>>>      struct xintc_irq_chip *irqc = d->host_data;
>>> @@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc
>>> *desc)
>>>      chained_irq_exit(chip, desc);
>>>  }
>>>
>>> +static void xil_intc_handle_irq(struct pt_regs *regs)
>>> +{
>>> +    u32 hwirq;
>>> +    struct xintc_irq_chip *irqc = primary_intc;
>>> +
>>> +    do {
>>> +        hwirq = xintc_read(irqc, IVR);
>>> +        if (likely(hwirq != -1U)) {
>>> +            int ret;
>>> +
>>> +            ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
>>> +            WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
>>> +            continue;
>>> +        }
>>> +
>>> +        break;
>>> +    } while (1);
>>> +}
>>> +
>>>  static int __init xilinx_intc_of_init(struct device_node *intc,
>>>                           struct device_node *parent)
>>>  {
>>> @@ -246,6 +251,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;
>>>
>>
>> Stephen reported compilation issue when this patch is applied because of
>> removal of xintc_get_irq() which is also used by ancient ppc405/ppc440
>> xilinx platforms. I have reported this twice to Marc already last week.
>
> Did you? I can't possibly find these emails.

Stephen was using your arm.com email.

https://lore.kernel.org/linux-next/[email protected]/

>
>> On Friday I have also send v1 of removal of that platforms (need to
>> send v2)
>> https://lore.kernel.org/alsa-devel/[email protected]/
>>
>
> You want to remove exising platforms two days before the start of the
> merge window?
> I don't think this is acceptable with such short notice.

I also don't think that will go through.

>
>> That's why please really consider next steps and let us know.
>
> I think the only option is to revert (at least partially) the Xilinx
> series.

Unfortunately.

Thanks,
Michal

2020-03-30 09:13:27

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 30. 03. 20 11:03, Michal Simek wrote:
> On 30. 03. 20 10:45, Marc Zyngier wrote:
>> On 2020-03-30 09:32, Michal Simek wrote:
>>> Hi Thomas and Marc,
>>>
>>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>>> The following commit has been merged into the irq/core branch of tip:
>>>>
>>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>>> Gitweb:       
>>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>>> Author:        Michal Simek <[email protected]>
>>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>>> Committer:     Marc Zyngier <[email protected]>
>>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>>
>>>> irqchip/xilinx: Enable generic irq multi handler
>>>>
>>>> Register default arch handler via driver instead of directly pointing to
>>>> xilinx intc controller. This patch makes architecture code more generic.
>>>>
>>>> Driver calls generic domain specific irq handler which does the most of
>>>> things self. Also get rid of concurrent_irq counting which hasn't been
>>>> exported anywhere.
>>>> Based on this loop was also optimized by using do/while loop instead of
>>>> goto loop.
>>>>
>>>> Signed-off-by: Michal Simek <[email protected]>
>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>>> Link:
>>>> https://lore.kernel.org/r/[email protected]
>>>>
>>>> ---
>>>>  arch/microblaze/Kconfig           |  2 ++-
>>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>>  drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
>>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>>> index 6a331bd..242f58e 100644
>>>> --- a/arch/microblaze/Kconfig
>>>> +++ b/arch/microblaze/Kconfig
>>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>>      select CPU_NO_EFFICIENT_FFS
>>>>      select MMU_GATHER_NO_RANGE if MMU
>>>>      select SPARSE_IRQ
>>>> +    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 eac2fb4..5166f08 100644
>>>> --- a/arch/microblaze/include/asm/irq.h
>>>> +++ b/arch/microblaze/include/asm/irq.h
>>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>>> --- a/arch/microblaze/kernel/irq.c
>>>> +++ b/arch/microblaze/kernel/irq.c
>>>> @@ -20,29 +20,10 @@
>>>>  #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;
>>>> -    }
>>>> -
>>>> -    irq_exit();
>>>> -    set_irq_regs(old_regs);
>>>> +    handle_arch_irq(regs);
>>>>      trace_hardirqs_on();
>>>>  }
>>>>
>>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>>> b/drivers/irqchip/irq-xilinx-intc.c
>>>> index 1d3d273..ea74181 100644
>>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>>> xintc_irq_chip *irqc)
>>>>      return irq;
>>>>  }
>>>>
>>>> -unsigned int xintc_get_irq(void)
>>>> -{
>>>> -    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);
>>>> -
>>>> -    return irq;
>>>> -}
>>>> -

One more thing. We could also get this function back and it will be fine
too. But up2you.

Thanks,
Michal



2020-03-30 09:15:07

by Marc Zyngier

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 2020-03-30 10:03, Michal Simek wrote:
> On 30. 03. 20 10:45, Marc Zyngier wrote:
>> On 2020-03-30 09:32, Michal Simek wrote:
>>> Hi Thomas and Marc,
>>>
>>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>>> The following commit has been merged into the irq/core branch of
>>>> tip:
>>>>
>>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>>> Gitweb:       
>>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>>> Author:        Michal Simek <[email protected]>
>>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>>> Committer:     Marc Zyngier <[email protected]>
>>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>>
>>>> irqchip/xilinx: Enable generic irq multi handler
>>>>
>>>> Register default arch handler via driver instead of directly
>>>> pointing to
>>>> xilinx intc controller. This patch makes architecture code more
>>>> generic.
>>>>
>>>> Driver calls generic domain specific irq handler which does the most
>>>> of
>>>> things self. Also get rid of concurrent_irq counting which hasn't
>>>> been
>>>> exported anywhere.
>>>> Based on this loop was also optimized by using do/while loop instead
>>>> of
>>>> goto loop.
>>>>
>>>> Signed-off-by: Michal Simek <[email protected]>
>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>>> Link:
>>>> https://lore.kernel.org/r/[email protected]
>>>>
>>>> ---
>>>>  arch/microblaze/Kconfig           |  2 ++-
>>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>>  drivers/irqchip/irq-xilinx-intc.c | 34
>>>> +++++++++++++++++-------------
>>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>>> index 6a331bd..242f58e 100644
>>>> --- a/arch/microblaze/Kconfig
>>>> +++ b/arch/microblaze/Kconfig
>>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>>      select CPU_NO_EFFICIENT_FFS
>>>>      select MMU_GATHER_NO_RANGE if MMU
>>>>      select SPARSE_IRQ
>>>> +    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 eac2fb4..5166f08 100644
>>>> --- a/arch/microblaze/include/asm/irq.h
>>>> +++ b/arch/microblaze/include/asm/irq.h
>>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>>> --- a/arch/microblaze/kernel/irq.c
>>>> +++ b/arch/microblaze/kernel/irq.c
>>>> @@ -20,29 +20,10 @@
>>>>  #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;
>>>> -    }
>>>> -
>>>> -    irq_exit();
>>>> -    set_irq_regs(old_regs);
>>>> +    handle_arch_irq(regs);
>>>>      trace_hardirqs_on();
>>>>  }
>>>>
>>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>>> b/drivers/irqchip/irq-xilinx-intc.c
>>>> index 1d3d273..ea74181 100644
>>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>>> xintc_irq_chip *irqc)
>>>>      return irq;
>>>>  }
>>>>
>>>> -unsigned int xintc_get_irq(void)
>>>> -{
>>>> -    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);
>>>> -
>>>> -    return irq;
>>>> -}
>>>> -
>>>>  static int xintc_map(struct irq_domain *d, unsigned int irq,
>>>> irq_hw_number_t hw)
>>>>  {
>>>>      struct xintc_irq_chip *irqc = d->host_data;
>>>> @@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct
>>>> irq_desc
>>>> *desc)
>>>>      chained_irq_exit(chip, desc);
>>>>  }
>>>>
>>>> +static void xil_intc_handle_irq(struct pt_regs *regs)
>>>> +{
>>>> +    u32 hwirq;
>>>> +    struct xintc_irq_chip *irqc = primary_intc;
>>>> +
>>>> +    do {
>>>> +        hwirq = xintc_read(irqc, IVR);
>>>> +        if (likely(hwirq != -1U)) {
>>>> +            int ret;
>>>> +
>>>> +            ret = handle_domain_irq(irqc->root_domain, hwirq,
>>>> regs);
>>>> +            WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        break;
>>>> +    } while (1);
>>>> +}
>>>> +
>>>>  static int __init xilinx_intc_of_init(struct device_node *intc,
>>>>                           struct device_node *parent)
>>>>  {
>>>> @@ -246,6 +251,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;
>>>>
>>>
>>> Stephen reported compilation issue when this patch is applied because
>>> of
>>> removal of xintc_get_irq() which is also used by ancient
>>> ppc405/ppc440
>>> xilinx platforms. I have reported this twice to Marc already last
>>> week.
>>
>> Did you? I can't possibly find these emails.
>
> Stephen was using your arm.com email.
>
> https://lore.kernel.org/linux-next/[email protected]/

Yeah, no chance I could read that, unfortunately. I've sent a separate
email to Stephen to update my email address.

>>
>>> On Friday I have also send v1 of removal of that platforms (need to
>>> send v2)
>>> https://lore.kernel.org/alsa-devel/[email protected]/
>>>
>>
>> You want to remove exising platforms two days before the start of the
>> merge window?
>> I don't think this is acceptable with such short notice.
>
> I also don't think that will go through.
>
>>
>>> That's why please really consider next steps and let us know.
>>
>> I think the only option is to revert (at least partially) the Xilinx
>> series.
>
> Unfortunately.

Can you please check that we're OK reverting just the last two patches?
Or what is the minimal revert that can be done for everything to keep
working?

M.
--
Jazz is not dead. It just smells funny...

2020-03-30 09:20:04

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 30. 03. 20 11:14, Marc Zyngier wrote:
> On 2020-03-30 10:03, Michal Simek wrote:
>> On 30. 03. 20 10:45, Marc Zyngier wrote:
>>> On 2020-03-30 09:32, Michal Simek wrote:
>>>> Hi Thomas and Marc,
>>>>
>>>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>>>> The following commit has been merged into the irq/core branch of tip:
>>>>>
>>>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>> Gitweb:       
>>>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>> Author:        Michal Simek <[email protected]>
>>>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>>>> Committer:     Marc Zyngier <[email protected]>
>>>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>>>
>>>>> irqchip/xilinx: Enable generic irq multi handler
>>>>>
>>>>> Register default arch handler via driver instead of directly
>>>>> pointing to
>>>>> xilinx intc controller. This patch makes architecture code more
>>>>> generic.
>>>>>
>>>>> Driver calls generic domain specific irq handler which does the
>>>>> most of
>>>>> things self. Also get rid of concurrent_irq counting which hasn't been
>>>>> exported anywhere.
>>>>> Based on this loop was also optimized by using do/while loop
>>>>> instead of
>>>>> goto loop.
>>>>>
>>>>> Signed-off-by: Michal Simek <[email protected]>
>>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>>>> Link:
>>>>> https://lore.kernel.org/r/[email protected]
>>>>>
>>>>>
>>>>> ---
>>>>>  arch/microblaze/Kconfig           |  2 ++-
>>>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>>>  drivers/irqchip/irq-xilinx-intc.c | 34 +++++++++++++++++-------------
>>>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>>>
>>>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>>>> index 6a331bd..242f58e 100644
>>>>> --- a/arch/microblaze/Kconfig
>>>>> +++ b/arch/microblaze/Kconfig
>>>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>>>      select CPU_NO_EFFICIENT_FFS
>>>>>      select MMU_GATHER_NO_RANGE if MMU
>>>>>      select SPARSE_IRQ
>>>>> +    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 eac2fb4..5166f08 100644
>>>>> --- a/arch/microblaze/include/asm/irq.h
>>>>> +++ b/arch/microblaze/include/asm/irq.h
>>>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>>>> --- a/arch/microblaze/kernel/irq.c
>>>>> +++ b/arch/microblaze/kernel/irq.c
>>>>> @@ -20,29 +20,10 @@
>>>>>  #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;
>>>>> -    }
>>>>> -
>>>>> -    irq_exit();
>>>>> -    set_irq_regs(old_regs);
>>>>> +    handle_arch_irq(regs);
>>>>>      trace_hardirqs_on();
>>>>>  }
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>>>> b/drivers/irqchip/irq-xilinx-intc.c
>>>>> index 1d3d273..ea74181 100644
>>>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>>>> xintc_irq_chip *irqc)
>>>>>      return irq;
>>>>>  }
>>>>>
>>>>> -unsigned int xintc_get_irq(void)
>>>>> -{
>>>>> -    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);
>>>>> -
>>>>> -    return irq;
>>>>> -}
>>>>> -
>>>>>  static int xintc_map(struct irq_domain *d, unsigned int irq,
>>>>> irq_hw_number_t hw)
>>>>>  {
>>>>>      struct xintc_irq_chip *irqc = d->host_data;
>>>>> @@ -177,6 +163,25 @@ static void xil_intc_irq_handler(struct irq_desc
>>>>> *desc)
>>>>>      chained_irq_exit(chip, desc);
>>>>>  }
>>>>>
>>>>> +static void xil_intc_handle_irq(struct pt_regs *regs)
>>>>> +{
>>>>> +    u32 hwirq;
>>>>> +    struct xintc_irq_chip *irqc = primary_intc;
>>>>> +
>>>>> +    do {
>>>>> +        hwirq = xintc_read(irqc, IVR);
>>>>> +        if (likely(hwirq != -1U)) {
>>>>> +            int ret;
>>>>> +
>>>>> +            ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
>>>>> +            WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
>>>>> +            continue;
>>>>> +        }
>>>>> +
>>>>> +        break;
>>>>> +    } while (1);
>>>>> +}
>>>>> +
>>>>>  static int __init xilinx_intc_of_init(struct device_node *intc,
>>>>>                           struct device_node *parent)
>>>>>  {
>>>>> @@ -246,6 +251,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;
>>>>>
>>>>
>>>> Stephen reported compilation issue when this patch is applied
>>>> because of
>>>> removal of xintc_get_irq() which is also used by ancient ppc405/ppc440
>>>> xilinx platforms. I have reported this twice to Marc already last week.
>>>
>>> Did you? I can't possibly find these emails.
>>
>> Stephen was using your arm.com email.
>>
>> https://lore.kernel.org/linux-next/[email protected]/
>>
>
> Yeah, no chance I could read that, unfortunately. I've sent a separate
> email to Stephen to update my email address.
>
>>>
>>>> On Friday I have also send v1 of removal of that platforms (need to
>>>> send v2)
>>>> https://lore.kernel.org/alsa-devel/[email protected]/
>>>>
>>>>
>>>
>>> You want to remove exising platforms two days before the start of the
>>> merge window?
>>> I don't think this is acceptable with such short notice.
>>
>> I also don't think that will go through.
>>
>>>
>>>> That's why please really consider next steps and let us know.
>>>
>>> I think the only option is to revert (at least partially) the Xilinx
>>> series.
>>
>> Unfortunately.
>
> Can you please check that we're OK reverting just the last two patches?
> Or what is the minimal revert that can be done for everything to keep
> working?

It looks like that none has any issue to remove these platforms. It
means we can just get back that function with comment and remove it
later when both xilinx ppc platforms are removed. What do you think?
Better to revert one patch instead of two.

From Stephen:
Caused by commit

a0789993bf82 ("irqchip/xilinx: Enable generic irq multi handler")

I have reverted that commit (and commit

9c2d4f525c00 ("irqchip/xilinx: Do not call irq_set_default_host()")

that conflicted with the other revert).

Thanks,
Michal

2020-03-30 09:20:14

by Marc Zyngier

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 2020-03-30 10:12, Michal Simek wrote:
> On 30. 03. 20 11:03, Michal Simek wrote:
>> On 30. 03. 20 10:45, Marc Zyngier wrote:
>>> On 2020-03-30 09:32, Michal Simek wrote:
>>>> Hi Thomas and Marc,
>>>>
>>>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>>>> The following commit has been merged into the irq/core branch of
>>>>> tip:
>>>>>
>>>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>> Gitweb:       
>>>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>> Author:        Michal Simek <[email protected]>
>>>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>>>> Committer:     Marc Zyngier <[email protected]>
>>>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>>>
>>>>> irqchip/xilinx: Enable generic irq multi handler
>>>>>
>>>>> Register default arch handler via driver instead of directly
>>>>> pointing to
>>>>> xilinx intc controller. This patch makes architecture code more
>>>>> generic.
>>>>>
>>>>> Driver calls generic domain specific irq handler which does the
>>>>> most of
>>>>> things self. Also get rid of concurrent_irq counting which hasn't
>>>>> been
>>>>> exported anywhere.
>>>>> Based on this loop was also optimized by using do/while loop
>>>>> instead of
>>>>> goto loop.
>>>>>
>>>>> Signed-off-by: Michal Simek <[email protected]>
>>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>>>> Link:
>>>>> https://lore.kernel.org/r/[email protected]
>>>>>
>>>>> ---
>>>>>  arch/microblaze/Kconfig           |  2 ++-
>>>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>>>  drivers/irqchip/irq-xilinx-intc.c | 34
>>>>> +++++++++++++++++-------------
>>>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>>>
>>>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>>>> index 6a331bd..242f58e 100644
>>>>> --- a/arch/microblaze/Kconfig
>>>>> +++ b/arch/microblaze/Kconfig
>>>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>>>      select CPU_NO_EFFICIENT_FFS
>>>>>      select MMU_GATHER_NO_RANGE if MMU
>>>>>      select SPARSE_IRQ
>>>>> +    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 eac2fb4..5166f08 100644
>>>>> --- a/arch/microblaze/include/asm/irq.h
>>>>> +++ b/arch/microblaze/include/asm/irq.h
>>>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>>>> --- a/arch/microblaze/kernel/irq.c
>>>>> +++ b/arch/microblaze/kernel/irq.c
>>>>> @@ -20,29 +20,10 @@
>>>>>  #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;
>>>>> -    }
>>>>> -
>>>>> -    irq_exit();
>>>>> -    set_irq_regs(old_regs);
>>>>> +    handle_arch_irq(regs);
>>>>>      trace_hardirqs_on();
>>>>>  }
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>>>> b/drivers/irqchip/irq-xilinx-intc.c
>>>>> index 1d3d273..ea74181 100644
>>>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>>>> xintc_irq_chip *irqc)
>>>>>      return irq;
>>>>>  }
>>>>>
>>>>> -unsigned int xintc_get_irq(void)
>>>>> -{
>>>>> -    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);
>>>>> -
>>>>> -    return irq;
>>>>> -}
>>>>> -
>
> One more thing. We could also get this function back and it will be
> fine
> too. But up2you.

If you leave it up to me, I'll revert the whole series right now.

What I'd expect from you is to tell me exactly what is the minimal
change that keeps it working on both ARM, microblaze and PPC.
If it is a revert, tell me which patches to revert. if it is a patch
on top, send me the fix so that I can queue it now.

M.
--
Jazz is not dead. It just smells funny...

2020-03-30 09:29:56

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 30. 03. 20 11:19, Marc Zyngier wrote:
> On 2020-03-30 10:12, Michal Simek wrote:
>> On 30. 03. 20 11:03, Michal Simek wrote:
>>> On 30. 03. 20 10:45, Marc Zyngier wrote:
>>>> On 2020-03-30 09:32, Michal Simek wrote:
>>>>> Hi Thomas and Marc,
>>>>>
>>>>> On 29. 03. 20 22:26, tip-bot2 for Michal Simek wrote:
>>>>>> The following commit has been merged into the irq/core branch of tip:
>>>>>>
>>>>>> Commit-ID:     a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>>> Gitweb:       
>>>>>> https://git.kernel.org/tip/a0789993bf8266e62fea6b4613945ba081c71e7d
>>>>>> Author:        Michal Simek <[email protected]>
>>>>>> AuthorDate:    Tue, 17 Mar 2020 18:25:59 +05:30
>>>>>> Committer:     Marc Zyngier <[email protected]>
>>>>>> CommitterDate: Sun, 22 Mar 2020 11:52:53
>>>>>>
>>>>>> irqchip/xilinx: Enable generic irq multi handler
>>>>>>
>>>>>> Register default arch handler via driver instead of directly
>>>>>> pointing to
>>>>>> xilinx intc controller. This patch makes architecture code more
>>>>>> generic.
>>>>>>
>>>>>> Driver calls generic domain specific irq handler which does the
>>>>>> most of
>>>>>> things self. Also get rid of concurrent_irq counting which hasn't
>>>>>> been
>>>>>> exported anywhere.
>>>>>> Based on this loop was also optimized by using do/while loop
>>>>>> instead of
>>>>>> goto loop.
>>>>>>
>>>>>> Signed-off-by: Michal Simek <[email protected]>
>>>>>> Signed-off-by: Marc Zyngier <[email protected]>
>>>>>> Reviewed-by: Stefan Asserhall <[email protected]>
>>>>>> Link:
>>>>>> https://lore.kernel.org/r/[email protected]
>>>>>>
>>>>>>
>>>>>> ---
>>>>>>  arch/microblaze/Kconfig           |  2 ++-
>>>>>>  arch/microblaze/include/asm/irq.h |  3 +---
>>>>>>  arch/microblaze/kernel/irq.c      | 21 +-------------------
>>>>>>  drivers/irqchip/irq-xilinx-intc.c | 34
>>>>>> +++++++++++++++++-------------
>>>>>>  4 files changed, 23 insertions(+), 37 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>>>>>> index 6a331bd..242f58e 100644
>>>>>> --- a/arch/microblaze/Kconfig
>>>>>> +++ b/arch/microblaze/Kconfig
>>>>>> @@ -47,6 +47,8 @@ config MICROBLAZE
>>>>>>      select CPU_NO_EFFICIENT_FFS
>>>>>>      select MMU_GATHER_NO_RANGE if MMU
>>>>>>      select SPARSE_IRQ
>>>>>> +    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 eac2fb4..5166f08 100644
>>>>>> --- a/arch/microblaze/include/asm/irq.h
>>>>>> +++ b/arch/microblaze/include/asm/irq.h
>>>>>> @@ -14,7 +14,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 903dad8..0b37dde 100644
>>>>>> --- a/arch/microblaze/kernel/irq.c
>>>>>> +++ b/arch/microblaze/kernel/irq.c
>>>>>> @@ -20,29 +20,10 @@
>>>>>>  #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;
>>>>>> -    }
>>>>>> -
>>>>>> -    irq_exit();
>>>>>> -    set_irq_regs(old_regs);
>>>>>> +    handle_arch_irq(regs);
>>>>>>      trace_hardirqs_on();
>>>>>>  }
>>>>>>
>>>>>> diff --git a/drivers/irqchip/irq-xilinx-intc.c
>>>>>> b/drivers/irqchip/irq-xilinx-intc.c
>>>>>> index 1d3d273..ea74181 100644
>>>>>> --- a/drivers/irqchip/irq-xilinx-intc.c
>>>>>> +++ b/drivers/irqchip/irq-xilinx-intc.c
>>>>>> @@ -124,20 +124,6 @@ static unsigned int xintc_get_irq_local(struct
>>>>>> xintc_irq_chip *irqc)
>>>>>>      return irq;
>>>>>>  }
>>>>>>
>>>>>> -unsigned int xintc_get_irq(void)
>>>>>> -{
>>>>>> -    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);
>>>>>> -
>>>>>> -    return irq;
>>>>>> -}
>>>>>> -
>>
>> One more thing. We could also get this function back and it will be fine
>> too. But up2you.
>
> If you leave it up to me, I'll revert the whole series right now.
>
> What I'd expect from you is to tell me exactly what is the minimal
> change that keeps it working on both ARM, microblaze and PPC.
> If it is a revert, tell me which patches to revert. if it is a patch
> on top, send me the fix so that I can queue it now.

It won't be that simple. Please revert patches

9c2d4f525c00 ("irqchip/xilinx: Do not call irq_set_default_host()")
a0789993bf82 ("irqchip/xilinx: Enable generic irq multi handler")

And we should be fine.

Thanks,
Michal


2020-03-30 10:05:14

by Marc Zyngier

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 2020-03-30 10:27, Michal Simek wrote:
> On 30. 03. 20 11:19, Marc Zyngier wrote:
>> On 2020-03-30 10:12, Michal Simek wrote:
>>> On 30. 03. 20 11:03, Michal Simek wrote:

[...]

>>> One more thing. We could also get this function back and it will be
>>> fine
>>> too. But up2you.
>>
>> If you leave it up to me, I'll revert the whole series right now.
>>
>> What I'd expect from you is to tell me exactly what is the minimal
>> change that keeps it working on both ARM, microblaze and PPC.
>> If it is a revert, tell me which patches to revert. if it is a patch
>> on top, send me the fix so that I can queue it now.
>
> It won't be that simple. Please revert patches
>
> 9c2d4f525c00 ("irqchip/xilinx: Do not call irq_set_default_host()")
> a0789993bf82 ("irqchip/xilinx: Enable generic irq multi handler")
>
> And we should be fine.

Now reverted and pushed out. I'll send a pull request to Thomas
tomorrow.

M.
--
Jazz is not dead. It just smells funny...

2020-03-30 10:12:57

by Michal Simek

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

On 30. 03. 20 12:04, Marc Zyngier wrote:
> On 2020-03-30 10:27, Michal Simek wrote:
>> On 30. 03. 20 11:19, Marc Zyngier wrote:
>>> On 2020-03-30 10:12, Michal Simek wrote:
>>>> On 30. 03. 20 11:03, Michal Simek wrote:
>
> [...]
>
>>>> One more thing. We could also get this function back and it will be
>>>> fine
>>>> too. But up2you.
>>>
>>> If you leave it up to me, I'll revert the whole series right now.
>>>
>>> What I'd expect from you is to tell me exactly what is the minimal
>>> change that keeps it working on both ARM, microblaze and PPC.
>>> If it is a revert, tell me which patches to revert. if it is a patch
>>> on top, send me the fix so that I can queue it now.
>>
>> It won't be that simple. Please revert patches
>>
>> 9c2d4f525c00 ("irqchip/xilinx: Do not call irq_set_default_host()")
>> a0789993bf82 ("irqchip/xilinx: Enable generic irq multi handler")
>>
>> And we should be fine.
>
> Now reverted and pushed out. I'll send a pull request to Thomas tomorrow.

Thanks,
Michal

2020-03-31 20:48:21

by Stephen Rothwell

[permalink] [raw]
Subject: Re: [tip: irq/core] irqchip/xilinx: Enable generic irq multi handler

Hi Marc,

On Mon, 30 Mar 2020 11:04:14 +0100 Marc Zyngier <[email protected]> wrote:
>
> On 2020-03-30 10:27, Michal Simek wrote:
> > On 30. 03. 20 11:19, Marc Zyngier wrote:
> >> On 2020-03-30 10:12, Michal Simek wrote:
> >>> On 30. 03. 20 11:03, Michal Simek wrote:
>
> [...]
>
> >>> One more thing. We could also get this function back and it will be
> >>> fine
> >>> too. But up2you.
> >>
> >> If you leave it up to me, I'll revert the whole series right now.
> >>
> >> What I'd expect from you is to tell me exactly what is the minimal
> >> change that keeps it working on both ARM, microblaze and PPC.
> >> If it is a revert, tell me which patches to revert. if it is a patch
> >> on top, send me the fix so that I can queue it now.
> >
> > It won't be that simple. Please revert patches
> >
> > 9c2d4f525c00 ("irqchip/xilinx: Do not call irq_set_default_host()")
> > a0789993bf82 ("irqchip/xilinx: Enable generic irq multi handler")
> >
> > And we should be fine.
>
> Now reverted and pushed out. I'll send a pull request to Thomas
> tomorrow.

Unfortunately, those commits made it to Linus' tree without the reverts :-(

--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature
Subject: [tip: irq/urgent] Revert "irqchip/xilinx: Enable generic irq multi handler"

The following commit has been merged into the irq/urgent branch of tip:

Commit-ID: 4cea749d56bec9409f3bd126d2b2f949dc6c66e2
Gitweb: https://git.kernel.org/tip/4cea749d56bec9409f3bd126d2b2f949dc6c66e2
Author: Marc Zyngier <[email protected]>
AuthorDate: Mon, 30 Mar 2020 10:43:59 +01:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Wed, 01 Apr 2020 09:12:24 +01:00

Revert "irqchip/xilinx: Enable generic irq multi handler"

This reverts commit a0789993bf8266e62fea6b4613945ba081c71e7d, which
breaks a number of PPC platforms.

Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/microblaze/Kconfig | 2 +--
arch/microblaze/include/asm/irq.h | 3 +++-
arch/microblaze/kernel/irq.c | 21 ++++++++++++++++++-
drivers/irqchip/irq-xilinx-intc.c | 34 ++++++++++++------------------
4 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 242f58e..6a331bd 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -47,8 +47,6 @@ config MICROBLAZE
select CPU_NO_EFFICIENT_FFS
select MMU_GATHER_NO_RANGE if MMU
select SPARSE_IRQ
- 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 5166f08..eac2fb4 100644
--- a/arch/microblaze/include/asm/irq.h
+++ b/arch/microblaze/include/asm/irq.h
@@ -14,4 +14,7 @@
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 0b37dde..903dad8 100644
--- a/arch/microblaze/kernel/irq.c
+++ b/arch/microblaze/kernel/irq.c
@@ -20,10 +20,29 @@
#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();
- handle_arch_irq(regs);
+
+ 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;
+ }
+
+ 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 ea74181..1d3d273 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -124,6 +124,20 @@ static unsigned int xintc_get_irq_local(struct xintc_irq_chip *irqc)
return irq;
}

+unsigned int xintc_get_irq(void)
+{
+ 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);
+
+ return irq;
+}
+
static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
{
struct xintc_irq_chip *irqc = d->host_data;
@@ -163,25 +177,6 @@ static void xil_intc_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

-static void xil_intc_handle_irq(struct pt_regs *regs)
-{
- u32 hwirq;
- struct xintc_irq_chip *irqc = primary_intc;
-
- do {
- hwirq = xintc_read(irqc, IVR);
- if (likely(hwirq != -1U)) {
- int ret;
-
- ret = handle_domain_irq(irqc->root_domain, hwirq, regs);
- WARN_ONCE(ret, "Unhandled HWIRQ %d\n", hwirq);
- continue;
- }
-
- break;
- } while (1);
-}
-
static int __init xilinx_intc_of_init(struct device_node *intc,
struct device_node *parent)
{
@@ -251,7 +246,6 @@ 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;

Subject: [tip: irq/urgent] Revert "irqchip/xilinx: Do not call irq_set_default_host()"

The following commit has been merged into the irq/urgent branch of tip:

Commit-ID: e02f6c01748df77b3fe202bbf7dde0aae6ecced7
Gitweb: https://git.kernel.org/tip/e02f6c01748df77b3fe202bbf7dde0aae6ecced7
Author: Marc Zyngier <[email protected]>
AuthorDate: Mon, 30 Mar 2020 10:41:58 +01:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Wed, 01 Apr 2020 09:10:45 +01:00

Revert "irqchip/xilinx: Do not call irq_set_default_host()"

This reverts commit 9c2d4f525c002591f4e0c14a37663663aaba1656, which
breaks a number of PPC platforms.

Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-xilinx-intc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 7f811fe..ea74181 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -250,6 +250,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);
}