* Disabling an interrupt in the handler locks the system up @ 2016-10-21 16:37 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 16:37 UTC (permalink / raw) To: Thomas Gleixner, Marc Zyngier; +Cc: LKML, Linux ARM, Sebastian Frias Hello, On my platform, one HW block pulls the interrupt line high as long as it remains idle, and low when it is busy. The device tree node is: test@22222 { compatible = "vendor,testme"; interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; }; I wrote a minimal driver which registers the irq. And in the interrupt handler, I disable said irq. Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as it is registered (because the block is idle). Here is the code I've been running, request_irq doesn't return. #include <linux/module.h> #include <linux/of_irq.h> #include <linux/interrupt.h> #include <linux/platform_device.h> static irqreturn_t test_isr(int irq, void *dev) { printk("irq=%d dev=%p\n", irq, dev); disable_irq_nosync(irq); printk("IRQ %d NOW DISABLED\n", irq); return IRQ_HANDLED; } static int test_probe(struct platform_device *pdev) { unsigned res, virq; printk("ENTER %s\n", __func__); virq = irq_of_parse_and_map(pdev->dev.of_node, 0); printk("virq = %u\n", virq); res = request_irq(virq, test_isr, 0, "testme", (void *)0x42); printk("request_irq = %d\n", res); return 0; } static const struct of_device_id test_ids[] = { { .compatible = "vendor,testme" }, { /* sentinel */ } }; static struct platform_driver test_driver = { .probe = test_probe, .driver = { .name = "test", .of_match_table = test_ids, }, }; module_platform_driver(test_driver); MODULE_LICENSE("GPL"); And here's what I get when I try to load the module: (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) $ insmod test.ko [ 91.571065] ENTER test_probe [ 91.574208] virq = 21 [ 91.576902] irq=21 dev=00000042 [ 91.580061] IRQ 21 NOW DISABLED [ 102.038544] nfs: server 172.27.64.1 not responding, still trying [ 112.571855] INFO: rcu_preempt self-detected stall on CPU [ 112.577201] 0-...: (6298 ticks this GP) idle=c45/140000000000002/0 softirq=1295/1295 fqs=2045 [ 112.585942] (t=6300 jiffies g=208 c=207 q=20) [ 112.590497] Task dump for CPU 0: [ 112.593735] insmod R running 0 951 929 0x00000002 [ 112.600124] Backtrace: [ 112.602601] [<c010b9a4>] (dump_backtrace) from [<c010bba0>] (show_stack+0x18/0x1c) [ 112.610207] r7:c0702434[ 112.612571] r6:c07024c8 r5:000003a1[ 112.616163] r4:df570dc0 [ 112.618701] [ 112.620202] [<c010bb88>] (show_stack) from [<c0143a28>] (sched_show_task+0xbc/0x110) [ 112.627989] [<c014396c>] (sched_show_task) from [<c0145ef0>] (dump_cpu_task+0x40/0x48) [ 112.635943] r5:00000000[ 112.638307] r4:00000000 [ 112.640845] [ 112.642347] [<c0145eb0>] (dump_cpu_task) from [<c01a8130>] (rcu_dump_cpu_stacks+0xb0/0xcc) [ 112.650651] r5:00000000[ 112.653014] r4:c070a840 [ 112.655553] [ 112.657055] [<c01a8080>] (rcu_dump_cpu_stacks) from [<c01743ec>] (rcu_check_callbacks+0x8f8/0x9b0) [ 112.666058] r10:c07024c0[ 112.668509] r9:c0702518 r8:1f5a3000[ 112.672100] r7:c070a840 r6:c0702100[ 112.675691] r5:c0637f40 [ 112.678230] r4:dfbdaf40[ 112.680592] r3:ffff2458 [ 112.683130] [ 112.684629] [<c0173af4>] (rcu_check_callbacks) from [<c0177b70>] (update_process_times+0x3c/0x64) [ 112.693545] r10:c0188c54[ 112.695995] r9:00000001 r8:dfbd870c[ 112.699588] r7:0000001a r6:00000000[ 112.703178] r5:df570dc0 [ 112.705717] r4:ffffe000[ 112.708080] [ 112.709581] [<c0177b34>] (update_process_times) from [<c0188c50>] (tick_sched_handle+0x50/0x54) [ 112.718322] r7:0000001a[ 112.720684] r6:35192494 r5:def2dab8[ 112.724274] r4:dfbd88f8 [ 112.726812] [ 112.728309] [<c0188c00>] (tick_sched_handle) from [<c0188cb0>] (tick_sched_timer+0x5c/0xa0) [ 112.736705] [<c0188c54>] (tick_sched_timer) from [<c0178bf4>] (__hrtimer_run_queues+0x11c/0x1ac) [ 112.745533] r7:00000000[ 112.747896] r6:dfbd8700 r5:dfbd88f8[ 112.751486] r4:dfbd86c0 [ 112.754025] [ 112.755519] [<c0178ad8>] (__hrtimer_run_queues) from [<c0178e88>] (hrtimer_interrupt+0xbc/0x20c) [ 112.764347] r10:dfbd8738[ 112.766798] r9:dfbd8778 r8:dfbd8758[ 112.770389] r7:dfbd86d4 r6:ffffffff[ 112.773979] r5:00000003 [ 112.776518] r4:dfbd86c0[ 112.778881] [ 112.780378] [<c0178dcc>] (hrtimer_interrupt) from [<c010ed0c>] (twd_handler+0x38/0x48) [ 112.788332] r10:def2dab8[ 112.790782] r9:df402400 r8:00000001[ 112.794373] r7:df408700 r6:00000013[ 112.797964] r5:c0702744 [ 112.800504] r4:00000001[ 112.802866] [ 112.804362] [<c010ecd4>] (twd_handler) from [<c016a134>] (handle_percpu_devid_irq+0x94/0x14c) [ 112.812928] r5:c0702744[ 112.815291] r4:df405300 [ 112.817829] [ 112.819327] [<c016a0a0>] (handle_percpu_devid_irq) from [<c0165134>] (generic_handle_irq+0x2c/0x3c) [ 112.828417] r7:def2dbd0[ 112.830780] r6:00000013 r5:00000000[ 112.834370] r4:c0635ec4 [ 112.836909] [ 112.838404] [<c0165108>] (generic_handle_irq) from [<c01656dc>] (__handle_domain_irq+0x84/0xf4) [ 112.847150] [<c0165658>] (__handle_domain_irq) from [<c01014ac>] (gic_handle_irq+0x50/0x94) [ 112.855541] r10:def2dbd0[ 112.857992] r9:e0803100 r8:e0802100[ 112.861583] r7:def2dab8 r6:e080210c[ 112.865173] r5:c0702744 [ 112.867712] r4:c070ff50[ 112.870076] r3:def2dab8 [ 112.872614] [ 112.874107] [<c010145c>] (gic_handle_irq) from [<c010c70c>] (__irq_svc+0x6c/0xa8) [ 112.881626] Exception stack(0xdef2dab8 to 0xdef2db00) [ 112.886699] daa0: 00000000 c057c9b4 [ 112.894919] dac0: c07216c0 00000000 00000202 ffffe000 00000013 00000000 00000001 df402400 [ 112.903140] dae0: def2dbd0 def2db64 def2daf8 def2db08 c030db68 c0121068 20000113 ffffffff [ 112.911356] r9:def2c000[ 112.913719] r8:00000001 r7:def2daec[ 112.917310] r6:ffffffff r5:20000113[ 112.920901] r4:c0121068 [ 112.923448] [<c0120fc0>] (__do_softirq) from [<c0121520>] (irq_exit+0xd4/0x110) [ 112.930792] r10:def2dbd0[ 112.933242] r9:df402400 r8:00000001[ 112.936833] r7:00000000 r6:00000013[ 112.940423] r5:00000000 [ 112.942962] r4:c0635ec4[ 112.945324] [ 112.946819] [<c012144c>] (irq_exit) from [<c01656e0>] (__handle_domain_irq+0x88/0xf4) [ 112.954690] [<c0165658>] (__handle_domain_irq) from [<c01014ac>] (gic_handle_irq+0x50/0x94) [ 112.963080] r10:00000000[ 112.965531] r9:e0803100 r8:e0802100[ 112.969122] r7:def2dbd0 r6:e080210c[ 112.972714] r5:c0702744 [ 112.975253] r4:c070ff50[ 112.977617] r3:def2dbd0 [ 112.980154] [ 112.981648] [<c010145c>] (gic_handle_irq) from [<c010c70c>] (__irq_svc+0x6c/0xa8) [ 112.989166] Exception stack(0xdef2dbd0 to 0xdef2dc18) [ 112.994240] dbc0: df506260 60000013 00000000 00000005 [ 113.002460] dbe0: dedbd680 df506200 00000015 df506260 60000013 df506238 00000000 def2dc2c [ 113.010679] dc00: def2dc30 def2dc20 c0167f90 c04d68d0 60000013 ffffffff [ 113.017324] r9:def2c000[ 113.019687] r8:60000013 r7:def2dc04[ 113.023277] r6:ffffffff r5:60000013[ 113.026868] r4:c04d68d0 [ 113.029418] [<c04d68a8>] (_raw_spin_unlock_irqrestore) from [<c0167f90>] (__setup_irq+0x440/0x5f0) [ 113.038426] [<c0167b50>] (__setup_irq) from [<c0168300>] (request_threaded_irq+0xf0/0x188) [ 113.046730] r10:00000000[ 113.049179] r9:00000015 r8:df506210[ 113.052770] r7:00000000 r6:df506200[ 113.056362] r5:00000000 [ 113.058901] r4:dedbd680[ 113.061263] [ 113.062767] [<c0168210>] (request_threaded_irq) from [<bf0040b8>] (test_probe+0x74/0x90 [zozo]) [ 113.071508] r10:00000000[ 113.073959] r9:21242a5c r8:00000001[ 113.077550] r7:fffffdfb r6:bf004320[ 113.081142] r5:df516810 [ 113.083681] r4:00000015[ 113.086044] r3:00000000 [ 113.088582] [ 113.090085] [<bf004044>] (test_probe [zozo]) from [<c03595e8>] (platform_drv_probe+0x54/0xb8) [ 113.098652] r4:c0752d30[ 113.101014] [ 113.102510] [<c0359594>] (platform_drv_probe) from [<c0357b88>] (driver_probe_device+0x228/0x2b8) [ 113.111425] r7:bf004320[ 113.113789] r6:df516844 r5:df516810[ 113.117379] r4:c0752d30 [ 113.119917] [ 113.121412] [<c0357960>] (driver_probe_device) from [<c0357cd8>] (__driver_attach+0xc0/0xc4) [ 113.129891] r9:21242a5c[ 113.132255] r8:00000001 r7:00000000[ 113.135845] r6:df516844 r5:bf004320[ 113.139436] r4:df516810 [ 113.141980] [<c0357c18>] (__driver_attach) from [<c0355d00>] (bus_for_each_dev+0x70/0xa4) [ 113.150196] r7:00000000[ 113.152560] r6:c0357c18 r5:bf004320[ 113.156150] r4:00000000 [ 113.158689] [ 113.160183] [<c0355c90>] (bus_for_each_dev) from [<c03573d4>] (driver_attach+0x24/0x28) [ 113.168224] r6:c0714090[ 113.170587] r5:df7efc80 r4:bf004320[ 113.174178] [ 113.175673] [<c03573b0>] (driver_attach) from [<c0356fa0>] (bus_add_driver+0x1a8/0x220) [ 113.183719] [<c0356df8>] (bus_add_driver) from [<c0358498>] (driver_register+0x80/0x100) [ 113.191848] r7:bf004380[ 113.194211] r6:dedbdc80 r5:bf006000[ 113.197801] r4:bf004320 [ 113.200339] [ 113.201834] [<c0358418>] (driver_register) from [<c0359548>] (__platform_driver_register+0x48/0x50) [ 113.210924] r5:bf006000[ 113.213287] r4:c0714090 [ 113.215826] [ 113.217323] [<c0359500>] (__platform_driver_register) from [<bf006020>] (test_driver_init+0x20/0x24 [zozo]) [ 113.227113] r5:bf006000[ 113.229475] r4:ffffe000 [ 113.232014] [ 113.233511] [<bf006000>] (test_driver_init [zozo]) from [<c01017f4>] (do_one_initcall+0x4c/0x17c) [ 113.242432] [<c01017a8>] (do_one_initcall) from [<c01a81b4>] (do_init_module+0x68/0x3a4) [ 113.250561] r10:dedbd5c8[ 113.253011] r9:21242a5c r8:00000001[ 113.256602] r7:bf004380 r6:dedbdc80[ 113.260193] r5:00000001 [ 113.262732] r4:bf004380[ 113.265094] [ 113.266591] [<c01a814c>] (do_init_module) from [<c01925f0>] (load_module+0x1dc0/0x2180) [ 113.274633] r6:dedbd5c0[ 113.276997] r5:00000001 r4:def2df34[ 113.280587] [ 113.282082] [<c0190830>] (load_module) from [<c0192b08>] (SyS_init_module+0x158/0x170) [ 113.290037] r10:00000051[ 113.292487] r9:def2c000 r8:01bbf008[ 113.296078] r7:e08682cc r6:00000000[ 113.299667] r5:01bc02e4 [ 113.302207] r4:000012cc[ 113.304569] [ 113.306065] [<c01929b0>] (SyS_init_module) from [<c0107ba0>] (ret_fast_syscall+0x0/0x3c) [ 113.314194] r10:00000000[ 113.316645] r9:def2c000 r8:c0107d64[ 113.320236] r7:00000080 r6:be98bb54[ 113.323828] r5:be98bc6d [ 113.326368] r4:000012cc[ 113.328729] Are we not supposed to disable the irq in the handler? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 16:37 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 16:37 UTC (permalink / raw) To: linux-arm-kernel Hello, On my platform, one HW block pulls the interrupt line high as long as it remains idle, and low when it is busy. The device tree node is: test at 22222 { compatible = "vendor,testme"; interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; }; I wrote a minimal driver which registers the irq. And in the interrupt handler, I disable said irq. Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as it is registered (because the block is idle). Here is the code I've been running, request_irq doesn't return. #include <linux/module.h> #include <linux/of_irq.h> #include <linux/interrupt.h> #include <linux/platform_device.h> static irqreturn_t test_isr(int irq, void *dev) { printk("irq=%d dev=%p\n", irq, dev); disable_irq_nosync(irq); printk("IRQ %d NOW DISABLED\n", irq); return IRQ_HANDLED; } static int test_probe(struct platform_device *pdev) { unsigned res, virq; printk("ENTER %s\n", __func__); virq = irq_of_parse_and_map(pdev->dev.of_node, 0); printk("virq = %u\n", virq); res = request_irq(virq, test_isr, 0, "testme", (void *)0x42); printk("request_irq = %d\n", res); return 0; } static const struct of_device_id test_ids[] = { { .compatible = "vendor,testme" }, { /* sentinel */ } }; static struct platform_driver test_driver = { .probe = test_probe, .driver = { .name = "test", .of_match_table = test_ids, }, }; module_platform_driver(test_driver); MODULE_LICENSE("GPL"); And here's what I get when I try to load the module: (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) $ insmod test.ko [ 91.571065] ENTER test_probe [ 91.574208] virq = 21 [ 91.576902] irq=21 dev=00000042 [ 91.580061] IRQ 21 NOW DISABLED [ 102.038544] nfs: server 172.27.64.1 not responding, still trying [ 112.571855] INFO: rcu_preempt self-detected stall on CPU [ 112.577201] 0-...: (6298 ticks this GP) idle=c45/140000000000002/0 softirq=1295/1295 fqs=2045 [ 112.585942] (t=6300 jiffies g=208 c=207 q=20) [ 112.590497] Task dump for CPU 0: [ 112.593735] insmod R running 0 951 929 0x00000002 [ 112.600124] Backtrace: [ 112.602601] [<c010b9a4>] (dump_backtrace) from [<c010bba0>] (show_stack+0x18/0x1c) [ 112.610207] r7:c0702434[ 112.612571] r6:c07024c8 r5:000003a1[ 112.616163] r4:df570dc0 [ 112.618701] [ 112.620202] [<c010bb88>] (show_stack) from [<c0143a28>] (sched_show_task+0xbc/0x110) [ 112.627989] [<c014396c>] (sched_show_task) from [<c0145ef0>] (dump_cpu_task+0x40/0x48) [ 112.635943] r5:00000000[ 112.638307] r4:00000000 [ 112.640845] [ 112.642347] [<c0145eb0>] (dump_cpu_task) from [<c01a8130>] (rcu_dump_cpu_stacks+0xb0/0xcc) [ 112.650651] r5:00000000[ 112.653014] r4:c070a840 [ 112.655553] [ 112.657055] [<c01a8080>] (rcu_dump_cpu_stacks) from [<c01743ec>] (rcu_check_callbacks+0x8f8/0x9b0) [ 112.666058] r10:c07024c0[ 112.668509] r9:c0702518 r8:1f5a3000[ 112.672100] r7:c070a840 r6:c0702100[ 112.675691] r5:c0637f40 [ 112.678230] r4:dfbdaf40[ 112.680592] r3:ffff2458 [ 112.683130] [ 112.684629] [<c0173af4>] (rcu_check_callbacks) from [<c0177b70>] (update_process_times+0x3c/0x64) [ 112.693545] r10:c0188c54[ 112.695995] r9:00000001 r8:dfbd870c[ 112.699588] r7:0000001a r6:00000000[ 112.703178] r5:df570dc0 [ 112.705717] r4:ffffe000[ 112.708080] [ 112.709581] [<c0177b34>] (update_process_times) from [<c0188c50>] (tick_sched_handle+0x50/0x54) [ 112.718322] r7:0000001a[ 112.720684] r6:35192494 r5:def2dab8[ 112.724274] r4:dfbd88f8 [ 112.726812] [ 112.728309] [<c0188c00>] (tick_sched_handle) from [<c0188cb0>] (tick_sched_timer+0x5c/0xa0) [ 112.736705] [<c0188c54>] (tick_sched_timer) from [<c0178bf4>] (__hrtimer_run_queues+0x11c/0x1ac) [ 112.745533] r7:00000000[ 112.747896] r6:dfbd8700 r5:dfbd88f8[ 112.751486] r4:dfbd86c0 [ 112.754025] [ 112.755519] [<c0178ad8>] (__hrtimer_run_queues) from [<c0178e88>] (hrtimer_interrupt+0xbc/0x20c) [ 112.764347] r10:dfbd8738[ 112.766798] r9:dfbd8778 r8:dfbd8758[ 112.770389] r7:dfbd86d4 r6:ffffffff[ 112.773979] r5:00000003 [ 112.776518] r4:dfbd86c0[ 112.778881] [ 112.780378] [<c0178dcc>] (hrtimer_interrupt) from [<c010ed0c>] (twd_handler+0x38/0x48) [ 112.788332] r10:def2dab8[ 112.790782] r9:df402400 r8:00000001[ 112.794373] r7:df408700 r6:00000013[ 112.797964] r5:c0702744 [ 112.800504] r4:00000001[ 112.802866] [ 112.804362] [<c010ecd4>] (twd_handler) from [<c016a134>] (handle_percpu_devid_irq+0x94/0x14c) [ 112.812928] r5:c0702744[ 112.815291] r4:df405300 [ 112.817829] [ 112.819327] [<c016a0a0>] (handle_percpu_devid_irq) from [<c0165134>] (generic_handle_irq+0x2c/0x3c) [ 112.828417] r7:def2dbd0[ 112.830780] r6:00000013 r5:00000000[ 112.834370] r4:c0635ec4 [ 112.836909] [ 112.838404] [<c0165108>] (generic_handle_irq) from [<c01656dc>] (__handle_domain_irq+0x84/0xf4) [ 112.847150] [<c0165658>] (__handle_domain_irq) from [<c01014ac>] (gic_handle_irq+0x50/0x94) [ 112.855541] r10:def2dbd0[ 112.857992] r9:e0803100 r8:e0802100[ 112.861583] r7:def2dab8 r6:e080210c[ 112.865173] r5:c0702744 [ 112.867712] r4:c070ff50[ 112.870076] r3:def2dab8 [ 112.872614] [ 112.874107] [<c010145c>] (gic_handle_irq) from [<c010c70c>] (__irq_svc+0x6c/0xa8) [ 112.881626] Exception stack(0xdef2dab8 to 0xdef2db00) [ 112.886699] daa0: 00000000 c057c9b4 [ 112.894919] dac0: c07216c0 00000000 00000202 ffffe000 00000013 00000000 00000001 df402400 [ 112.903140] dae0: def2dbd0 def2db64 def2daf8 def2db08 c030db68 c0121068 20000113 ffffffff [ 112.911356] r9:def2c000[ 112.913719] r8:00000001 r7:def2daec[ 112.917310] r6:ffffffff r5:20000113[ 112.920901] r4:c0121068 [ 112.923448] [<c0120fc0>] (__do_softirq) from [<c0121520>] (irq_exit+0xd4/0x110) [ 112.930792] r10:def2dbd0[ 112.933242] r9:df402400 r8:00000001[ 112.936833] r7:00000000 r6:00000013[ 112.940423] r5:00000000 [ 112.942962] r4:c0635ec4[ 112.945324] [ 112.946819] [<c012144c>] (irq_exit) from [<c01656e0>] (__handle_domain_irq+0x88/0xf4) [ 112.954690] [<c0165658>] (__handle_domain_irq) from [<c01014ac>] (gic_handle_irq+0x50/0x94) [ 112.963080] r10:00000000[ 112.965531] r9:e0803100 r8:e0802100[ 112.969122] r7:def2dbd0 r6:e080210c[ 112.972714] r5:c0702744 [ 112.975253] r4:c070ff50[ 112.977617] r3:def2dbd0 [ 112.980154] [ 112.981648] [<c010145c>] (gic_handle_irq) from [<c010c70c>] (__irq_svc+0x6c/0xa8) [ 112.989166] Exception stack(0xdef2dbd0 to 0xdef2dc18) [ 112.994240] dbc0: df506260 60000013 00000000 00000005 [ 113.002460] dbe0: dedbd680 df506200 00000015 df506260 60000013 df506238 00000000 def2dc2c [ 113.010679] dc00: def2dc30 def2dc20 c0167f90 c04d68d0 60000013 ffffffff [ 113.017324] r9:def2c000[ 113.019687] r8:60000013 r7:def2dc04[ 113.023277] r6:ffffffff r5:60000013[ 113.026868] r4:c04d68d0 [ 113.029418] [<c04d68a8>] (_raw_spin_unlock_irqrestore) from [<c0167f90>] (__setup_irq+0x440/0x5f0) [ 113.038426] [<c0167b50>] (__setup_irq) from [<c0168300>] (request_threaded_irq+0xf0/0x188) [ 113.046730] r10:00000000[ 113.049179] r9:00000015 r8:df506210[ 113.052770] r7:00000000 r6:df506200[ 113.056362] r5:00000000 [ 113.058901] r4:dedbd680[ 113.061263] [ 113.062767] [<c0168210>] (request_threaded_irq) from [<bf0040b8>] (test_probe+0x74/0x90 [zozo]) [ 113.071508] r10:00000000[ 113.073959] r9:21242a5c r8:00000001[ 113.077550] r7:fffffdfb r6:bf004320[ 113.081142] r5:df516810 [ 113.083681] r4:00000015[ 113.086044] r3:00000000 [ 113.088582] [ 113.090085] [<bf004044>] (test_probe [zozo]) from [<c03595e8>] (platform_drv_probe+0x54/0xb8) [ 113.098652] r4:c0752d30[ 113.101014] [ 113.102510] [<c0359594>] (platform_drv_probe) from [<c0357b88>] (driver_probe_device+0x228/0x2b8) [ 113.111425] r7:bf004320[ 113.113789] r6:df516844 r5:df516810[ 113.117379] r4:c0752d30 [ 113.119917] [ 113.121412] [<c0357960>] (driver_probe_device) from [<c0357cd8>] (__driver_attach+0xc0/0xc4) [ 113.129891] r9:21242a5c[ 113.132255] r8:00000001 r7:00000000[ 113.135845] r6:df516844 r5:bf004320[ 113.139436] r4:df516810 [ 113.141980] [<c0357c18>] (__driver_attach) from [<c0355d00>] (bus_for_each_dev+0x70/0xa4) [ 113.150196] r7:00000000[ 113.152560] r6:c0357c18 r5:bf004320[ 113.156150] r4:00000000 [ 113.158689] [ 113.160183] [<c0355c90>] (bus_for_each_dev) from [<c03573d4>] (driver_attach+0x24/0x28) [ 113.168224] r6:c0714090[ 113.170587] r5:df7efc80 r4:bf004320[ 113.174178] [ 113.175673] [<c03573b0>] (driver_attach) from [<c0356fa0>] (bus_add_driver+0x1a8/0x220) [ 113.183719] [<c0356df8>] (bus_add_driver) from [<c0358498>] (driver_register+0x80/0x100) [ 113.191848] r7:bf004380[ 113.194211] r6:dedbdc80 r5:bf006000[ 113.197801] r4:bf004320 [ 113.200339] [ 113.201834] [<c0358418>] (driver_register) from [<c0359548>] (__platform_driver_register+0x48/0x50) [ 113.210924] r5:bf006000[ 113.213287] r4:c0714090 [ 113.215826] [ 113.217323] [<c0359500>] (__platform_driver_register) from [<bf006020>] (test_driver_init+0x20/0x24 [zozo]) [ 113.227113] r5:bf006000[ 113.229475] r4:ffffe000 [ 113.232014] [ 113.233511] [<bf006000>] (test_driver_init [zozo]) from [<c01017f4>] (do_one_initcall+0x4c/0x17c) [ 113.242432] [<c01017a8>] (do_one_initcall) from [<c01a81b4>] (do_init_module+0x68/0x3a4) [ 113.250561] r10:dedbd5c8[ 113.253011] r9:21242a5c r8:00000001[ 113.256602] r7:bf004380 r6:dedbdc80[ 113.260193] r5:00000001 [ 113.262732] r4:bf004380[ 113.265094] [ 113.266591] [<c01a814c>] (do_init_module) from [<c01925f0>] (load_module+0x1dc0/0x2180) [ 113.274633] r6:dedbd5c0[ 113.276997] r5:00000001 r4:def2df34[ 113.280587] [ 113.282082] [<c0190830>] (load_module) from [<c0192b08>] (SyS_init_module+0x158/0x170) [ 113.290037] r10:00000051[ 113.292487] r9:def2c000 r8:01bbf008[ 113.296078] r7:e08682cc r6:00000000[ 113.299667] r5:01bc02e4 [ 113.302207] r4:000012cc[ 113.304569] [ 113.306065] [<c01929b0>] (SyS_init_module) from [<c0107ba0>] (ret_fast_syscall+0x0/0x3c) [ 113.314194] r10:00000000[ 113.316645] r9:def2c000 r8:c0107d64[ 113.320236] r7:00000080 r6:be98bb54[ 113.323828] r5:be98bc6d [ 113.326368] r4:000012cc[ 113.328729] Are we not supposed to disable the irq in the handler? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 16:37 ` Mason @ 2016-10-21 17:46 ` Marc Zyngier -1 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-21 17:46 UTC (permalink / raw) To: Mason, Thomas Gleixner; +Cc: LKML, Linux ARM, Sebastian Frias On 21/10/16 17:37, Mason wrote: > Hello, > > On my platform, one HW block pulls the interrupt line high > as long as it remains idle, and low when it is busy. > > The device tree node is: > > test@22222 { > compatible = "vendor,testme"; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; > }; I assume that this is for the sake of the discussion, and that you do not actually intend to put together such a monstrosity. > > I wrote a minimal driver which registers the irq. > And in the interrupt handler, I disable said irq. > > Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as > it is registered (because the block is idle). > > Here is the code I've been running, request_irq doesn't return. [...] > And here's what I get when I try to load the module: > (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) [...] > Are we not supposed to disable the irq in the handler? You can. It then depends on what your interrupt controller does to actually ensure that the interrupt is disabled. Only you can trace it on your HW to find out. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 17:46 ` Marc Zyngier 0 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-21 17:46 UTC (permalink / raw) To: linux-arm-kernel On 21/10/16 17:37, Mason wrote: > Hello, > > On my platform, one HW block pulls the interrupt line high > as long as it remains idle, and low when it is busy. > > The device tree node is: > > test at 22222 { > compatible = "vendor,testme"; > interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; > }; I assume that this is for the sake of the discussion, and that you do not actually intend to put together such a monstrosity. > > I wrote a minimal driver which registers the irq. > And in the interrupt handler, I disable said irq. > > Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as > it is registered (because the block is idle). > > Here is the code I've been running, request_irq doesn't return. [...] > And here's what I get when I try to load the module: > (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) [...] > Are we not supposed to disable the irq in the handler? You can. It then depends on what your interrupt controller does to actually ensure that the interrupt is disabled. Only you can trace it on your HW to find out. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 17:46 ` Marc Zyngier @ 2016-10-21 18:39 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 18:39 UTC (permalink / raw) To: Marc Zyngier, Thomas Gleixner, Jason Cooper Cc: LKML, Linux ARM, Sebastian Frias On 21/10/2016 19:46, Marc Zyngier wrote: > On 21/10/16 17:37, Mason wrote: > >> On my platform, one HW block pulls the interrupt line high >> as long as it remains idle, and low when it is busy. >> >> The device tree node is: >> >> test@22222 { >> compatible = "vendor,testme"; >> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >> }; > > I assume that this is for the sake of the discussion, and that you do > not actually intend to put together such a monstrosity. It's just missing a reg properties to be a valid node, right? >> I wrote a minimal driver which registers the irq. >> And in the interrupt handler, I disable said irq. >> >> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >> it is registered (because the block is idle). >> >> Here is the code I've been running, request_irq doesn't return. > > [...] > >> And here's what I get when I try to load the module: >> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) > > [...] > >> Are we not supposed to disable the irq in the handler? > > You can. It then depends on what your interrupt controller does to > actually ensure that the interrupt is disabled. Only you can trace it on > your HW to find out. I'm using an upstream driver on v4.9-rc1 http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c Given that the system locks up, is it possible there is a bug in the driver? Which call-back handles enabling/disabling interrupts? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 18:39 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 18:39 UTC (permalink / raw) To: linux-arm-kernel On 21/10/2016 19:46, Marc Zyngier wrote: > On 21/10/16 17:37, Mason wrote: > >> On my platform, one HW block pulls the interrupt line high >> as long as it remains idle, and low when it is busy. >> >> The device tree node is: >> >> test at 22222 { >> compatible = "vendor,testme"; >> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >> }; > > I assume that this is for the sake of the discussion, and that you do > not actually intend to put together such a monstrosity. It's just missing a reg properties to be a valid node, right? >> I wrote a minimal driver which registers the irq. >> And in the interrupt handler, I disable said irq. >> >> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >> it is registered (because the block is idle). >> >> Here is the code I've been running, request_irq doesn't return. > > [...] > >> And here's what I get when I try to load the module: >> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) > > [...] > >> Are we not supposed to disable the irq in the handler? > > You can. It then depends on what your interrupt controller does to > actually ensure that the interrupt is disabled. Only you can trace it on > your HW to find out. I'm using an upstream driver on v4.9-rc1 http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c Given that the system locks up, is it possible there is a bug in the driver? Which call-back handles enabling/disabling interrupts? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 18:39 ` Mason @ 2016-10-21 19:14 ` Marc Zyngier -1 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-21 19:14 UTC (permalink / raw) To: Mason; +Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On Fri, 21 Oct 2016 20:39:41 +0200 Mason <slash.tmp@free.fr> wrote: > On 21/10/2016 19:46, Marc Zyngier wrote: > > > On 21/10/16 17:37, Mason wrote: > > > >> On my platform, one HW block pulls the interrupt line high > >> as long as it remains idle, and low when it is busy. > >> > >> The device tree node is: > >> > >> test@22222 { > >> compatible = "vendor,testme"; > >> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; > >> }; > > > > I assume that this is for the sake of the discussion, and that you do > > not actually intend to put together such a monstrosity. > > It's just missing a reg properties to be a valid node, right? If connecting a device that signals its interrupt as level low to an input line configured as level high doesn't strike you as a major issue, nothing will. At that point, you can put anything you want in your DT. > > >> I wrote a minimal driver which registers the irq. > >> And in the interrupt handler, I disable said irq. > >> > >> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as > >> it is registered (because the block is idle). > >> > >> Here is the code I've been running, request_irq doesn't return. > > > > [...] > > > >> And here's what I get when I try to load the module: > >> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) > > > > [...] > > > >> Are we not supposed to disable the irq in the handler? > > > > You can. It then depends on what your interrupt controller does to > > actually ensure that the interrupt is disabled. Only you can trace it on > > your HW to find out. > > I'm using an upstream driver on v4.9-rc1 > > http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c > > Given that the system locks up, is it possible there is a bug > in the driver? That's possible. > Which call-back handles enabling/disabling interrupts? How about irq_unmask/irq_mask? Thanks, M. -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 19:14 ` Marc Zyngier 0 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-21 19:14 UTC (permalink / raw) To: linux-arm-kernel On Fri, 21 Oct 2016 20:39:41 +0200 Mason <slash.tmp@free.fr> wrote: > On 21/10/2016 19:46, Marc Zyngier wrote: > > > On 21/10/16 17:37, Mason wrote: > > > >> On my platform, one HW block pulls the interrupt line high > >> as long as it remains idle, and low when it is busy. > >> > >> The device tree node is: > >> > >> test at 22222 { > >> compatible = "vendor,testme"; > >> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; > >> }; > > > > I assume that this is for the sake of the discussion, and that you do > > not actually intend to put together such a monstrosity. > > It's just missing a reg properties to be a valid node, right? If connecting a device that signals its interrupt as level low to an input line configured as level high doesn't strike you as a major issue, nothing will. At that point, you can put anything you want in your DT. > > >> I wrote a minimal driver which registers the irq. > >> And in the interrupt handler, I disable said irq. > >> > >> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as > >> it is registered (because the block is idle). > >> > >> Here is the code I've been running, request_irq doesn't return. > > > > [...] > > > >> And here's what I get when I try to load the module: > >> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) > > > > [...] > > > >> Are we not supposed to disable the irq in the handler? > > > > You can. It then depends on what your interrupt controller does to > > actually ensure that the interrupt is disabled. Only you can trace it on > > your HW to find out. > > I'm using an upstream driver on v4.9-rc1 > > http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c > > Given that the system locks up, is it possible there is a bug > in the driver? That's possible. > Which call-back handles enabling/disabling interrupts? How about irq_unmask/irq_mask? Thanks, M. -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 19:14 ` Marc Zyngier @ 2016-10-21 19:47 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 19:47 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On 21/10/2016 21:14, Marc Zyngier wrote: > Mason wrote: > >> On 21/10/2016 19:46, Marc Zyngier wrote: >> >>> On 21/10/16 17:37, Mason wrote: >>> >>>> On my platform, one HW block pulls the interrupt line high >>>> as long as it remains idle, and low when it is busy. >>>> >>>> The device tree node is: >>>> >>>> test@22222 { >>>> compatible = "vendor,testme"; >>>> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >>>> }; >>> >>> I assume that this is for the sake of the discussion, and that you do >>> not actually intend to put together such a monstrosity. >> >> It's just missing a reg properties to be a valid node, right? > > If connecting a device that signals its interrupt as level low to an > input line configured as level high doesn't strike you as a major > issue, nothing will. At that point, you can put anything you want in > your DT. If I understand correctly, you are saying that I should have specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? If the HW outputs 1 when idle, and 0 when busy, that is level low? (Sorry if this is obvious, I'm absolutely clueless in this subject matter.) >>>> I wrote a minimal driver which registers the irq. >>>> And in the interrupt handler, I disable said irq. >>>> >>>> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >>>> it is registered (because the block is idle). >>>> >>>> Here is the code I've been running, request_irq doesn't return. >>> >>> [...] >>> >>>> And here's what I get when I try to load the module: >>>> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) >>> >>> [...] >>> >>>> Are we not supposed to disable the irq in the handler? >>> >>> You can. It then depends on what your interrupt controller does to >>> actually ensure that the interrupt is disabled. Only you can trace it on >>> your HW to find out. >> >> I'm using an upstream driver on v4.9-rc1 >> >> http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c >> >> Given that the system locks up, is it possible there is a bug >> in the driver? > > That's possible. > >> Which call-back handles enabling/disabling interrupts? > > How about irq_unmask/irq_mask? I tried following the source from disable_irq_nosync() as far down as I could. disable_irq_nosync -> __disable_irq_nosync -> __disable_irq -> irq_disable -> ?? http://lxr.free-electrons.com/source/kernel/irq/chip.c#L232 I don't know if desc->irq_data.chip->irq_disable is defined by the driver I'm using? I don't know how the trail goes to irq_mask? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 19:47 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 19:47 UTC (permalink / raw) To: linux-arm-kernel On 21/10/2016 21:14, Marc Zyngier wrote: > Mason wrote: > >> On 21/10/2016 19:46, Marc Zyngier wrote: >> >>> On 21/10/16 17:37, Mason wrote: >>> >>>> On my platform, one HW block pulls the interrupt line high >>>> as long as it remains idle, and low when it is busy. >>>> >>>> The device tree node is: >>>> >>>> test at 22222 { >>>> compatible = "vendor,testme"; >>>> interrupts = <23 IRQ_TYPE_LEVEL_HIGH>; >>>> }; >>> >>> I assume that this is for the sake of the discussion, and that you do >>> not actually intend to put together such a monstrosity. >> >> It's just missing a reg properties to be a valid node, right? > > If connecting a device that signals its interrupt as level low to an > input line configured as level high doesn't strike you as a major > issue, nothing will. At that point, you can put anything you want in > your DT. If I understand correctly, you are saying that I should have specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? If the HW outputs 1 when idle, and 0 when busy, that is level low? (Sorry if this is obvious, I'm absolutely clueless in this subject matter.) >>>> I wrote a minimal driver which registers the irq. >>>> And in the interrupt handler, I disable said irq. >>>> >>>> Since the irq is IRQ_TYPE_LEVEL_HIGH, it will fire as soon as >>>> it is registered (because the block is idle). >>>> >>>> Here is the code I've been running, request_irq doesn't return. >>> >>> [...] >>> >>>> And here's what I get when I try to load the module: >>>> (I'm using the default CONFIG_RCU_CPU_STALL_TIMEOUT=21) >>> >>> [...] >>> >>>> Are we not supposed to disable the irq in the handler? >>> >>> You can. It then depends on what your interrupt controller does to >>> actually ensure that the interrupt is disabled. Only you can trace it on >>> your HW to find out. >> >> I'm using an upstream driver on v4.9-rc1 >> >> http://lxr.free-electrons.com/source/drivers/irqchip/irq-tango.c >> >> Given that the system locks up, is it possible there is a bug >> in the driver? > > That's possible. > >> Which call-back handles enabling/disabling interrupts? > > How about irq_unmask/irq_mask? I tried following the source from disable_irq_nosync() as far down as I could. disable_irq_nosync -> __disable_irq_nosync -> __disable_irq -> irq_disable -> ?? http://lxr.free-electrons.com/source/kernel/irq/chip.c#L232 I don't know if desc->irq_data.chip->irq_disable is defined by the driver I'm using? I don't know how the trail goes to irq_mask? Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 19:47 ` Mason @ 2016-10-21 19:49 ` Thomas Gleixner -1 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-21 19:49 UTC (permalink / raw) To: Mason; +Cc: Marc Zyngier, Jason Cooper, LKML, Linux ARM, Sebastian Frias On Fri, 21 Oct 2016, Mason wrote: > On 21/10/2016 21:14, Marc Zyngier wrote: > > If connecting a device that signals its interrupt as level low to an > > input line configured as level high doesn't strike you as a major > > issue, nothing will. At that point, you can put anything you want in > > your DT. > > If I understand correctly, you are saying that I should have > specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? > > If the HW outputs 1 when idle, and 0 when busy, that > is level low? (Sorry if this is obvious, I'm absolutely > clueless in this subject matter.) We describe the level which is raising the interrupt. So in your case the line goes to 0 when the interrupt is active, so the level is LOW. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 19:49 ` Thomas Gleixner 0 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-21 19:49 UTC (permalink / raw) To: linux-arm-kernel On Fri, 21 Oct 2016, Mason wrote: > On 21/10/2016 21:14, Marc Zyngier wrote: > > If connecting a device that signals its interrupt as level low to an > > input line configured as level high doesn't strike you as a major > > issue, nothing will. At that point, you can put anything you want in > > your DT. > > If I understand correctly, you are saying that I should have > specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? > > If the HW outputs 1 when idle, and 0 when busy, that > is level low? (Sorry if this is obvious, I'm absolutely > clueless in this subject matter.) We describe the level which is raising the interrupt. So in your case the line goes to 0 when the interrupt is active, so the level is LOW. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 19:49 ` Thomas Gleixner @ 2016-10-21 20:27 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 20:27 UTC (permalink / raw) To: Thomas Gleixner Cc: Marc Zyngier, Jason Cooper, LKML, Linux ARM, Sebastian Frias On 21/10/2016 21:49, Thomas Gleixner wrote: > On Fri, 21 Oct 2016, Mason wrote: >> On 21/10/2016 21:14, Marc Zyngier wrote: >>> If connecting a device that signals its interrupt as level low to an >>> input line configured as level high doesn't strike you as a major >>> issue, nothing will. At that point, you can put anything you want in >>> your DT. >> >> If I understand correctly, you are saying that I should have >> specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? >> >> If the HW outputs 1 when idle, and 0 when busy, that >> is level low? (Sorry if this is obvious, I'm absolutely >> clueless in this subject matter.) > > We describe the level which is raising the interrupt. So in your case the > line goes to 0 when the interrupt is active, so the level is LOW. I see. I'll try that on Monday. In my mental picture of interrupts (which is obviously so incomplete as to be wrong) interrupts are a way for hardware to tell the CPU that they urgently need the CPU's attention. Obviously, the hardware being idle (line high) is not an urgent matter which interests the CPU. Likewise, I'm not sure the CPU cares that the hardware is busy (line low). It seems to me the interesting event from the CPU's perspective is when the hardware completes a "task" (transition from low to high). So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. (There is an edge detection block in the irqchip, but the HW designer warned me that at low frequencies, it is possible to "miss" some edges, and we should prefer level triggers if possible.) Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-21 20:27 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-21 20:27 UTC (permalink / raw) To: linux-arm-kernel On 21/10/2016 21:49, Thomas Gleixner wrote: > On Fri, 21 Oct 2016, Mason wrote: >> On 21/10/2016 21:14, Marc Zyngier wrote: >>> If connecting a device that signals its interrupt as level low to an >>> input line configured as level high doesn't strike you as a major >>> issue, nothing will. At that point, you can put anything you want in >>> your DT. >> >> If I understand correctly, you are saying that I should have >> specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? >> >> If the HW outputs 1 when idle, and 0 when busy, that >> is level low? (Sorry if this is obvious, I'm absolutely >> clueless in this subject matter.) > > We describe the level which is raising the interrupt. So in your case the > line goes to 0 when the interrupt is active, so the level is LOW. I see. I'll try that on Monday. In my mental picture of interrupts (which is obviously so incomplete as to be wrong) interrupts are a way for hardware to tell the CPU that they urgently need the CPU's attention. Obviously, the hardware being idle (line high) is not an urgent matter which interests the CPU. Likewise, I'm not sure the CPU cares that the hardware is busy (line low). It seems to me the interesting event from the CPU's perspective is when the hardware completes a "task" (transition from low to high). So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. (There is an edge detection block in the irqchip, but the HW designer warned me that at low frequencies, it is possible to "miss" some edges, and we should prefer level triggers if possible.) Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-21 20:27 ` Mason @ 2016-10-22 11:37 ` Marc Zyngier -1 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-22 11:37 UTC (permalink / raw) To: Mason; +Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On Fri, 21 Oct 2016 22:27:23 +0200 Mason <slash.tmp@free.fr> wrote: > On 21/10/2016 21:49, Thomas Gleixner wrote: > > On Fri, 21 Oct 2016, Mason wrote: > >> On 21/10/2016 21:14, Marc Zyngier wrote: > >>> If connecting a device that signals its interrupt as level low to an > >>> input line configured as level high doesn't strike you as a major > >>> issue, nothing will. At that point, you can put anything you want in > >>> your DT. > >> > >> If I understand correctly, you are saying that I should have > >> specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? > >> > >> If the HW outputs 1 when idle, and 0 when busy, that > >> is level low? (Sorry if this is obvious, I'm absolutely > >> clueless in this subject matter.) > > > > We describe the level which is raising the interrupt. So in your case the > > line goes to 0 when the interrupt is active, so the level is LOW. > > I see. I'll try that on Monday. > > In my mental picture of interrupts (which is obviously so > incomplete as to be wrong) interrupts are a way for hardware > to tell the CPU that they urgently need the CPU's attention. That's how the CPU interprets it, but this is even more basic than that, see below. > Obviously, the hardware being idle (line high) is not an urgent > matter which interests the CPU. Likewise, I'm not sure the CPU > cares that the hardware is busy (line low). It seems to me the > interesting event from the CPU's perspective is when the > hardware completes a "task" (transition from low to high). There is no such thing as "busy" when it comes to interrupts. An interrupt signals the CPU that some device-specific condition has been satisfied. It could be "I've received a packet" or "Battery is about to explode", depending if the device is a network controller or a temperature sensor. The interrupt doesn't describe the process that leads to that condition (packet being received or temperature rising), but the condition itself. In your cases, as the device seems to do some form of processing (you're talking about task completion), then the interrupt seems to describe exactly this ("I'm done"). > So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. > (There is an edge detection block in the irqchip, but the HW designer > warned me that at low frequencies, it is possible to "miss" some edges, > and we should prefer level triggers if possible.) Level and edge are not interchangeable. They do describe very different thing: - Level indicates a persistent state, which implies that the device needs to be serviced so that this condition can be cleared (the UART has received a character, and won't be able to received another until it has been read by the CPU). Once the device has been serviced and that condition cleared, it will lower its interrupt line. - Edge is indicative of an event having occurred ("I'm done") that doesn't require any action from the CPU. Because the device can continue its life without being poked by the CPU, it can continue delivering interrupts even if the first one hasn't been serviced. Being edge triggered, the signals get coalesced into a single interrupt. For example, the temperature sensor will say "Temperature rising" multiple times before the battery explodes, and it is the CPU's job to go and read the sensor to find out by how much it has risen. If your device only sends a pulse, then it is edge triggered, and it should be treated as such, no matter what your HW guy is saying. This usually involves looking at the device to find out how many times the interrupt has been generated (assuming the device is some kind of processing element). Of course, this is racy (interrupts can still be generated whilst you're processing them), and you should design your interrupt handler to take care of the possible race. So, to make it short: find out how your device works, and configure your interrupt controller in a similar way. Write your device driver with the interrupt policy in mind (state vs event). Keep it simple. Thanks, M. -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-22 11:37 ` Marc Zyngier 0 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-22 11:37 UTC (permalink / raw) To: linux-arm-kernel On Fri, 21 Oct 2016 22:27:23 +0200 Mason <slash.tmp@free.fr> wrote: > On 21/10/2016 21:49, Thomas Gleixner wrote: > > On Fri, 21 Oct 2016, Mason wrote: > >> On 21/10/2016 21:14, Marc Zyngier wrote: > >>> If connecting a device that signals its interrupt as level low to an > >>> input line configured as level high doesn't strike you as a major > >>> issue, nothing will. At that point, you can put anything you want in > >>> your DT. > >> > >> If I understand correctly, you are saying that I should have > >> specified IRQ_TYPE_LEVEL_LOW, instead of IRQ_TYPE_LEVEL_HIGH? > >> > >> If the HW outputs 1 when idle, and 0 when busy, that > >> is level low? (Sorry if this is obvious, I'm absolutely > >> clueless in this subject matter.) > > > > We describe the level which is raising the interrupt. So in your case the > > line goes to 0 when the interrupt is active, so the level is LOW. > > I see. I'll try that on Monday. > > In my mental picture of interrupts (which is obviously so > incomplete as to be wrong) interrupts are a way for hardware > to tell the CPU that they urgently need the CPU's attention. That's how the CPU interprets it, but this is even more basic than that, see below. > Obviously, the hardware being idle (line high) is not an urgent > matter which interests the CPU. Likewise, I'm not sure the CPU > cares that the hardware is busy (line low). It seems to me the > interesting event from the CPU's perspective is when the > hardware completes a "task" (transition from low to high). There is no such thing as "busy" when it comes to interrupts. An interrupt signals the CPU that some device-specific condition has been satisfied. It could be "I've received a packet" or "Battery is about to explode", depending if the device is a network controller or a temperature sensor. The interrupt doesn't describe the process that leads to that condition (packet being received or temperature rising), but the condition itself. In your cases, as the device seems to do some form of processing (you're talking about task completion), then the interrupt seems to describe exactly this ("I'm done"). > So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. > (There is an edge detection block in the irqchip, but the HW designer > warned me that at low frequencies, it is possible to "miss" some edges, > and we should prefer level triggers if possible.) Level and edge are not interchangeable. They do describe very different thing: - Level indicates a persistent state, which implies that the device needs to be serviced so that this condition can be cleared (the UART has received a character, and won't be able to received another until it has been read by the CPU). Once the device has been serviced and that condition cleared, it will lower its interrupt line. - Edge is indicative of an event having occurred ("I'm done") that doesn't require any action from the CPU. Because the device can continue its life without being poked by the CPU, it can continue delivering interrupts even if the first one hasn't been serviced. Being edge triggered, the signals get coalesced into a single interrupt. For example, the temperature sensor will say "Temperature rising" multiple times before the battery explodes, and it is the CPU's job to go and read the sensor to find out by how much it has risen. If your device only sends a pulse, then it is edge triggered, and it should be treated as such, no matter what your HW guy is saying. This usually involves looking at the device to find out how many times the interrupt has been generated (assuming the device is some kind of processing element). Of course, this is racy (interrupts can still be generated whilst you're processing them), and you should design your interrupt handler to take care of the possible race. So, to make it short: find out how your device works, and configure your interrupt controller in a similar way. Write your device driver with the interrupt policy in mind (state vs event). Keep it simple. Thanks, M. -- Jazz is not dead. It just smells funny. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-22 11:37 ` Marc Zyngier @ 2016-10-22 23:10 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-22 23:10 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On 22/10/2016 13:37, Marc Zyngier wrote: > Mason wrote: > >> In my mental picture of interrupts (which is obviously so >> incomplete as to be wrong) interrupts are a way for hardware >> to tell the CPU that they urgently need the CPU's attention. > > That's how the CPU interprets it, but this is even more basic than > that, see below. > >> Obviously, the hardware being idle (line high) is not an urgent >> matter which interests the CPU. Likewise, I'm not sure the CPU >> cares that the hardware is busy (line low). It seems to me the >> interesting event from the CPU's perspective is when the >> hardware completes a "task" (transition from low to high). > > There is no such thing as "busy" when it comes to interrupts. An > interrupt signals the CPU that some device-specific condition has been > satisfied. It could be "I've received a packet" or "Battery is about to > explode", depending if the device is a network controller or a > temperature sensor. The interrupt doesn't describe the process that > leads to that condition (packet being received or temperature rising), > but the condition itself. > > In your cases, as the device seems to do some form of processing > (you're talking about task completion), then the interrupt seems to > describe exactly this ("I'm done"). The device is a graphics engine, which can be programmed to perform some operation on one or several frame buffers stored in memory. It outputs its state (idle vs busy) on interrupt line 23. >> So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. >> (There is an edge detection block in the irqchip, but the HW designer >> warned me that at low frequencies, it is possible to "miss" some edges, >> and we should prefer level triggers if possible.) > > Level and edge are not interchangeable. They do describe very different > thing: > > - Level indicates a persistent state, which implies that the device > needs to be serviced so that this condition can be cleared (the UART > has received a character, and won't be able to received another until > it has been read by the CPU). Once the device has been serviced and > that condition cleared, it will lower its interrupt line. With this graphics engine, there is nothing the CPU can do to change what the engine outputs on the interrupt line: When the graphics engine is idle, the line remains high, forever. When the graphics engine is busy, the line remains low, until all operations have been performed (engine idle). All the CPU can do is mask the interrupt line at the interrupt controller, as far as I understand. > - Edge is indicative of an event having occurred ("I'm done") that > doesn't require any action from the CPU. Because the device can > continue its life without being poked by the CPU, it can continue > delivering interrupts even if the first one hasn't been serviced. > Being edge triggered, the signals get coalesced into a single > interrupt. For example, the temperature sensor will say "Temperature > rising" multiple times before the battery explodes, and it is the > CPU's job to go and read the sensor to find out by how much it has > risen. > > If your device only sends a pulse, then it is edge triggered, and it > should be treated as such, no matter what your HW guy is saying. This > usually involves looking at the device to find out how many times the > interrupt has been generated (assuming the device is some kind of > processing element). Of course, this is racy (interrupts can still be > generated whilst you're processing them), and you should design your > interrupt handler to take care of the possible race. It is clear that the block does not send a pulse on the interrupt line. For reasons I don't understand, Linux didn't hang when I set the IRQ type to IRQ_TYPE_EDGE_RISING, so it seemed better than locking up the system. I'm also fuzzy on what purpose the edge detector is supposed to serve... I had the impression is what supposed to "capture" an edge, to turn it into a level? > So, to make it short: find out how your device works, and configure > your interrupt controller in a similar way. Write your device driver > with the interrupt policy in mind (state vs event). Keep it simple. Thomas said "We describe the level which is raising the interrupt". But I'm not sure I want the state "engine is busy" to raise an interrupt. "engine is idle" makes more sense. But you said it's stupid to set IRQ_TYPE_LEVEL_HIGH... /me confused Maybe the fact that disable_irq locks the system up is an orthogonal issue that needs to be fixed anyway. Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-22 23:10 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-22 23:10 UTC (permalink / raw) To: linux-arm-kernel On 22/10/2016 13:37, Marc Zyngier wrote: > Mason wrote: > >> In my mental picture of interrupts (which is obviously so >> incomplete as to be wrong) interrupts are a way for hardware >> to tell the CPU that they urgently need the CPU's attention. > > That's how the CPU interprets it, but this is even more basic than > that, see below. > >> Obviously, the hardware being idle (line high) is not an urgent >> matter which interests the CPU. Likewise, I'm not sure the CPU >> cares that the hardware is busy (line low). It seems to me the >> interesting event from the CPU's perspective is when the >> hardware completes a "task" (transition from low to high). > > There is no such thing as "busy" when it comes to interrupts. An > interrupt signals the CPU that some device-specific condition has been > satisfied. It could be "I've received a packet" or "Battery is about to > explode", depending if the device is a network controller or a > temperature sensor. The interrupt doesn't describe the process that > leads to that condition (packet being received or temperature rising), > but the condition itself. > > In your cases, as the device seems to do some form of processing > (you're talking about task completion), then the interrupt seems to > describe exactly this ("I'm done"). The device is a graphics engine, which can be programmed to perform some operation on one or several frame buffers stored in memory. It outputs its state (idle vs busy) on interrupt line 23. >> So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. >> (There is an edge detection block in the irqchip, but the HW designer >> warned me that at low frequencies, it is possible to "miss" some edges, >> and we should prefer level triggers if possible.) > > Level and edge are not interchangeable. They do describe very different > thing: > > - Level indicates a persistent state, which implies that the device > needs to be serviced so that this condition can be cleared (the UART > has received a character, and won't be able to received another until > it has been read by the CPU). Once the device has been serviced and > that condition cleared, it will lower its interrupt line. With this graphics engine, there is nothing the CPU can do to change what the engine outputs on the interrupt line: When the graphics engine is idle, the line remains high, forever. When the graphics engine is busy, the line remains low, until all operations have been performed (engine idle). All the CPU can do is mask the interrupt line at the interrupt controller, as far as I understand. > - Edge is indicative of an event having occurred ("I'm done") that > doesn't require any action from the CPU. Because the device can > continue its life without being poked by the CPU, it can continue > delivering interrupts even if the first one hasn't been serviced. > Being edge triggered, the signals get coalesced into a single > interrupt. For example, the temperature sensor will say "Temperature > rising" multiple times before the battery explodes, and it is the > CPU's job to go and read the sensor to find out by how much it has > risen. > > If your device only sends a pulse, then it is edge triggered, and it > should be treated as such, no matter what your HW guy is saying. This > usually involves looking at the device to find out how many times the > interrupt has been generated (assuming the device is some kind of > processing element). Of course, this is racy (interrupts can still be > generated whilst you're processing them), and you should design your > interrupt handler to take care of the possible race. It is clear that the block does not send a pulse on the interrupt line. For reasons I don't understand, Linux didn't hang when I set the IRQ type to IRQ_TYPE_EDGE_RISING, so it seemed better than locking up the system. I'm also fuzzy on what purpose the edge detector is supposed to serve... I had the impression is what supposed to "capture" an edge, to turn it into a level? > So, to make it short: find out how your device works, and configure > your interrupt controller in a similar way. Write your device driver > with the interrupt policy in mind (state vs event). Keep it simple. Thomas said "We describe the level which is raising the interrupt". But I'm not sure I want the state "engine is busy" to raise an interrupt. "engine is idle" makes more sense. But you said it's stupid to set IRQ_TYPE_LEVEL_HIGH... /me confused Maybe the fact that disable_irq locks the system up is an orthogonal issue that needs to be fixed anyway. Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-22 23:10 ` Mason @ 2016-10-24 8:17 ` Marc Zyngier -1 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-24 8:17 UTC (permalink / raw) To: Mason; +Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On 23/10/16 00:10, Mason wrote: > On 22/10/2016 13:37, Marc Zyngier wrote: > >> Mason wrote: >> >>> In my mental picture of interrupts (which is obviously so >>> incomplete as to be wrong) interrupts are a way for hardware >>> to tell the CPU that they urgently need the CPU's attention. >> >> That's how the CPU interprets it, but this is even more basic than >> that, see below. >> >>> Obviously, the hardware being idle (line high) is not an urgent >>> matter which interests the CPU. Likewise, I'm not sure the CPU >>> cares that the hardware is busy (line low). It seems to me the >>> interesting event from the CPU's perspective is when the >>> hardware completes a "task" (transition from low to high). >> >> There is no such thing as "busy" when it comes to interrupts. An >> interrupt signals the CPU that some device-specific condition has been >> satisfied. It could be "I've received a packet" or "Battery is about to >> explode", depending if the device is a network controller or a >> temperature sensor. The interrupt doesn't describe the process that >> leads to that condition (packet being received or temperature rising), >> but the condition itself. >> >> In your cases, as the device seems to do some form of processing >> (you're talking about task completion), then the interrupt seems to >> describe exactly this ("I'm done"). > > The device is a graphics engine, which can be programmed to perform > some operation on one or several frame buffers stored in memory. > It outputs its state (idle vs busy) on interrupt line 23. > >>> So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. >>> (There is an edge detection block in the irqchip, but the HW designer >>> warned me that at low frequencies, it is possible to "miss" some edges, >>> and we should prefer level triggers if possible.) >> >> Level and edge are not interchangeable. They do describe very different >> thing: >> >> - Level indicates a persistent state, which implies that the device >> needs to be serviced so that this condition can be cleared (the UART >> has received a character, and won't be able to received another until >> it has been read by the CPU). Once the device has been serviced and >> that condition cleared, it will lower its interrupt line. > > With this graphics engine, there is nothing the CPU can do to > change what the engine outputs on the interrupt line: > > When the graphics engine is idle, the line remains high, forever. > When the graphics engine is busy, the line remains low, until > all operations have been performed (engine idle). > > All the CPU can do is mask the interrupt line at the interrupt > controller, as far as I understand. Then this is unambiguously a rising edge interrupt. > >> - Edge is indicative of an event having occurred ("I'm done") that >> doesn't require any action from the CPU. Because the device can >> continue its life without being poked by the CPU, it can continue >> delivering interrupts even if the first one hasn't been serviced. >> Being edge triggered, the signals get coalesced into a single >> interrupt. For example, the temperature sensor will say "Temperature >> rising" multiple times before the battery explodes, and it is the >> CPU's job to go and read the sensor to find out by how much it has >> risen. >> >> If your device only sends a pulse, then it is edge triggered, and it >> should be treated as such, no matter what your HW guy is saying. This >> usually involves looking at the device to find out how many times the >> interrupt has been generated (assuming the device is some kind of >> processing element). Of course, this is racy (interrupts can still be >> generated whilst you're processing them), and you should design your >> interrupt handler to take care of the possible race. > > It is clear that the block does not send a pulse on the > interrupt line. > > For reasons I don't understand, Linux didn't hang when I set > the IRQ type to IRQ_TYPE_EDGE_RISING, so it seemed better > than locking up the system. Because that's exactly what that is. > > I'm also fuzzy on what purpose the edge detector is supposed > to serve... I had the impression is what supposed to "capture" > an edge, to turn it into a level? If you care to read the explanation I've given above, you'll realize that you cannot turn an edge into a level, because they don't represent the same thing (state vs event). The interrupt controller will latch on a rising edge (for example), and present that information to the CPU, no matter what the line does after that. >> So, to make it short: find out how your device works, and configure >> your interrupt controller in a similar way. Write your device driver >> with the interrupt policy in mind (state vs event). Keep it simple. > > Thomas said "We describe the level which is raising the interrupt". > But I'm not sure I want the state "engine is busy" to raise an > interrupt. "engine is idle" makes more sense. But you said it's > stupid to set IRQ_TYPE_LEVEL_HIGH... /me confused Because you insist on considering as a level something that is an edge. Once you try to understand the nature of the signal the device is providing, then you may stop getting confused. You definitely don't want to generate an interrupt when the device is idle, because that's a state on which you cannot act (apart from constantly generating job for your graphics engine). What you want to detect is the *transition* from busy to idle (event). Nothing else matters. > Maybe the fact that disable_irq locks the system up is an orthogonal > issue that needs to be fixed anyway. Indeed. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-24 8:17 ` Marc Zyngier 0 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-24 8:17 UTC (permalink / raw) To: linux-arm-kernel On 23/10/16 00:10, Mason wrote: > On 22/10/2016 13:37, Marc Zyngier wrote: > >> Mason wrote: >> >>> In my mental picture of interrupts (which is obviously so >>> incomplete as to be wrong) interrupts are a way for hardware >>> to tell the CPU that they urgently need the CPU's attention. >> >> That's how the CPU interprets it, but this is even more basic than >> that, see below. >> >>> Obviously, the hardware being idle (line high) is not an urgent >>> matter which interests the CPU. Likewise, I'm not sure the CPU >>> cares that the hardware is busy (line low). It seems to me the >>> interesting event from the CPU's perspective is when the >>> hardware completes a "task" (transition from low to high). >> >> There is no such thing as "busy" when it comes to interrupts. An >> interrupt signals the CPU that some device-specific condition has been >> satisfied. It could be "I've received a packet" or "Battery is about to >> explode", depending if the device is a network controller or a >> temperature sensor. The interrupt doesn't describe the process that >> leads to that condition (packet being received or temperature rising), >> but the condition itself. >> >> In your cases, as the device seems to do some form of processing >> (you're talking about task completion), then the interrupt seems to >> describe exactly this ("I'm done"). > > The device is a graphics engine, which can be programmed to perform > some operation on one or several frame buffers stored in memory. > It outputs its state (idle vs busy) on interrupt line 23. > >>> So I had originally configured the interrupt as IRQ_TYPE_EDGE_RISING. >>> (There is an edge detection block in the irqchip, but the HW designer >>> warned me that at low frequencies, it is possible to "miss" some edges, >>> and we should prefer level triggers if possible.) >> >> Level and edge are not interchangeable. They do describe very different >> thing: >> >> - Level indicates a persistent state, which implies that the device >> needs to be serviced so that this condition can be cleared (the UART >> has received a character, and won't be able to received another until >> it has been read by the CPU). Once the device has been serviced and >> that condition cleared, it will lower its interrupt line. > > With this graphics engine, there is nothing the CPU can do to > change what the engine outputs on the interrupt line: > > When the graphics engine is idle, the line remains high, forever. > When the graphics engine is busy, the line remains low, until > all operations have been performed (engine idle). > > All the CPU can do is mask the interrupt line at the interrupt > controller, as far as I understand. Then this is unambiguously a rising edge interrupt. > >> - Edge is indicative of an event having occurred ("I'm done") that >> doesn't require any action from the CPU. Because the device can >> continue its life without being poked by the CPU, it can continue >> delivering interrupts even if the first one hasn't been serviced. >> Being edge triggered, the signals get coalesced into a single >> interrupt. For example, the temperature sensor will say "Temperature >> rising" multiple times before the battery explodes, and it is the >> CPU's job to go and read the sensor to find out by how much it has >> risen. >> >> If your device only sends a pulse, then it is edge triggered, and it >> should be treated as such, no matter what your HW guy is saying. This >> usually involves looking at the device to find out how many times the >> interrupt has been generated (assuming the device is some kind of >> processing element). Of course, this is racy (interrupts can still be >> generated whilst you're processing them), and you should design your >> interrupt handler to take care of the possible race. > > It is clear that the block does not send a pulse on the > interrupt line. > > For reasons I don't understand, Linux didn't hang when I set > the IRQ type to IRQ_TYPE_EDGE_RISING, so it seemed better > than locking up the system. Because that's exactly what that is. > > I'm also fuzzy on what purpose the edge detector is supposed > to serve... I had the impression is what supposed to "capture" > an edge, to turn it into a level? If you care to read the explanation I've given above, you'll realize that you cannot turn an edge into a level, because they don't represent the same thing (state vs event). The interrupt controller will latch on a rising edge (for example), and present that information to the CPU, no matter what the line does after that. >> So, to make it short: find out how your device works, and configure >> your interrupt controller in a similar way. Write your device driver >> with the interrupt policy in mind (state vs event). Keep it simple. > > Thomas said "We describe the level which is raising the interrupt". > But I'm not sure I want the state "engine is busy" to raise an > interrupt. "engine is idle" makes more sense. But you said it's > stupid to set IRQ_TYPE_LEVEL_HIGH... /me confused Because you insist on considering as a level something that is an edge. Once you try to understand the nature of the signal the device is providing, then you may stop getting confused. You definitely don't want to generate an interrupt when the device is idle, because that's a state on which you cannot act (apart from constantly generating job for your graphics engine). What you want to detect is the *transition* from busy to idle (event). Nothing else matters. > Maybe the fact that disable_irq locks the system up is an orthogonal > issue that needs to be fixed anyway. Indeed. M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-22 23:10 ` Mason @ 2016-10-24 16:12 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-24 16:12 UTC (permalink / raw) To: Marc Zyngier Cc: Thomas Gleixner, Jason Cooper, LKML, Linux ARM, Sebastian Frias On 23/10/2016 01:10, Mason wrote: > Maybe the fact that disable_irq locks the system up is an orthogonal > issue that needs to be fixed anyway. disable_irq_nosync() eventually calls irq_disable() void irq_disable(struct irq_desc *desc) { irq_state_set_disabled(desc); if (desc->irq_data.chip->irq_disable) { desc->irq_data.chip->irq_disable(&desc->irq_data); irq_state_set_masked(desc); } else if (irq_settings_disable_unlazy(desc)) { mask_irq(desc); } } irq_disable() is a NOP on my platform, because the intc driver does not implement irq_disable, and the second test is false as well in this instance. The function's description is interesting. /** * irq_disable - Mark interrupt disabled * @desc: irq descriptor which should be disabled * * If the chip does not implement the irq_disable callback, we * use a lazy disable approach. That means we mark the interrupt * disabled, but leave the hardware unmasked. That's an * optimization because we avoid the hardware access for the * common case where no interrupt happens after we marked it * disabled. If an interrupt happens, then the interrupt flow * handler masks the line at the hardware level and marks it * pending. * * If the interrupt chip does not implement the irq_disable callback, * a driver can disable the lazy approach for a particular irq line by * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can * be used for devices which cannot disable the interrupt at the * device level under certain circumstances and have to use * disable_irq[_nosync] instead. */ (I assume "chip" and "interrupt chip" refer to the same abstraction.) I took a look at commit e9849777d0e27, but my brain dumped core on the notions of "disabling unlazy" and "disabling a disable". * IRQ_DISABLE_UNLAZY - Disable lazy irq disable For the record, setting the IRQ_DISABLE_UNLAZY flag for this device makes the system lock-up disappear. Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-24 16:12 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-24 16:12 UTC (permalink / raw) To: linux-arm-kernel On 23/10/2016 01:10, Mason wrote: > Maybe the fact that disable_irq locks the system up is an orthogonal > issue that needs to be fixed anyway. disable_irq_nosync() eventually calls irq_disable() void irq_disable(struct irq_desc *desc) { irq_state_set_disabled(desc); if (desc->irq_data.chip->irq_disable) { desc->irq_data.chip->irq_disable(&desc->irq_data); irq_state_set_masked(desc); } else if (irq_settings_disable_unlazy(desc)) { mask_irq(desc); } } irq_disable() is a NOP on my platform, because the intc driver does not implement irq_disable, and the second test is false as well in this instance. The function's description is interesting. /** * irq_disable - Mark interrupt disabled * @desc: irq descriptor which should be disabled * * If the chip does not implement the irq_disable callback, we * use a lazy disable approach. That means we mark the interrupt * disabled, but leave the hardware unmasked. That's an * optimization because we avoid the hardware access for the * common case where no interrupt happens after we marked it * disabled. If an interrupt happens, then the interrupt flow * handler masks the line at the hardware level and marks it * pending. * * If the interrupt chip does not implement the irq_disable callback, * a driver can disable the lazy approach for a particular irq line by * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can * be used for devices which cannot disable the interrupt at the * device level under certain circumstances and have to use * disable_irq[_nosync] instead. */ (I assume "chip" and "interrupt chip" refer to the same abstraction.) I took a look at commit e9849777d0e27, but my brain dumped core on the notions of "disabling unlazy" and "disabling a disable". * IRQ_DISABLE_UNLAZY - Disable lazy irq disable For the record, setting the IRQ_DISABLE_UNLAZY flag for this device makes the system lock-up disappear. Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-24 16:12 ` Mason @ 2016-10-24 16:55 ` Thomas Gleixner -1 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-24 16:55 UTC (permalink / raw) To: Mason; +Cc: Marc Zyngier, Jason Cooper, LKML, Linux ARM, Sebastian Frias On Mon, 24 Oct 2016, Mason wrote: > > For the record, setting the IRQ_DISABLE_UNLAZY flag for this device > makes the system lock-up disappear. The way how lazy irq disabling works is: 1) Interrupt is marked disabled in software, but the hardware is not masked 2) If the interrupt fires befor the interrupt is reenabled, then it's masked at the hardware level in the low level interrupt flow handler. I have no idea why that does not work on your hardware. You might instrument handle_level_irq() to see what effect that has. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-24 16:55 ` Thomas Gleixner 0 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-24 16:55 UTC (permalink / raw) To: linux-arm-kernel On Mon, 24 Oct 2016, Mason wrote: > > For the record, setting the IRQ_DISABLE_UNLAZY flag for this device > makes the system lock-up disappear. The way how lazy irq disabling works is: 1) Interrupt is marked disabled in software, but the hardware is not masked 2) If the interrupt fires befor the interrupt is reenabled, then it's masked at the hardware level in the low level interrupt flow handler. I have no idea why that does not work on your hardware. You might instrument handle_level_irq() to see what effect that has. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-24 16:55 ` Thomas Gleixner @ 2016-10-25 8:29 ` Sebastian Frias -1 siblings, 0 replies; 36+ messages in thread From: Sebastian Frias @ 2016-10-25 8:29 UTC (permalink / raw) To: Thomas Gleixner, Mason; +Cc: Marc Zyngier, Jason Cooper, LKML, Linux ARM Hi Thomas, On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > On Mon, 24 Oct 2016, Mason wrote: >> >> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >> makes the system lock-up disappear. > > The way how lazy irq disabling works is: > > 1) Interrupt is marked disabled in software, but the hardware is not masked > > 2) If the interrupt fires befor the interrupt is reenabled, then it's > masked at the hardware level in the low level interrupt flow handler. > Would you mind explaining what is the intention behind? Because it does not seem obvious why there isn't a direct map between "disable_irq*()" and "mask_irq()" Thanks in advance. Best regards, Sebastian ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 8:29 ` Sebastian Frias 0 siblings, 0 replies; 36+ messages in thread From: Sebastian Frias @ 2016-10-25 8:29 UTC (permalink / raw) To: linux-arm-kernel Hi Thomas, On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > On Mon, 24 Oct 2016, Mason wrote: >> >> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >> makes the system lock-up disappear. > > The way how lazy irq disabling works is: > > 1) Interrupt is marked disabled in software, but the hardware is not masked > > 2) If the interrupt fires befor the interrupt is reenabled, then it's > masked at the hardware level in the low level interrupt flow handler. > Would you mind explaining what is the intention behind? Because it does not seem obvious why there isn't a direct map between "disable_irq*()" and "mask_irq()" Thanks in advance. Best regards, Sebastian ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-25 8:29 ` Sebastian Frias @ 2016-10-25 8:36 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-25 8:36 UTC (permalink / raw) To: Sebastian Frias, Thomas Gleixner Cc: Marc Zyngier, Jason Cooper, LKML, Linux ARM On 25/10/2016 10:29, Sebastian Frias wrote: > On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > >> On Mon, 24 Oct 2016, Mason wrote: >> >>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>> makes the system lock-up disappear. >> >> The way how lazy irq disabling works is: >> >> 1) Interrupt is marked disabled in software, but the hardware is not masked >> >> 2) If the interrupt fires befor the interrupt is reenabled, then it's >> masked at the hardware level in the low level interrupt flow handler. > > Would you mind explaining what is the intention behind? > Because it does not seem obvious why there isn't a direct map between > "disable_irq*()" and "mask_irq()" I had a similar, but slightly different question: What is the difference between struct irq_chip's * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) * @irq_disable: disable the interrupt * @irq_mask: mask an interrupt source (enable seems to default to unmask, but disable does not default to mask) Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 8:36 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-25 8:36 UTC (permalink / raw) To: linux-arm-kernel On 25/10/2016 10:29, Sebastian Frias wrote: > On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > >> On Mon, 24 Oct 2016, Mason wrote: >> >>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>> makes the system lock-up disappear. >> >> The way how lazy irq disabling works is: >> >> 1) Interrupt is marked disabled in software, but the hardware is not masked >> >> 2) If the interrupt fires befor the interrupt is reenabled, then it's >> masked at the hardware level in the low level interrupt flow handler. > > Would you mind explaining what is the intention behind? > Because it does not seem obvious why there isn't a direct map between > "disable_irq*()" and "mask_irq()" I had a similar, but slightly different question: What is the difference between struct irq_chip's * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) * @irq_disable: disable the interrupt * @irq_mask: mask an interrupt source (enable seems to default to unmask, but disable does not default to mask) Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-25 8:36 ` Mason @ 2016-10-25 10:45 ` Marc Zyngier -1 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-25 10:45 UTC (permalink / raw) To: Mason, Sebastian Frias, Thomas Gleixner; +Cc: Jason Cooper, LKML, Linux ARM On 25/10/16 09:36, Mason wrote: > On 25/10/2016 10:29, Sebastian Frias wrote: > >> On 10/24/2016 06:55 PM, Thomas Gleixner wrote: >> >>> On Mon, 24 Oct 2016, Mason wrote: >>> >>>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>>> makes the system lock-up disappear. >>> >>> The way how lazy irq disabling works is: >>> >>> 1) Interrupt is marked disabled in software, but the hardware is not masked >>> >>> 2) If the interrupt fires befor the interrupt is reenabled, then it's >>> masked at the hardware level in the low level interrupt flow handler. >> >> Would you mind explaining what is the intention behind? >> Because it does not seem obvious why there isn't a direct map between >> "disable_irq*()" and "mask_irq()" > > I had a similar, but slightly different question: > > What is the difference between struct irq_chip's > > * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) > * @irq_disable: disable the interrupt > * @irq_mask: mask an interrupt source One important difference between disable and mask is that disable is perfectly allowed not to care about pending signals, whereas mask must preserve an interrupt becoming pending whilst masked. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 10:45 ` Marc Zyngier 0 siblings, 0 replies; 36+ messages in thread From: Marc Zyngier @ 2016-10-25 10:45 UTC (permalink / raw) To: linux-arm-kernel On 25/10/16 09:36, Mason wrote: > On 25/10/2016 10:29, Sebastian Frias wrote: > >> On 10/24/2016 06:55 PM, Thomas Gleixner wrote: >> >>> On Mon, 24 Oct 2016, Mason wrote: >>> >>>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>>> makes the system lock-up disappear. >>> >>> The way how lazy irq disabling works is: >>> >>> 1) Interrupt is marked disabled in software, but the hardware is not masked >>> >>> 2) If the interrupt fires befor the interrupt is reenabled, then it's >>> masked at the hardware level in the low level interrupt flow handler. >> >> Would you mind explaining what is the intention behind? >> Because it does not seem obvious why there isn't a direct map between >> "disable_irq*()" and "mask_irq()" > > I had a similar, but slightly different question: > > What is the difference between struct irq_chip's > > * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) > * @irq_disable: disable the interrupt > * @irq_mask: mask an interrupt source One important difference between disable and mask is that disable is perfectly allowed not to care about pending signals, whereas mask must preserve an interrupt becoming pending whilst masked. Thanks, M. -- Jazz is not dead. It just smells funny... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-25 10:45 ` Marc Zyngier @ 2016-10-25 13:56 ` Mason -1 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-25 13:56 UTC (permalink / raw) To: Marc Zyngier, Sebastian Frias, Thomas Gleixner Cc: Jason Cooper, LKML, Linux ARM On 25/10/2016 12:45, Marc Zyngier wrote: > On 25/10/16 09:36, Mason wrote: >> On 25/10/2016 10:29, Sebastian Frias wrote: >> >>> On 10/24/2016 06:55 PM, Thomas Gleixner wrote: >>> >>>> On Mon, 24 Oct 2016, Mason wrote: >>>> >>>>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>>>> makes the system lock-up disappear. >>>> >>>> The way how lazy irq disabling works is: >>>> >>>> 1) Interrupt is marked disabled in software, but the hardware is not masked >>>> >>>> 2) If the interrupt fires befor the interrupt is reenabled, then it's >>>> masked at the hardware level in the low level interrupt flow handler. >>> >>> Would you mind explaining what is the intention behind? >>> Because it does not seem obvious why there isn't a direct map between >>> "disable_irq*()" and "mask_irq()" >> >> I had a similar, but slightly different question: >> >> What is the difference between struct irq_chip's >> >> * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) >> * @irq_disable: disable the interrupt >> * @irq_mask: mask an interrupt source > > One important difference between disable and mask is that disable is > perfectly allowed not to care about pending signals, whereas mask must > preserve an interrupt becoming pending whilst masked. (For my information) Is it correct to say that "mask" is supposed to defer any interrupt until sometime later; while "disable" will simply discard incoming interrupts, losing them forever. Is the irq_mask() call-back exposed via some module-visible API? include/linux/interrupt.h documents mostly enable/disable variants. extern void disable_irq_nosync(unsigned int irq); extern bool disable_hardirq(unsigned int irq); extern void disable_irq(unsigned int irq); extern void disable_percpu_irq(unsigned int irq); extern void enable_irq(unsigned int irq); extern void enable_percpu_irq(unsigned int irq, unsigned int type); extern bool irq_percpu_is_enabled(unsigned int irq); extern void irq_wake_thread(unsigned int irq, void *dev_id); Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 13:56 ` Mason 0 siblings, 0 replies; 36+ messages in thread From: Mason @ 2016-10-25 13:56 UTC (permalink / raw) To: linux-arm-kernel On 25/10/2016 12:45, Marc Zyngier wrote: > On 25/10/16 09:36, Mason wrote: >> On 25/10/2016 10:29, Sebastian Frias wrote: >> >>> On 10/24/2016 06:55 PM, Thomas Gleixner wrote: >>> >>>> On Mon, 24 Oct 2016, Mason wrote: >>>> >>>>> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device >>>>> makes the system lock-up disappear. >>>> >>>> The way how lazy irq disabling works is: >>>> >>>> 1) Interrupt is marked disabled in software, but the hardware is not masked >>>> >>>> 2) If the interrupt fires befor the interrupt is reenabled, then it's >>>> masked at the hardware level in the low level interrupt flow handler. >>> >>> Would you mind explaining what is the intention behind? >>> Because it does not seem obvious why there isn't a direct map between >>> "disable_irq*()" and "mask_irq()" >> >> I had a similar, but slightly different question: >> >> What is the difference between struct irq_chip's >> >> * @irq_shutdown: shut down the interrupt (defaults to ->disable if NULL) >> * @irq_disable: disable the interrupt >> * @irq_mask: mask an interrupt source > > One important difference between disable and mask is that disable is > perfectly allowed not to care about pending signals, whereas mask must > preserve an interrupt becoming pending whilst masked. (For my information) Is it correct to say that "mask" is supposed to defer any interrupt until sometime later; while "disable" will simply discard incoming interrupts, losing them forever. Is the irq_mask() call-back exposed via some module-visible API? include/linux/interrupt.h documents mostly enable/disable variants. extern void disable_irq_nosync(unsigned int irq); extern bool disable_hardirq(unsigned int irq); extern void disable_irq(unsigned int irq); extern void disable_percpu_irq(unsigned int irq); extern void enable_irq(unsigned int irq); extern void enable_percpu_irq(unsigned int irq, unsigned int type); extern bool irq_percpu_is_enabled(unsigned int irq); extern void irq_wake_thread(unsigned int irq, void *dev_id); Regards. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-25 13:56 ` Mason @ 2016-10-25 13:56 ` Thomas Gleixner -1 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-25 13:56 UTC (permalink / raw) To: Mason; +Cc: Marc Zyngier, Sebastian Frias, Jason Cooper, LKML, Linux ARM On Tue, 25 Oct 2016, Mason wrote: > Is the irq_mask() call-back exposed via some module-visible API? No. And there is no reason to do so. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 13:56 ` Thomas Gleixner 0 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-25 13:56 UTC (permalink / raw) To: linux-arm-kernel On Tue, 25 Oct 2016, Mason wrote: > Is the irq_mask() call-back exposed via some module-visible API? No. And there is no reason to do so. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Disabling an interrupt in the handler locks the system up 2016-10-25 8:29 ` Sebastian Frias @ 2016-10-25 9:20 ` Thomas Gleixner -1 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-25 9:20 UTC (permalink / raw) To: Sebastian Frias; +Cc: Mason, Marc Zyngier, Jason Cooper, LKML, Linux ARM On Tue, 25 Oct 2016, Sebastian Frias wrote: > On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > > On Mon, 24 Oct 2016, Mason wrote: > >> > >> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device > >> makes the system lock-up disappear. > > > > The way how lazy irq disabling works is: > > > > 1) Interrupt is marked disabled in software, but the hardware is not masked > > > > 2) If the interrupt fires befor the interrupt is reenabled, then it's > > masked at the hardware level in the low level interrupt flow handler. > > > Would you mind explaining what is the intention behind? > Because it does not seem obvious why there isn't a direct map between > "disable_irq*()" and "mask_irq()" Two reasons for this: 1) If you mask edge type interrupts then you might race with an incoming interrupt which then gets lost and eventually you won't get another interrupt from that device. Even if there is no race, then on many irq chips edge type interrupts are not latched when the interrupt line is masked. That also can result in a stale interrupt line. With the lazy disabling we mask only if an interrupt fires while it's disabled in software. We note that it is pending and resend it when the interrupt gets reenabled. 2) Accessing irq chip hardware can be slow and we have situations where interrupts are disabled/enabled fast. So it's an optimization to avoid the hardware access, which is sensible as we do not expect an interrupt to fire in most cases. If it fires then we mask it when the interrupt handler sees the disabled flag. That should really work on any hardware and the IRQ_DISABLE_UNLAZY flag is just there to deal with devices which are known to keep the (level based) irq line active. In that case we know that we always take an interrupt to mask it right away, so we can avoid the overhead. Though you should not set that flag on edge type interrupts, unless your hardware guarantees to avoid the issues described in #1. Hope that helps. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Disabling an interrupt in the handler locks the system up @ 2016-10-25 9:20 ` Thomas Gleixner 0 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2016-10-25 9:20 UTC (permalink / raw) To: linux-arm-kernel On Tue, 25 Oct 2016, Sebastian Frias wrote: > On 10/24/2016 06:55 PM, Thomas Gleixner wrote: > > On Mon, 24 Oct 2016, Mason wrote: > >> > >> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device > >> makes the system lock-up disappear. > > > > The way how lazy irq disabling works is: > > > > 1) Interrupt is marked disabled in software, but the hardware is not masked > > > > 2) If the interrupt fires befor the interrupt is reenabled, then it's > > masked at the hardware level in the low level interrupt flow handler. > > > Would you mind explaining what is the intention behind? > Because it does not seem obvious why there isn't a direct map between > "disable_irq*()" and "mask_irq()" Two reasons for this: 1) If you mask edge type interrupts then you might race with an incoming interrupt which then gets lost and eventually you won't get another interrupt from that device. Even if there is no race, then on many irq chips edge type interrupts are not latched when the interrupt line is masked. That also can result in a stale interrupt line. With the lazy disabling we mask only if an interrupt fires while it's disabled in software. We note that it is pending and resend it when the interrupt gets reenabled. 2) Accessing irq chip hardware can be slow and we have situations where interrupts are disabled/enabled fast. So it's an optimization to avoid the hardware access, which is sensible as we do not expect an interrupt to fire in most cases. If it fires then we mask it when the interrupt handler sees the disabled flag. That should really work on any hardware and the IRQ_DISABLE_UNLAZY flag is just there to deal with devices which are known to keep the (level based) irq line active. In that case we know that we always take an interrupt to mask it right away, so we can avoid the overhead. Though you should not set that flag on edge type interrupts, unless your hardware guarantees to avoid the issues described in #1. Hope that helps. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2016-10-25 13:59 UTC | newest] Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-21 16:37 Disabling an interrupt in the handler locks the system up Mason 2016-10-21 16:37 ` Mason 2016-10-21 17:46 ` Marc Zyngier 2016-10-21 17:46 ` Marc Zyngier 2016-10-21 18:39 ` Mason 2016-10-21 18:39 ` Mason 2016-10-21 19:14 ` Marc Zyngier 2016-10-21 19:14 ` Marc Zyngier 2016-10-21 19:47 ` Mason 2016-10-21 19:47 ` Mason 2016-10-21 19:49 ` Thomas Gleixner 2016-10-21 19:49 ` Thomas Gleixner 2016-10-21 20:27 ` Mason 2016-10-21 20:27 ` Mason 2016-10-22 11:37 ` Marc Zyngier 2016-10-22 11:37 ` Marc Zyngier 2016-10-22 23:10 ` Mason 2016-10-22 23:10 ` Mason 2016-10-24 8:17 ` Marc Zyngier 2016-10-24 8:17 ` Marc Zyngier 2016-10-24 16:12 ` Mason 2016-10-24 16:12 ` Mason 2016-10-24 16:55 ` Thomas Gleixner 2016-10-24 16:55 ` Thomas Gleixner 2016-10-25 8:29 ` Sebastian Frias 2016-10-25 8:29 ` Sebastian Frias 2016-10-25 8:36 ` Mason 2016-10-25 8:36 ` Mason 2016-10-25 10:45 ` Marc Zyngier 2016-10-25 10:45 ` Marc Zyngier 2016-10-25 13:56 ` Mason 2016-10-25 13:56 ` Mason 2016-10-25 13:56 ` Thomas Gleixner 2016-10-25 13:56 ` Thomas Gleixner 2016-10-25 9:20 ` Thomas Gleixner 2016-10-25 9:20 ` Thomas Gleixner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.