* [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
@ 2015-07-06 10:18 Thomas Gleixner
2015-07-06 12:07 ` Jisheng Zhang
2015-07-11 21:30 ` [tip:irq/core] " tip-bot for Thomas Gleixner
0 siblings, 2 replies; 11+ messages in thread
From: Thomas Gleixner @ 2015-07-06 10:18 UTC (permalink / raw)
To: LKML; +Cc: Sebastian Hesselbarth, Jisheng Zhang, Mark Rutland, Jason Cooper
[-- Attachment #1: irqchip-dw-apb-ictl-fix-wreckage.patch --]
[-- Type: text/plain, Size: 4479 bytes --]
The num_ct argument of irq_alloc_domain_generic_chips() tells the core
code how many chip types (for different control flows,
e.g. edge/level) should be allocated. It does not control how many
generic chip instances are created because that's determined from the
irq domain size and the number of interrupts per chip.
The dw-apb init abuses the num_ct argument for allocating one or two
chip types depending on the number of interrupts. That's completely
wrong because the alternate type is never used.
This code was obviously never tested on a system which has more than
32 interrupts as that would have never worked due to the unitialized
second generic chip instance.
Hand in the proper num_ct=1 and fixup the chip initialization along
with the interrupt handler.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Jisheng Zhang <jszhang@marvell.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
---
drivers/irqchip/irq-dw-apb-ictl.c | 53 +++++++++++++++-----------------------
1 file changed, 22 insertions(+), 31 deletions(-)
Index: tip/drivers/irqchip/irq-dw-apb-ictl.c
===================================================================
--- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c
+++ tip/drivers/irqchip/irq-dw-apb-ictl.c
@@ -25,24 +25,25 @@
#define APB_INT_MASK_H 0x0c
#define APB_INT_FINALSTATUS_L 0x30
#define APB_INT_FINALSTATUS_H 0x34
+#define APB_INT_BASE_OFFSET 0x04
static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
{
- struct irq_chip *chip = irq_get_chip(irq);
- struct irq_chip_generic *gc = irq_get_handler_data(irq);
- struct irq_domain *d = gc->private;
- u32 stat;
+ struct irq_domain *d = irq_desc_get_handler_data(desc);
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
int n;
chained_irq_enter(chip, desc);
- for (n = 0; n < gc->num_ct; n++) {
- stat = readl_relaxed(gc->reg_base +
- APB_INT_FINALSTATUS_L + 4 * n);
+ for (n = 0; n < d->gc->num_chips; n++, gc++) {
+ u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
while (stat) {
u32 hwirq = ffs(stat) - 1;
- generic_handle_irq(irq_find_mapping(d,
- gc->irq_base + hwirq + 32 * n));
+ u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
+
+ generic_handle_irq(virq);
stat &= ~(1 << hwirq);
}
}
@@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc
struct irq_domain *domain;
struct irq_chip_generic *gc;
void __iomem *iobase;
- int ret, nrirqs, irq;
+ int ret, nrirqs, irq, i;
u32 reg;
/* Map the parent interrupt for the chained handler */
@@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc
goto err_unmap;
}
- ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
- np->name, handle_level_irq, clr, 0,
- IRQ_GC_MASK_CACHE_PER_TYPE |
+ ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
+ handle_level_irq, clr, 0,
IRQ_GC_INIT_MASK_CACHE);
if (ret) {
pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
goto err_unmap;
}
- gc = irq_get_domain_generic_chip(domain, 0);
- gc->private = domain;
- gc->reg_base = iobase;
-
- gc->chip_types[0].regs.mask = APB_INT_MASK_L;
- gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
- gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
-
- if (nrirqs > 32) {
- gc->chip_types[1].regs.mask = APB_INT_MASK_H;
- gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
- gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
+ for (i = 0; i < nrirqs / 32; i++) {
+ gc = irq_get_domain_generic_chip(domain, i * 32);
+ gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
+ gc->chip_types[0].regs.mask = APB_INT_MASK_L;
+ gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
+ gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+ gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+ gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_handler_data(irq, gc);
- irq_set_chained_handler(irq, dw_apb_ictl_handler);
+ irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
return 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 10:18 [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage Thomas Gleixner
@ 2015-07-06 12:07 ` Jisheng Zhang
2015-07-06 12:31 ` Jisheng Zhang
2015-07-11 21:30 ` [tip:irq/core] " tip-bot for Thomas Gleixner
1 sibling, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2015-07-06 12:07 UTC (permalink / raw)
To: Thomas Gleixner, Sebastian Hesselbarth, Mark Rutland,
Jason Cooper; +Cc: LKML
Dear Thomas,
On Mon, 6 Jul 2015 10:18:28 +0000
Thomas Gleixner <tglx@linutronix.de> wrote:
> The num_ct argument of irq_alloc_domain_generic_chips() tells the core
> code how many chip types (for different control flows,
> e.g. edge/level) should be allocated. It does not control how many
> generic chip instances are created because that's determined from the
> irq domain size and the number of interrupts per chip.
>
> The dw-apb init abuses the num_ct argument for allocating one or two
> chip types depending on the number of interrupts. That's completely
> wrong because the alternate type is never used.
>
> This code was obviously never tested on a system which has more than
> 32 interrupts as that would have never worked due to the unitialized
> second generic chip instance.
>
> Hand in the proper num_ct=1 and fixup the chip initialization along
> with the interrupt handler.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Cc: Jisheng Zhang <jszhang@marvell.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> ---
> drivers/irqchip/irq-dw-apb-ictl.c | 53 +++++++++++++++-----------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> Index: tip/drivers/irqchip/irq-dw-apb-ictl.c
> ===================================================================
> --- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c
> +++ tip/drivers/irqchip/irq-dw-apb-ictl.c
> @@ -25,24 +25,25 @@
> #define APB_INT_MASK_H 0x0c
> #define APB_INT_FINALSTATUS_L 0x30
> #define APB_INT_FINALSTATUS_H 0x34
> +#define APB_INT_BASE_OFFSET 0x04
>
> static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> {
> - struct irq_chip *chip = irq_get_chip(irq);
> - struct irq_chip_generic *gc = irq_get_handler_data(irq);
> - struct irq_domain *d = gc->private;
> - u32 stat;
> + struct irq_domain *d = irq_desc_get_handler_data(desc);
> + struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> int n;
>
> chained_irq_enter(chip, desc);
>
> - for (n = 0; n < gc->num_ct; n++) {
> - stat = readl_relaxed(gc->reg_base +
> - APB_INT_FINALSTATUS_L + 4 * n);
> + for (n = 0; n < d->gc->num_chips; n++, gc++) {
> + u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
I simply tested this patch, got panic like the following, on berlin, gc->num_chips
is always 2, it seems that the the gc->reg_base is null when n=1.
[ 0.511313] Unable to handle kernel NULL pointer dereference at virtual address 00000030
[ 0.519669] pgd = c0004000
[ 0.522467] [00000030] *pgd=00000000
[ 0.526183] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 0.531671] Modules linked in:
[ 0.534849] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.0-next-20150703+ #14
[ 0.542306] Hardware name: Marvell Berlin
[ 0.546451] task: ee454000 ti: ee442000 task.ti: ee442000
[ 0.552037] PC is at dw_apb_ictl_handler+0xbc/0x120
[ 0.557084] LR is at console_unlock.part.19+0x31c/0x3c8
[ 0.562485] pc : [<c022a954>] lr : [<c0060200>] psr: 20000193
[ 0.562485] sp : ee443b00 ip : ee443a50 fp : ee443b34
[ 0.574337] r10: 00000000 r9 : 00000008 r8 : 00000001
[ 0.579736] r7 : ee402a64 r6 : ee405cc4 r5 : ee402800 r4 : c04e7f00
[ 0.586478] r3 : 00000000 r2 : 00010001 r1 : 60000193 r0 : 0000000a
[ 0.593223] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
[ 0.600861] Control: 10c5387d Table: 0100406a DAC: 00000015
[ 0.606797] Process swapper/0 (pid: 1, stack limit = 0xee442210)
[ 0.613002] Stack: (0xee443b00 to 0xee444000)
[ 0.617510] 3b00: 00000000 00000001 ee443b24 00000011 00000000 00000000 00000031 ee408000
[ 0.625960] 3b20: ee41d4bc 60000113 ee443b4c ee443b38 c006171c c022a8a4 0000001b c04d6e4c
[ 0.634410] 3b40: ee443b74 ee443b50 c0061848 c0061700 ee443ba0 ee443ba0 c04dcfb4 00000021
[ 0.642860] 3b60: ef80200c ef802000 ee443b9c ee443b78 c00093b4 c00617f8 c0063910 c02c3da8
[ 0.651310] 3b80: 60000113 ffffffff ee443bd4 0000001a ee443bf4 ee443ba0 c0013580 c0009398
[ 0.659760] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370
[ 0.668210] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8
[ 0.676659] 3be0: 60000113 ffffffff ee443c34 ee443bf8 c0063910 c02c3d8c c04ed084 c0515e80
[ 0.685109] 3c00: 00000080 00000080 ee443c24 ee5d2b40 00000080 ee41d480 c0247370 0000001a
[ 0.693559] 3c20: 00000000 ee5d2b00 ee443c64 ee443c38 c0063bd8 c006361c 00000080 c0515e80
[ 0.702009] 3c40: ee5d2b00 00000080 00000000 ee5d2b0c ee5e8800 c05164d8 ee443c94 ee443c68
[ 0.710459] 3c60: c024a280 c0063b10 c0357f4c ee5d2b00 c0515e80 00000002 00000002 20000113
[ 0.718908] 3c80: 00000000 00000000 ee443cac ee443c98 c024a35c c024a140 c024a2bc c0515e80
[ 0.727358] 3ca0: ee443cd4 ee443cb0 c0249348 c024a2c8 c0515e80 ee4a9400 00000000 ee5e8800
[ 0.735808] 3cc0: 00000000 00000000 ee443ce4 ee443cd8 c02498a0 c0249230 ee443d0c ee443ce8
[ 0.744258] 3ce0: c02442e8 c0249888 00000002 ee4a9400 ee5e8800 ee4a949c ee4b3d80 ee5e895c
[ 0.752708] 3d00: ee443d34 ee443d10 c0244f44 c0244298 ee4b3d80 ee4408c8 c02e6610 00500001
[ 0.761158] 3d20: 00000002 ee5e8800 ee443d74 ee443d38 c0239820 c0244e74 c0254ee4 ee533000
[ 0.769608] 3d40: 00000001 00000000 000000d0 ee4408c8 ee4b3d80 c02e675c c0515ce8 c0515ce8
[ 0.778058] 3d60: ee4b3d80 ee001d48 ee443da4 ee443d78 c00ecfdc c02397f4 00000000 00000000
[ 0.786507] 3d80: ee443da4 ee4b3d80 ee4408c8 ee4b3d88 c00ecf2c 00000000 ee443dcc ee443da8
[ 0.794957] 3da0: c00e74cc c00ecf38 ee4b3d80 ee443f34 00000002 ee443e5c 00000000 ee001d48
[ 0.803407] 3dc0: ee443de4 ee443dd0 c00e83e8 c00e73e8 ee454000 ee443e88 ee443e4c ee443de8
[ 0.811857] 3de0: c00f57a0 c00e8390 ee443e88 ee413015 ee443e0c ee443e00 ee443e18 00000000
[ 0.820306] 3e00: 00000000 00000026 00000004 ee4408c8 00000000 00000000 ee40c250 ee001e58
[ 0.828756] 3e20: c00f3dc0 ee443e88 ee4b3d80 ee443f34 00000041 00000000 00000000 00000000
[ 0.837205] 3e40: ee443e84 ee443e50 c00f5dd0 c00f5684 00000001 00000007 0000000e 00000000
[ 0.845655] 3e60: 0000001a ee443f34 ee443e88 00000001 00000000 00000000 ee443f24 ee443e88
[ 0.854105] 3e80: c00f6c1c c00f5d68 ee40c250 ee001e58 0f4756ea 00000007 ee413015 00000000
[ 0.862554] 3ea0: 00000000 ee001088 ee4408c8 00000101 00000004 00000016 00000000 00000000
[ 0.871004] 3ec0: 00000000 ee443ec8 ee41a018 00000000 ee443f14 ee443ee0 c0103478 c02c3d34
[ 0.879453] 3ee0: c00f6514 00000002 00000002 00000002 ee413000 ffffff9c ee413000 00000000
[ 0.887903] 3f00: 00000002 ffffff9c 00000000 ee413000 ffffff9c 00000000 ee443f6c ee443f28
[ 0.896353] 3f20: c00e8738 c00f6bc0 00000000 00000000 c03ade9c 00000002 00000000 00000026
[ 0.904802] 3f40: 00000100 00000001 c03ade9c c03ade9c 00000000 00000000 00000000 00000000
[ 0.913252] 3f60: ee443f7c ee443f70 c00e880c c00e8618 ee443f94 ee443f80 c038eef8 c00e87f4
[ 0.921702] 3f80: 00000000 c02b6138 ee443fac ee443f98 c02b6148 c038ee50 ee442000 00000000
[ 0.930151] 3fa0: 00000000 ee443fb0 c000fa08 c02b6144 00000000 00000000 00000000 00000000
[ 0.938600] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 0.947049] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 964ec888 865a0c8b
[ 0.955491] Backtrace:
[ 0.958044] [<c022a898>] (dw_apb_ictl_handler) from [<c006171c>] (generic_handle_irq+0x28/
0x38)
[ 0.967026] r10:60000113 r9:ee41d4bc r8:ee408000 r7:00000031 r6:00000000 r5:00000000
[ 0.975191] r4:00000011
[ 0.977832] [<c00616f4>] (generic_handle_irq) from [<c0061848>] (__handle_domain_irq+0x5c/
0xb4)
[ 0.986814] r4:c04d6e4c r3:0000001b
[ 0.990548] [<c00617ec>] (__handle_domain_irq) from [<c00093b4>] (gic_handle_irq+0x28/0x68
)
[ 0.999169] r8:ef802000 r7:ef80200c r6:00000021 r5:c04dcfb4 r4:ee443ba0 r3:ee443ba0
[ 1.007252] [<c000938c>] (gic_handle_irq) from [<c0013580>] (__irq_svc+0x40/0x74)
[ 1.014980] Exception stack(0xee443ba0 to 0xee443be8)
[ 1.020205] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370
[ 1.028655] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8
[ 1.037100] 3be0: 60000113 ffffffff
[ 1.040704] r8:0000001a r7:ee443bd4 r6:ffffffff r5:60000113 r4:c02c3da8 r3:c0063910
[ 1.048783] [<c02c3d80>] (_raw_spin_unlock_irqrestore) from [<c0063910>] (__setup_irq+0x30
0/0x4f4)
[ 1.058040] [<c0063610>] (__setup_irq) from [<c0063bd8>] (request_threaded_irq+0xd4/0x150)
[ 1.066573] r10:ee5d2b00 r9:00000000 r8:0000001a r7:c0247370 r6:ee41d480 r5:00000080
[ 1.074738] r4:ee5d2b40
[ 1.077381] [<c0063b04>] (request_threaded_irq) from [<c024a280>] (serial_link_irq_chain+0
x14c/0x188)
[ 1.086900] r10:c05164d8 r9:ee5e8800 r8:ee5d2b0c r7:00000000 r6:00000080 r5:ee5d2b00
[ 1.095066] r4:c0515e80 r3:00000080
[ 1.098802] [<c024a134>] (serial_link_irq_chain) from [<c024a35c>] (univ8250_setup_irq+0xa
0/0xac)
[ 1.107962] r10:00000000 r8:00000000 r7:20000113 r6:00000002 r5:00000002 r4:c0515e80
[ 1.116135] [<c024a2bc>] (univ8250_setup_irq) from [<c0249348>] (serial8250_do_startup+0x1
24/0x658)
[ 1.125475] r4:c0515e80 r3:c024a2bc
[ 1.129208] [<c0249224>] (serial8250_do_startup) from [<c02498a0>] (serial8250_startup+0x2
4/0x28)
[ 1.138369] r10:00000000 r8:00000000 r7:ee5e8800 r6:00000000 r5:ee4a9400 r4:c0515e80
[ 1.146544] [<c024987c>] (serial8250_startup) from [<c02442e8>] (uart_port_startup+0x5c/0x
140)
[ 1.155442] [<c024428c>] (uart_port_startup) from [<c0244f44>] (uart_open+0xdc/0x144)
[ 1.163527] r8:ee5e895c r7:ee4b3d80 r6:ee4a949c r5:ee5e8800 r4:ee4a9400 r3:00000002
[ 1.171610] [<c0244e68>] (uart_open) from [<c0239820>] (tty_open+0x38/0x344)
[ 1.178888] r10:ee5e8800 r8:00000002 r7:00500001 r6:c02e6610 r5:ee4408c8 r4:ee4b3d80
[ 1.187064] [<c02397e8>] (tty_open) from [<c00ecfdc>] (chrdev_open+0xb0/0x168)
[ 1.194521] r10:ee001d48 r9:ee4b3d80 r8:c0515ce8 r7:c0515ce8 r6:c02e675c r5:ee4b3d80
[ 1.202685] r4:ee4408c8
[ 1.205330] [<c00ecf2c>] (chrdev_open) from [<c00e74cc>] (do_dentry_open.isra.13+0xf0/0x2f
8)
[ 1.214042] r8:00000000 r7:c00ecf2c r6:ee4b3d88 r5:ee4408c8 r4:ee4b3d80
[ 1.221049] [<c00e73dc>] (do_dentry_open.isra.13) from [<c00e83e8>] (vfs_open+0x64/0x68)
[ 1.229403] r10:ee001d48 r8:00000000 r7:ee443e5c r6:00000002 r5:ee443f34 r4:ee4b3d80
[ 1.237577] [<c00e8384>] (vfs_open) from [<c00f57a0>] (do_last+0x128/0x6e4)
[ 1.244766] r4:ee443e88 r3:ee454000
[ 1.248497] [<c00f5678>] (do_last) from [<c00f5dd0>] (path_openat+0x74/0x140)
[ 1.255866] r10:00000000 r9:00000000 r8:00000000 r7:00000041 r6:ee443f34 r5:ee4b3d80
[ 1.264028] r4:ee443e88
[ 1.266667] [<c00f5d5c>] (path_openat) from [<c00f6c1c>] (do_filp_open+0x68/0xbc)
[ 1.274393] r8:00000000 r7:00000000 r6:00000001 r5:ee443e88 r4:ee443f34
[ 1.281398] [<c00f6bb4>] (do_filp_open) from [<c00e8738>] (do_sys_open+0x12c/0x1dc)
[ 1.289303] r7:00000000 r6:ffffff9c r5:ee413000 r4:00000000
[ 1.295219] [<c00e860c>] (do_sys_open) from [<c00e880c>] (SyS_open+0x24/0x28)
[ 1.302588] r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c03ade9c r4:c03ade9c
[ 1.310763] [<c00e87e8>] (SyS_open) from [<c038eef8>] (kernel_init_freeable+0xb4/0x140)
[ 1.319037] [<c038ee44>] (kernel_init_freeable) from [<c02b6148>] (kernel_init+0x10/0xec)
[ 1.327480] r5:c02b6138 r4:00000000
[ 1.331216] [<c02b6138>] (kernel_init) from [<c000fa08>] (ret_from_fork+0x14/0x2c)
[ 1.339031] r4:00000000 r3:ee442000
[ 1.342762] Code: e5972004 e59f0064 eb023931 e5973004 (e593a030)
> +
> while (stat) {
> u32 hwirq = ffs(stat) - 1;
> - generic_handle_irq(irq_find_mapping(d,
> - gc->irq_base + hwirq + 32 * n));
> + u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
> +
> + generic_handle_irq(virq);
> stat &= ~(1 << hwirq);
> }
> }
> @@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc
> struct irq_domain *domain;
> struct irq_chip_generic *gc;
> void __iomem *iobase;
> - int ret, nrirqs, irq;
> + int ret, nrirqs, irq, i;
> u32 reg;
>
> /* Map the parent interrupt for the chained handler */
> @@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc
> goto err_unmap;
> }
>
> - ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
> - np->name, handle_level_irq, clr, 0,
> - IRQ_GC_MASK_CACHE_PER_TYPE |
> + ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
> + handle_level_irq, clr, 0,
> IRQ_GC_INIT_MASK_CACHE);
> if (ret) {
> pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
> goto err_unmap;
> }
>
> - gc = irq_get_domain_generic_chip(domain, 0);
> - gc->private = domain;
> - gc->reg_base = iobase;
> -
> - gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> - gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> - gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> -
> - if (nrirqs > 32) {
> - gc->chip_types[1].regs.mask = APB_INT_MASK_H;
> - gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
> - gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
> - gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
> - gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
> + for (i = 0; i < nrirqs / 32; i++) {
> + gc = irq_get_domain_generic_chip(domain, i * 32);
> + gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
> + gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> + gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> + gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> }
>
> - irq_set_handler_data(irq, gc);
> - irq_set_chained_handler(irq, dw_apb_ictl_handler);
> + irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
>
> return 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 12:07 ` Jisheng Zhang
@ 2015-07-06 12:31 ` Jisheng Zhang
2015-07-06 12:36 ` Thomas Gleixner
0 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2015-07-06 12:31 UTC (permalink / raw)
To: Thomas Gleixner, Sebastian Hesselbarth, Mark Rutland,
Jason Cooper; +Cc: LKML
Dear Thomas,
On Mon, 6 Jul 2015 20:07:36 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:
> Dear Thomas,
>
> On Mon, 6 Jul 2015 10:18:28 +0000
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > The num_ct argument of irq_alloc_domain_generic_chips() tells the core
> > code how many chip types (for different control flows,
> > e.g. edge/level) should be allocated. It does not control how many
> > generic chip instances are created because that's determined from the
> > irq domain size and the number of interrupts per chip.
> >
> > The dw-apb init abuses the num_ct argument for allocating one or two
> > chip types depending on the number of interrupts. That's completely
> > wrong because the alternate type is never used.
> >
> > This code was obviously never tested on a system which has more than
> > 32 interrupts as that would have never worked due to the unitialized
> > second generic chip instance.
> >
> > Hand in the proper num_ct=1 and fixup the chip initialization along
> > with the interrupt handler.
> >
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> > Cc: Jisheng Zhang <jszhang@marvell.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Jason Cooper <jason@lakedaemon.net>
> > ---
> > drivers/irqchip/irq-dw-apb-ictl.c | 53 +++++++++++++++-----------------------
> > 1 file changed, 22 insertions(+), 31 deletions(-)
> >
> > Index: tip/drivers/irqchip/irq-dw-apb-ictl.c
> > ===================================================================
> > --- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c
> > +++ tip/drivers/irqchip/irq-dw-apb-ictl.c
> > @@ -25,24 +25,25 @@
> > #define APB_INT_MASK_H 0x0c
> > #define APB_INT_FINALSTATUS_L 0x30
> > #define APB_INT_FINALSTATUS_H 0x34
> > +#define APB_INT_BASE_OFFSET 0x04
> >
> > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> > {
> > - struct irq_chip *chip = irq_get_chip(irq);
> > - struct irq_chip_generic *gc = irq_get_handler_data(irq);
> > - struct irq_domain *d = gc->private;
> > - u32 stat;
> > + struct irq_domain *d = irq_desc_get_handler_data(desc);
> > + struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> > + struct irq_chip *chip = irq_desc_get_chip(desc);
> > int n;
> >
> > chained_irq_enter(chip, desc);
> >
> > - for (n = 0; n < gc->num_ct; n++) {
> > - stat = readl_relaxed(gc->reg_base +
> > - APB_INT_FINALSTATUS_L + 4 * n);
> > + for (n = 0; n < d->gc->num_chips; n++, gc++) {
> > + u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
>
> I simply tested this patch, got panic like the following, on berlin, gc->num_chips
> is always 2, it seems that the the gc->reg_base is null when n=1.
the following patch seems fix the panic, but I dunno whether it's correct or not,
could you please help to check?
Thanks,
Jisheng
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index f4a0e11..8d996cb 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -30,13 +30,14 @@
static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
{
struct irq_domain *d = irq_desc_get_handler_data(desc);
- struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
+ struct irq_chip_generic *gc;
struct irq_chip *chip = irq_desc_get_chip(desc);
int n;
chained_irq_enter(chip, desc);
- for (n = 0; n < d->gc->num_chips; n++, gc++) {
+ for (n = 0; n < d->gc->num_chips; n++) {
+ gc = irq_get_domain_generic_chip(d, n * 32);
u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
while (stat) {
>
> [ 0.511313] Unable to handle kernel NULL pointer dereference at virtual address 00000030
> [ 0.519669] pgd = c0004000
> [ 0.522467] [00000030] *pgd=00000000
> [ 0.526183] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 0.531671] Modules linked in:
> [ 0.534849] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.0-next-20150703+ #14
> [ 0.542306] Hardware name: Marvell Berlin
> [ 0.546451] task: ee454000 ti: ee442000 task.ti: ee442000
> [ 0.552037] PC is at dw_apb_ictl_handler+0xbc/0x120
> [ 0.557084] LR is at console_unlock.part.19+0x31c/0x3c8
> [ 0.562485] pc : [<c022a954>] lr : [<c0060200>] psr: 20000193
> [ 0.562485] sp : ee443b00 ip : ee443a50 fp : ee443b34
> [ 0.574337] r10: 00000000 r9 : 00000008 r8 : 00000001
> [ 0.579736] r7 : ee402a64 r6 : ee405cc4 r5 : ee402800 r4 : c04e7f00
> [ 0.586478] r3 : 00000000 r2 : 00010001 r1 : 60000193 r0 : 0000000a
> [ 0.593223] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
> [ 0.600861] Control: 10c5387d Table: 0100406a DAC: 00000015
> [ 0.606797] Process swapper/0 (pid: 1, stack limit = 0xee442210)
> [ 0.613002] Stack: (0xee443b00 to 0xee444000)
> [ 0.617510] 3b00: 00000000 00000001 ee443b24 00000011 00000000 00000000 00000031 ee408000
> [ 0.625960] 3b20: ee41d4bc 60000113 ee443b4c ee443b38 c006171c c022a8a4 0000001b c04d6e4c
> [ 0.634410] 3b40: ee443b74 ee443b50 c0061848 c0061700 ee443ba0 ee443ba0 c04dcfb4 00000021
> [ 0.642860] 3b60: ef80200c ef802000 ee443b9c ee443b78 c00093b4 c00617f8 c0063910 c02c3da8
> [ 0.651310] 3b80: 60000113 ffffffff ee443bd4 0000001a ee443bf4 ee443ba0 c0013580 c0009398
> [ 0.659760] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370
> [ 0.668210] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8
> [ 0.676659] 3be0: 60000113 ffffffff ee443c34 ee443bf8 c0063910 c02c3d8c c04ed084 c0515e80
> [ 0.685109] 3c00: 00000080 00000080 ee443c24 ee5d2b40 00000080 ee41d480 c0247370 0000001a
> [ 0.693559] 3c20: 00000000 ee5d2b00 ee443c64 ee443c38 c0063bd8 c006361c 00000080 c0515e80
> [ 0.702009] 3c40: ee5d2b00 00000080 00000000 ee5d2b0c ee5e8800 c05164d8 ee443c94 ee443c68
> [ 0.710459] 3c60: c024a280 c0063b10 c0357f4c ee5d2b00 c0515e80 00000002 00000002 20000113
> [ 0.718908] 3c80: 00000000 00000000 ee443cac ee443c98 c024a35c c024a140 c024a2bc c0515e80
> [ 0.727358] 3ca0: ee443cd4 ee443cb0 c0249348 c024a2c8 c0515e80 ee4a9400 00000000 ee5e8800
> [ 0.735808] 3cc0: 00000000 00000000 ee443ce4 ee443cd8 c02498a0 c0249230 ee443d0c ee443ce8
> [ 0.744258] 3ce0: c02442e8 c0249888 00000002 ee4a9400 ee5e8800 ee4a949c ee4b3d80 ee5e895c
> [ 0.752708] 3d00: ee443d34 ee443d10 c0244f44 c0244298 ee4b3d80 ee4408c8 c02e6610 00500001
> [ 0.761158] 3d20: 00000002 ee5e8800 ee443d74 ee443d38 c0239820 c0244e74 c0254ee4 ee533000
> [ 0.769608] 3d40: 00000001 00000000 000000d0 ee4408c8 ee4b3d80 c02e675c c0515ce8 c0515ce8
> [ 0.778058] 3d60: ee4b3d80 ee001d48 ee443da4 ee443d78 c00ecfdc c02397f4 00000000 00000000
> [ 0.786507] 3d80: ee443da4 ee4b3d80 ee4408c8 ee4b3d88 c00ecf2c 00000000 ee443dcc ee443da8
> [ 0.794957] 3da0: c00e74cc c00ecf38 ee4b3d80 ee443f34 00000002 ee443e5c 00000000 ee001d48
> [ 0.803407] 3dc0: ee443de4 ee443dd0 c00e83e8 c00e73e8 ee454000 ee443e88 ee443e4c ee443de8
> [ 0.811857] 3de0: c00f57a0 c00e8390 ee443e88 ee413015 ee443e0c ee443e00 ee443e18 00000000
> [ 0.820306] 3e00: 00000000 00000026 00000004 ee4408c8 00000000 00000000 ee40c250 ee001e58
> [ 0.828756] 3e20: c00f3dc0 ee443e88 ee4b3d80 ee443f34 00000041 00000000 00000000 00000000
> [ 0.837205] 3e40: ee443e84 ee443e50 c00f5dd0 c00f5684 00000001 00000007 0000000e 00000000
> [ 0.845655] 3e60: 0000001a ee443f34 ee443e88 00000001 00000000 00000000 ee443f24 ee443e88
> [ 0.854105] 3e80: c00f6c1c c00f5d68 ee40c250 ee001e58 0f4756ea 00000007 ee413015 00000000
> [ 0.862554] 3ea0: 00000000 ee001088 ee4408c8 00000101 00000004 00000016 00000000 00000000
> [ 0.871004] 3ec0: 00000000 ee443ec8 ee41a018 00000000 ee443f14 ee443ee0 c0103478 c02c3d34
> [ 0.879453] 3ee0: c00f6514 00000002 00000002 00000002 ee413000 ffffff9c ee413000 00000000
> [ 0.887903] 3f00: 00000002 ffffff9c 00000000 ee413000 ffffff9c 00000000 ee443f6c ee443f28
> [ 0.896353] 3f20: c00e8738 c00f6bc0 00000000 00000000 c03ade9c 00000002 00000000 00000026
> [ 0.904802] 3f40: 00000100 00000001 c03ade9c c03ade9c 00000000 00000000 00000000 00000000
> [ 0.913252] 3f60: ee443f7c ee443f70 c00e880c c00e8618 ee443f94 ee443f80 c038eef8 c00e87f4
> [ 0.921702] 3f80: 00000000 c02b6138 ee443fac ee443f98 c02b6148 c038ee50 ee442000 00000000
> [ 0.930151] 3fa0: 00000000 ee443fb0 c000fa08 c02b6144 00000000 00000000 00000000 00000000
> [ 0.938600] 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 0.947049] 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 964ec888 865a0c8b
> [ 0.955491] Backtrace:
> [ 0.958044] [<c022a898>] (dw_apb_ictl_handler) from [<c006171c>] (generic_handle_irq+0x28/
> 0x38)
> [ 0.967026] r10:60000113 r9:ee41d4bc r8:ee408000 r7:00000031 r6:00000000 r5:00000000
> [ 0.975191] r4:00000011
> [ 0.977832] [<c00616f4>] (generic_handle_irq) from [<c0061848>] (__handle_domain_irq+0x5c/
> 0xb4)
> [ 0.986814] r4:c04d6e4c r3:0000001b
> [ 0.990548] [<c00617ec>] (__handle_domain_irq) from [<c00093b4>] (gic_handle_irq+0x28/0x68
> )
> [ 0.999169] r8:ef802000 r7:ef80200c r6:00000021 r5:c04dcfb4 r4:ee443ba0 r3:ee443ba0
> [ 1.007252] [<c000938c>] (gic_handle_irq) from [<c0013580>] (__irq_svc+0x40/0x74)
> [ 1.014980] Exception stack(0xee443ba0 to 0xee443be8)
> [ 1.020205] 3ba0: ee41d4e4 60000113 00000001 00000005 ee41d480 ee5d2b40 ee41d4e4 c0247370
> [ 1.028655] 3bc0: 0000001a ee41d4bc 60000113 ee443bf4 ee443bf8 ee443be8 c0063910 c02c3da8
> [ 1.037100] 3be0: 60000113 ffffffff
> [ 1.040704] r8:0000001a r7:ee443bd4 r6:ffffffff r5:60000113 r4:c02c3da8 r3:c0063910
> [ 1.048783] [<c02c3d80>] (_raw_spin_unlock_irqrestore) from [<c0063910>] (__setup_irq+0x30
> 0/0x4f4)
> [ 1.058040] [<c0063610>] (__setup_irq) from [<c0063bd8>] (request_threaded_irq+0xd4/0x150)
> [ 1.066573] r10:ee5d2b00 r9:00000000 r8:0000001a r7:c0247370 r6:ee41d480 r5:00000080
> [ 1.074738] r4:ee5d2b40
> [ 1.077381] [<c0063b04>] (request_threaded_irq) from [<c024a280>] (serial_link_irq_chain+0
> x14c/0x188)
> [ 1.086900] r10:c05164d8 r9:ee5e8800 r8:ee5d2b0c r7:00000000 r6:00000080 r5:ee5d2b00
> [ 1.095066] r4:c0515e80 r3:00000080
> [ 1.098802] [<c024a134>] (serial_link_irq_chain) from [<c024a35c>] (univ8250_setup_irq+0xa
> 0/0xac)
> [ 1.107962] r10:00000000 r8:00000000 r7:20000113 r6:00000002 r5:00000002 r4:c0515e80
> [ 1.116135] [<c024a2bc>] (univ8250_setup_irq) from [<c0249348>] (serial8250_do_startup+0x1
> 24/0x658)
> [ 1.125475] r4:c0515e80 r3:c024a2bc
> [ 1.129208] [<c0249224>] (serial8250_do_startup) from [<c02498a0>] (serial8250_startup+0x2
> 4/0x28)
> [ 1.138369] r10:00000000 r8:00000000 r7:ee5e8800 r6:00000000 r5:ee4a9400 r4:c0515e80
> [ 1.146544] [<c024987c>] (serial8250_startup) from [<c02442e8>] (uart_port_startup+0x5c/0x
> 140)
> [ 1.155442] [<c024428c>] (uart_port_startup) from [<c0244f44>] (uart_open+0xdc/0x144)
> [ 1.163527] r8:ee5e895c r7:ee4b3d80 r6:ee4a949c r5:ee5e8800 r4:ee4a9400 r3:00000002
> [ 1.171610] [<c0244e68>] (uart_open) from [<c0239820>] (tty_open+0x38/0x344)
> [ 1.178888] r10:ee5e8800 r8:00000002 r7:00500001 r6:c02e6610 r5:ee4408c8 r4:ee4b3d80
> [ 1.187064] [<c02397e8>] (tty_open) from [<c00ecfdc>] (chrdev_open+0xb0/0x168)
> [ 1.194521] r10:ee001d48 r9:ee4b3d80 r8:c0515ce8 r7:c0515ce8 r6:c02e675c r5:ee4b3d80
> [ 1.202685] r4:ee4408c8
> [ 1.205330] [<c00ecf2c>] (chrdev_open) from [<c00e74cc>] (do_dentry_open.isra.13+0xf0/0x2f
> 8)
> [ 1.214042] r8:00000000 r7:c00ecf2c r6:ee4b3d88 r5:ee4408c8 r4:ee4b3d80
> [ 1.221049] [<c00e73dc>] (do_dentry_open.isra.13) from [<c00e83e8>] (vfs_open+0x64/0x68)
> [ 1.229403] r10:ee001d48 r8:00000000 r7:ee443e5c r6:00000002 r5:ee443f34 r4:ee4b3d80
> [ 1.237577] [<c00e8384>] (vfs_open) from [<c00f57a0>] (do_last+0x128/0x6e4)
> [ 1.244766] r4:ee443e88 r3:ee454000
> [ 1.248497] [<c00f5678>] (do_last) from [<c00f5dd0>] (path_openat+0x74/0x140)
> [ 1.255866] r10:00000000 r9:00000000 r8:00000000 r7:00000041 r6:ee443f34 r5:ee4b3d80
> [ 1.264028] r4:ee443e88
> [ 1.266667] [<c00f5d5c>] (path_openat) from [<c00f6c1c>] (do_filp_open+0x68/0xbc)
> [ 1.274393] r8:00000000 r7:00000000 r6:00000001 r5:ee443e88 r4:ee443f34
> [ 1.281398] [<c00f6bb4>] (do_filp_open) from [<c00e8738>] (do_sys_open+0x12c/0x1dc)
> [ 1.289303] r7:00000000 r6:ffffff9c r5:ee413000 r4:00000000
> [ 1.295219] [<c00e860c>] (do_sys_open) from [<c00e880c>] (SyS_open+0x24/0x28)
> [ 1.302588] r10:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c03ade9c r4:c03ade9c
> [ 1.310763] [<c00e87e8>] (SyS_open) from [<c038eef8>] (kernel_init_freeable+0xb4/0x140)
> [ 1.319037] [<c038ee44>] (kernel_init_freeable) from [<c02b6148>] (kernel_init+0x10/0xec)
> [ 1.327480] r5:c02b6138 r4:00000000
> [ 1.331216] [<c02b6138>] (kernel_init) from [<c000fa08>] (ret_from_fork+0x14/0x2c)
> [ 1.339031] r4:00000000 r3:ee442000
> [ 1.342762] Code: e5972004 e59f0064 eb023931 e5973004 (e593a030)
>
>
> > +
> > while (stat) {
> > u32 hwirq = ffs(stat) - 1;
> > - generic_handle_irq(irq_find_mapping(d,
> > - gc->irq_base + hwirq + 32 * n));
> > + u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
> > +
> > + generic_handle_irq(virq);
> > stat &= ~(1 << hwirq);
> > }
> > }
> > @@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc
> > struct irq_domain *domain;
> > struct irq_chip_generic *gc;
> > void __iomem *iobase;
> > - int ret, nrirqs, irq;
> > + int ret, nrirqs, irq, i;
> > u32 reg;
> >
> > /* Map the parent interrupt for the chained handler */
> > @@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc
> > goto err_unmap;
> > }
> >
> > - ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
> > - np->name, handle_level_irq, clr, 0,
> > - IRQ_GC_MASK_CACHE_PER_TYPE |
> > + ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
> > + handle_level_irq, clr, 0,
> > IRQ_GC_INIT_MASK_CACHE);
> > if (ret) {
> > pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
> > goto err_unmap;
> > }
> >
> > - gc = irq_get_domain_generic_chip(domain, 0);
> > - gc->private = domain;
> > - gc->reg_base = iobase;
> > -
> > - gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> > - gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> > - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> > - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> > - gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> > -
> > - if (nrirqs > 32) {
> > - gc->chip_types[1].regs.mask = APB_INT_MASK_H;
> > - gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
> > - gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
> > - gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
> > - gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
> > + for (i = 0; i < nrirqs / 32; i++) {
> > + gc = irq_get_domain_generic_chip(domain, i * 32);
> > + gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
> > + gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> > + gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> > + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> > + gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> > }
> >
> > - irq_set_handler_data(irq, gc);
> > - irq_set_chained_handler(irq, dw_apb_ictl_handler);
> > + irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
> >
> > return 0;
> >
>
>
>
>
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 12:31 ` Jisheng Zhang
@ 2015-07-06 12:36 ` Thomas Gleixner
2015-07-06 12:45 ` Jisheng Zhang
2015-07-06 12:55 ` Thomas Gleixner
0 siblings, 2 replies; 11+ messages in thread
From: Thomas Gleixner @ 2015-07-06 12:36 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > Thomas Gleixner <tglx@linutronix.de> wrote:
> the following patch seems fix the panic, but I dunno whether it's correct or not,
> could you please help to check?
>
> Thanks,
> Jisheng
>
> diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
> index f4a0e11..8d996cb 100644
> --- a/drivers/irqchip/irq-dw-apb-ictl.c
> +++ b/drivers/irqchip/irq-dw-apb-ictl.c
> @@ -30,13 +30,14 @@
> static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> {
> struct irq_domain *d = irq_desc_get_handler_data(desc);
> - struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> + struct irq_chip_generic *gc;
> struct irq_chip *chip = irq_desc_get_chip(desc);
> int n;
>
> chained_irq_enter(chip, desc);
>
> - for (n = 0; n < d->gc->num_chips; n++, gc++) {
> + for (n = 0; n < d->gc->num_chips; n++) {
> + gc = irq_get_domain_generic_chip(d, n * 32);
> u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
Yes it's correct. Seems I tried to be overly clever by avoiding the
lookup of the second chip. Will fold back.
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 12:36 ` Thomas Gleixner
@ 2015-07-06 12:45 ` Jisheng Zhang
2015-07-06 12:55 ` Thomas Gleixner
1 sibling, 0 replies; 11+ messages in thread
From: Jisheng Zhang @ 2015-07-06 12:45 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015 14:36:40 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > > Thomas Gleixner <tglx@linutronix.de> wrote:
> > the following patch seems fix the panic, but I dunno whether it's correct or not,
> > could you please help to check?
> >
> > Thanks,
> > Jisheng
> >
> > diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
> > index f4a0e11..8d996cb 100644
> > --- a/drivers/irqchip/irq-dw-apb-ictl.c
> > +++ b/drivers/irqchip/irq-dw-apb-ictl.c
> > @@ -30,13 +30,14 @@
> > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> > {
> > struct irq_domain *d = irq_desc_get_handler_data(desc);
> > - struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> > + struct irq_chip_generic *gc;
> > struct irq_chip *chip = irq_desc_get_chip(desc);
> > int n;
> >
> > chained_irq_enter(chip, desc);
> >
> > - for (n = 0; n < d->gc->num_chips; n++, gc++) {
> > + for (n = 0; n < d->gc->num_chips; n++) {
> > + gc = irq_get_domain_generic_chip(d, n * 32);
> > u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
>
> Yes it's correct. Seems I tried to be overly clever by avoiding the
> lookup of the second chip. Will fold back.
>
Great! Feel free to add my Tested-by.
I'll also cherry-pick your patch into marvell internal repo.
Thank you very much,
Jisheng
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 12:36 ` Thomas Gleixner
2015-07-06 12:45 ` Jisheng Zhang
@ 2015-07-06 12:55 ` Thomas Gleixner
2015-07-06 13:04 ` Jisheng Zhang
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2015-07-06 12:55 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015, Thomas Gleixner wrote:
> On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > > Thomas Gleixner <tglx@linutronix.de> wrote:
> > the following patch seems fix the panic, but I dunno whether it's correct or not,
> > could you please help to check?
> >
> > Thanks,
> > Jisheng
> >
> > diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
> > index f4a0e11..8d996cb 100644
> > --- a/drivers/irqchip/irq-dw-apb-ictl.c
> > +++ b/drivers/irqchip/irq-dw-apb-ictl.c
> > @@ -30,13 +30,14 @@
> > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> > {
> > struct irq_domain *d = irq_desc_get_handler_data(desc);
> > - struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> > + struct irq_chip_generic *gc;
> > struct irq_chip *chip = irq_desc_get_chip(desc);
> > int n;
> >
> > chained_irq_enter(chip, desc);
> >
> > - for (n = 0; n < d->gc->num_chips; n++, gc++) {
> > + for (n = 0; n < d->gc->num_chips; n++) {
> > + gc = irq_get_domain_generic_chip(d, n * 32);
> > u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
>
> Yes it's correct. Seems I tried to be overly clever by avoiding the
> lookup of the second chip. Will fold back.
Hmm. That does not make sense because the real issue is here:
- for (i = 0; i < nrirqs / 32; i++) {
+ for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 12:55 ` Thomas Gleixner
@ 2015-07-06 13:04 ` Jisheng Zhang
2015-07-06 13:24 ` Thomas Gleixner
0 siblings, 1 reply; 11+ messages in thread
From: Jisheng Zhang @ 2015-07-06 13:04 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015 14:55:43 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 6 Jul 2015, Thomas Gleixner wrote:
> > On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > > > Thomas Gleixner <tglx@linutronix.de> wrote:
> > > the following patch seems fix the panic, but I dunno whether it's correct or not,
> > > could you please help to check?
> > >
> > > Thanks,
> > > Jisheng
> > >
> > > diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
> > > index f4a0e11..8d996cb 100644
> > > --- a/drivers/irqchip/irq-dw-apb-ictl.c
> > > +++ b/drivers/irqchip/irq-dw-apb-ictl.c
> > > @@ -30,13 +30,14 @@
> > > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> > > {
> > > struct irq_domain *d = irq_desc_get_handler_data(desc);
> > > - struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> > > + struct irq_chip_generic *gc;
> > > struct irq_chip *chip = irq_desc_get_chip(desc);
> > > int n;
> > >
> > > chained_irq_enter(chip, desc);
> > >
> > > - for (n = 0; n < d->gc->num_chips; n++, gc++) {
> > > + for (n = 0; n < d->gc->num_chips; n++) {
> > > + gc = irq_get_domain_generic_chip(d, n * 32);
> > > u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
> >
> > Yes it's correct. Seems I tried to be overly clever by avoiding the
> > lookup of the second chip. Will fold back.
>
> Hmm. That does not make sense because the real issue is here:
>
> - for (i = 0; i < nrirqs / 32; i++) {
> + for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
>
OOPS, we need the above DIV_ROUND_UP fix. But...
On Berlin SoC, nrirqs = 64, so it doesn't make difference and we get the same
panic.
Thanks,
Jisheng
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 13:04 ` Jisheng Zhang
@ 2015-07-06 13:24 ` Thomas Gleixner
2015-07-06 13:32 ` Thomas Gleixner
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2015-07-06 13:24 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> On Mon, 6 Jul 2015 14:55:43 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> > On Mon, 6 Jul 2015, Thomas Gleixner wrote:
> > > On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > > > > Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > the following patch seems fix the panic, but I dunno whether it's correct or not,
> > > > could you please help to check?
> > > >
> > > > Thanks,
> > > > Jisheng
> > > >
> > > > diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
> > > > index f4a0e11..8d996cb 100644
> > > > --- a/drivers/irqchip/irq-dw-apb-ictl.c
> > > > +++ b/drivers/irqchip/irq-dw-apb-ictl.c
> > > > @@ -30,13 +30,14 @@
> > > > static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> > > > {
> > > > struct irq_domain *d = irq_desc_get_handler_data(desc);
> > > > - struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
> > > > + struct irq_chip_generic *gc;
> > > > struct irq_chip *chip = irq_desc_get_chip(desc);
> > > > int n;
> > > >
> > > > chained_irq_enter(chip, desc);
> > > >
> > > > - for (n = 0; n < d->gc->num_chips; n++, gc++) {
> > > > + for (n = 0; n < d->gc->num_chips; n++) {
> > > > + gc = irq_get_domain_generic_chip(d, n * 32);
> > > > u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
> > >
> > > Yes it's correct. Seems I tried to be overly clever by avoiding the
> > > lookup of the second chip. Will fold back.
> >
> > Hmm. That does not make sense because the real issue is here:
> >
> > - for (i = 0; i < nrirqs / 32; i++) {
> > + for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
> >
>
> OOPS, we need the above DIV_ROUND_UP fix. But...
>
> On Berlin SoC, nrirqs = 64, so it doesn't make difference and we get the same
> panic.
Yes, that's right. irq_domain_chip_generic->gc is an array of
pointers, not an array of generic chips. Stupid me...
Thanks,
tglx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 13:24 ` Thomas Gleixner
@ 2015-07-06 13:32 ` Thomas Gleixner
2015-07-06 13:43 ` Jisheng Zhang
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Gleixner @ 2015-07-06 13:32 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015, Thomas Gleixner wrote:
> On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > OOPS, we need the above DIV_ROUND_UP fix. But...
> >
> > On Berlin SoC, nrirqs = 64, so it doesn't make difference and we get the same
> > panic.
>
> Yes, that's right. irq_domain_chip_generic->gc is an array of
> pointers, not an array of generic chips. Stupid me...
So here is the final version. Can you verify that again, please?
Thanks,
tglx
---
Index: tip/drivers/irqchip/irq-dw-apb-ictl.c
===================================================================
--- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c
+++ tip/drivers/irqchip/irq-dw-apb-ictl.c
@@ -25,24 +25,25 @@
#define APB_INT_MASK_H 0x0c
#define APB_INT_FINALSTATUS_L 0x30
#define APB_INT_FINALSTATUS_H 0x34
+#define APB_INT_BASE_OFFSET 0x04
static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
{
- struct irq_chip *chip = irq_get_chip(irq);
- struct irq_chip_generic *gc = irq_get_handler_data(irq);
- struct irq_domain *d = gc->private;
- u32 stat;
+ struct irq_domain *d = irq_desc_get_handler_data(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
int n;
chained_irq_enter(chip, desc);
- for (n = 0; n < gc->num_ct; n++) {
- stat = readl_relaxed(gc->reg_base +
- APB_INT_FINALSTATUS_L + 4 * n);
+ for (n = 0; n < d->revmap_size; n += 32) {
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
+ u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
while (stat) {
u32 hwirq = ffs(stat) - 1;
- generic_handle_irq(irq_find_mapping(d,
- gc->irq_base + hwirq + 32 * n));
+ u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
+
+ generic_handle_irq(virq);
stat &= ~(1 << hwirq);
}
}
@@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc
struct irq_domain *domain;
struct irq_chip_generic *gc;
void __iomem *iobase;
- int ret, nrirqs, irq;
+ int ret, nrirqs, irq, i;
u32 reg;
/* Map the parent interrupt for the chained handler */
@@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc
goto err_unmap;
}
- ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
- np->name, handle_level_irq, clr, 0,
- IRQ_GC_MASK_CACHE_PER_TYPE |
+ ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
+ handle_level_irq, clr, 0,
IRQ_GC_INIT_MASK_CACHE);
if (ret) {
pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
goto err_unmap;
}
- gc = irq_get_domain_generic_chip(domain, 0);
- gc->private = domain;
- gc->reg_base = iobase;
-
- gc->chip_types[0].regs.mask = APB_INT_MASK_L;
- gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
- gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
-
- if (nrirqs > 32) {
- gc->chip_types[1].regs.mask = APB_INT_MASK_H;
- gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
- gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
+ for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
+ gc = irq_get_domain_generic_chip(domain, i * 32);
+ gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
+ gc->chip_types[0].regs.mask = APB_INT_MASK_L;
+ gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
+ gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+ gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+ gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_handler_data(irq, gc);
- irq_set_chained_handler(irq, dw_apb_ictl_handler);
+ irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
return 0;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 13:32 ` Thomas Gleixner
@ 2015-07-06 13:43 ` Jisheng Zhang
0 siblings, 0 replies; 11+ messages in thread
From: Jisheng Zhang @ 2015-07-06 13:43 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Sebastian Hesselbarth, Mark Rutland, Jason Cooper, LKML
On Mon, 6 Jul 2015 15:32:25 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 6 Jul 2015, Thomas Gleixner wrote:
> > On Mon, 6 Jul 2015, Jisheng Zhang wrote:
> > > OOPS, we need the above DIV_ROUND_UP fix. But...
> > >
> > > On Berlin SoC, nrirqs = 64, so it doesn't make difference and we get the same
> > > panic.
> >
> > Yes, that's right. irq_domain_chip_generic->gc is an array of
> > pointers, not an array of generic chips. Stupid me...
>
> So here is the final version. Can you verify that again, please?
Sure, it's my pleasure.
Per my test, the following version works perfectly! So
Tested-by: Jisheng Zhang <jszhang@marvell.com>
Thanks a lot,
Jisheng
>
> Thanks,
>
> tglx
> ---
> Index: tip/drivers/irqchip/irq-dw-apb-ictl.c
> ===================================================================
> --- tip.orig/drivers/irqchip/irq-dw-apb-ictl.c
> +++ tip/drivers/irqchip/irq-dw-apb-ictl.c
> @@ -25,24 +25,25 @@
> #define APB_INT_MASK_H 0x0c
> #define APB_INT_FINALSTATUS_L 0x30
> #define APB_INT_FINALSTATUS_H 0x34
> +#define APB_INT_BASE_OFFSET 0x04
>
> static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
> {
> - struct irq_chip *chip = irq_get_chip(irq);
> - struct irq_chip_generic *gc = irq_get_handler_data(irq);
> - struct irq_domain *d = gc->private;
> - u32 stat;
> + struct irq_domain *d = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> int n;
>
> chained_irq_enter(chip, desc);
>
> - for (n = 0; n < gc->num_ct; n++) {
> - stat = readl_relaxed(gc->reg_base +
> - APB_INT_FINALSTATUS_L + 4 * n);
> + for (n = 0; n < d->revmap_size; n += 32) {
> + struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
> + u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
> +
> while (stat) {
> u32 hwirq = ffs(stat) - 1;
> - generic_handle_irq(irq_find_mapping(d,
> - gc->irq_base + hwirq + 32 * n));
> + u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
> +
> + generic_handle_irq(virq);
> stat &= ~(1 << hwirq);
> }
> }
> @@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struc
> struct irq_domain *domain;
> struct irq_chip_generic *gc;
> void __iomem *iobase;
> - int ret, nrirqs, irq;
> + int ret, nrirqs, irq, i;
> u32 reg;
>
> /* Map the parent interrupt for the chained handler */
> @@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struc
> goto err_unmap;
> }
>
> - ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
> - np->name, handle_level_irq, clr, 0,
> - IRQ_GC_MASK_CACHE_PER_TYPE |
> + ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
> + handle_level_irq, clr, 0,
> IRQ_GC_INIT_MASK_CACHE);
> if (ret) {
> pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
> goto err_unmap;
> }
>
> - gc = irq_get_domain_generic_chip(domain, 0);
> - gc->private = domain;
> - gc->reg_base = iobase;
> -
> - gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> - gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> - gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> - gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> - gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> -
> - if (nrirqs > 32) {
> - gc->chip_types[1].regs.mask = APB_INT_MASK_H;
> - gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
> - gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
> - gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
> - gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
> + for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
> + gc = irq_get_domain_generic_chip(domain, i * 32);
> + gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
> + gc->chip_types[0].regs.mask = APB_INT_MASK_L;
> + gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
> + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
> + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
> + gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
> }
>
> - irq_set_handler_data(irq, gc);
> - irq_set_chained_handler(irq, dw_apb_ictl_handler);
> + irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
>
> return 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip:irq/core] irqchip/dw-apb-ictl: Fix generic domain chip wreckage
2015-07-06 10:18 [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage Thomas Gleixner
2015-07-06 12:07 ` Jisheng Zhang
@ 2015-07-11 21:30 ` tip-bot for Thomas Gleixner
1 sibling, 0 replies; 11+ messages in thread
From: tip-bot for Thomas Gleixner @ 2015-07-11 21:30 UTC (permalink / raw)
To: linux-tip-commits
Cc: mark.rutland, hpa, tglx, jszhang, jason, linux-kernel,
sebastian.hesselbarth, mingo
Commit-ID: b66231183a8542de1414e42326dd1c6bc4af75f4
Gitweb: http://git.kernel.org/tip/b66231183a8542de1414e42326dd1c6bc4af75f4
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 6 Jul 2015 15:32:25 +0200
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 11 Jul 2015 23:14:23 +0200
irqchip/dw-apb-ictl: Fix generic domain chip wreckage
The num_ct argument of irq_alloc_domain_generic_chips() tells the core
code how many chip types (for different control flows,
e.g. edge/level) should be allocated. It does not control how many
generic chip instances are created because that's determined from the
irq domain size and the number of interrupts per chip.
The dw-apb init abuses the num_ct argument for allocating one or two
chip types depending on the number of interrupts. That's completely
wrong because the alternate type is never used.
This code was obviously never tested on a system which has more than
32 interrupts as that would have never worked due to the unitialized
second generic chip instance.
Hand in the proper num_ct=1 and fixup the chip initialization along
with the interrupt handler.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Jisheng Zhang <jszhang@marvell.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/20150706101543.373582262@linutronix.de
---
drivers/irqchip/irq-dw-apb-ictl.c | 53 ++++++++++++++++-----------------------
1 file changed, 22 insertions(+), 31 deletions(-)
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index 53bb732..ca22f4e 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -25,24 +25,25 @@
#define APB_INT_MASK_H 0x0c
#define APB_INT_FINALSTATUS_L 0x30
#define APB_INT_FINALSTATUS_H 0x34
+#define APB_INT_BASE_OFFSET 0x04
static void dw_apb_ictl_handler(unsigned int irq, struct irq_desc *desc)
{
- struct irq_chip *chip = irq_get_chip(irq);
- struct irq_chip_generic *gc = irq_get_handler_data(irq);
- struct irq_domain *d = gc->private;
- u32 stat;
+ struct irq_domain *d = irq_desc_get_handler_data(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
int n;
chained_irq_enter(chip, desc);
- for (n = 0; n < gc->num_ct; n++) {
- stat = readl_relaxed(gc->reg_base +
- APB_INT_FINALSTATUS_L + 4 * n);
+ for (n = 0; n < d->revmap_size; n += 32) {
+ struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, n);
+ u32 stat = readl_relaxed(gc->reg_base + APB_INT_FINALSTATUS_L);
+
while (stat) {
u32 hwirq = ffs(stat) - 1;
- generic_handle_irq(irq_find_mapping(d,
- gc->irq_base + hwirq + 32 * n));
+ u32 virq = irq_find_mapping(d, gc->irq_base + hwirq);
+
+ generic_handle_irq(virq);
stat &= ~(1 << hwirq);
}
}
@@ -73,7 +74,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
struct irq_domain *domain;
struct irq_chip_generic *gc;
void __iomem *iobase;
- int ret, nrirqs, irq;
+ int ret, nrirqs, irq, i;
u32 reg;
/* Map the parent interrupt for the chained handler */
@@ -128,35 +129,25 @@ static int __init dw_apb_ictl_init(struct device_node *np,
goto err_unmap;
}
- ret = irq_alloc_domain_generic_chips(domain, 32, (nrirqs > 32) ? 2 : 1,
- np->name, handle_level_irq, clr, 0,
- IRQ_GC_MASK_CACHE_PER_TYPE |
+ ret = irq_alloc_domain_generic_chips(domain, 32, 1, np->name,
+ handle_level_irq, clr, 0,
IRQ_GC_INIT_MASK_CACHE);
if (ret) {
pr_err("%s: unable to alloc irq domain gc\n", np->full_name);
goto err_unmap;
}
- gc = irq_get_domain_generic_chip(domain, 0);
- gc->private = domain;
- gc->reg_base = iobase;
-
- gc->chip_types[0].regs.mask = APB_INT_MASK_L;
- gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
- gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
-
- if (nrirqs > 32) {
- gc->chip_types[1].regs.mask = APB_INT_MASK_H;
- gc->chip_types[1].regs.enable = APB_INT_ENABLE_H;
- gc->chip_types[1].chip.irq_mask = irq_gc_mask_set_bit;
- gc->chip_types[1].chip.irq_unmask = irq_gc_mask_clr_bit;
- gc->chip_types[1].chip.irq_resume = dw_apb_ictl_resume;
+ for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
+ gc = irq_get_domain_generic_chip(domain, i * 32);
+ gc->reg_base = iobase + i * APB_INT_BASE_OFFSET;
+ gc->chip_types[0].regs.mask = APB_INT_MASK_L;
+ gc->chip_types[0].regs.enable = APB_INT_ENABLE_L;
+ gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+ gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+ gc->chip_types[0].chip.irq_resume = dw_apb_ictl_resume;
}
- irq_set_handler_data(irq, gc);
- irq_set_chained_handler(irq, dw_apb_ictl_handler);
+ irq_set_chained_handler_and_data(irq, dw_apb_ictl_handler, domain);
return 0;
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-07-11 21:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-06 10:18 [patch] irqchip/dw-apb-ictl: Fix generic domain chip wreckage Thomas Gleixner
2015-07-06 12:07 ` Jisheng Zhang
2015-07-06 12:31 ` Jisheng Zhang
2015-07-06 12:36 ` Thomas Gleixner
2015-07-06 12:45 ` Jisheng Zhang
2015-07-06 12:55 ` Thomas Gleixner
2015-07-06 13:04 ` Jisheng Zhang
2015-07-06 13:24 ` Thomas Gleixner
2015-07-06 13:32 ` Thomas Gleixner
2015-07-06 13:43 ` Jisheng Zhang
2015-07-11 21:30 ` [tip:irq/core] " tip-bot for Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox