Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756126AbbDUR4f (ORCPT ); Tue, 21 Apr 2015 13:56:35 -0400 Received: from mail-oi0-f45.google.com ([209.85.218.45]:32971 "EHLO mail-oi0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753058AbbDUR4b (ORCPT ); Tue, 21 Apr 2015 13:56:31 -0400 MIME-Version: 1.0 In-Reply-To: <20150421150115.14288.88519.sendpatchset@little-apple> References: <20150421150115.14288.88519.sendpatchset@little-apple> Date: Tue, 21 Apr 2015 19:56:30 +0200 X-Google-Sender-Auth: 6TuNwT8U40ppuLNVUtPXC4Txr1I Message-ID: Subject: Re: [PATCH/RFC 00/03] irqchip: renesas-irqc: Fine grained Runtime PM support From: Geert Uytterhoeven To: Magnus Damm Cc: Linux-sh list , Jason Cooper , Geert Uytterhoeven , Linux PM list , "linux-kernel@vger.kernel.org" , Simon Horman , Thomas Gleixner Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7959 Lines: 184 Hi Magnus, On Tue, Apr 21, 2015 at 5:01 PM, Magnus Damm wrote: > irqchip: renesas-irqc: Fine grained Runtime PM support > > [PATCH/RFC 01/03] irqchip: renesas-irqc: Add irq_enable() and irq_disable() > [PATCH/RFC 02/03] irqchip: renesas-irqc: Add fine grained Runtime PM code > [PATCH/RFC 03/03] irqchip: renesas-irqc: Rely on Runtime PM for wakeup Thanks for your patches! > These patches attempt to convert the IRQC driver from using a mix of clock > framework and Runtime PM into only using Runtime PM and doing that in a > more fine grained way than before. With these patches in place, if there > is no interrupt used then the clock and/or power domain will not be used. > > Basic operation is that With these patches applied ->irq_enable() will > perform Runtime PM 'get sync' and ->irq_disable() simply performs > Runtime PM 'put'. The trigger mode callback is assumed to happen at any > time so there is a get/put wrapper there. > > Unless I'm misunderstanding the IRQ core code this means that the IRQC > struct device will be in Runtime PM 'get sync' state after someone has > started using an interrupt. I'm afraid you can't call pm_runtime_get_sync() from these methods, as they may be called from interrupt context. BTW, I ran into similar issues with rcar-gpio, when trying to improve its Runtime PM handling (still have to finish my WIP). On r8a73a4/ape6evm, I now get during boot: -----8<----- ================================= [ INFO: inconsistent lock state ] 4.0.0-ape6evm-10563-g8b333096057b3c10 #213 Not tainted --------------------------------- inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. swapper/0/0 [HC1[1]:SC0[0]:HE0:SE1] takes: (&irq_desc_lock_class){?.+...}, at: [] handle_fasteoi_irq+0x18/0x190 {HARDIRQ-ON-W} state was registered at: [] _raw_spin_unlock_irq+0x24/0x2c [] __rpm_callback+0x50/0x60 [] rpm_callback+0x20/0x80 [] rpm_resume+0x3a4/0x5ec [] __pm_runtime_resume+0x4c/0x64 [] irqc_irq_set_type+0x34/0x9c [] __irq_set_trigger+0x54/0x11c [] irq_set_irq_type+0x34/0x5c [] irq_create_of_mapping+0x114/0x168 [] irq_of_parse_and_map+0x24/0x2c [] of_irq_to_resource+0x18/0xb8 [] of_irq_to_resource_table+0x3c/0x54 [] of_device_alloc+0x104/0x170 [] of_platform_device_create_pdata+0x48/0xac [] of_platform_bus_create+0x94/0x130 [] of_platform_populate+0x180/0x1c4 [] simple_pm_bus_probe+0x30/0x38 [] platform_drv_probe+0x44/0xa4 [] driver_probe_device+0x178/0x2bc [] __driver_attach+0x94/0x98 [] bus_for_each_dev+0x6c/0xa0 [] bus_add_driver+0x140/0x1e8 [] driver_register+0x78/0xf8 [] do_one_initcall+0x118/0x1c8 [] kernel_init_freeable+0x144/0x1e4 [] kernel_init+0x8/0xe8 [] ret_from_fork+0x14/0x24 irq event stamp: 2304 hardirqs last enabled at (2301): [] arch_cpu_idle+0x20/0x3c hardirqs last disabled at (2302): [] __irq_svc+0x34/0x5c softirqs last enabled at (2304): [] irq_enter+0x68/0x84 softirqs last disabled at (2303): [] irq_enter+0x54/0x84 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&irq_desc_lock_class); lock(&irq_desc_lock_class); *** DEADLOCK *** no locks held by swapper/0/0. stack backtrace: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.0.0-ape6evm-10563-g8b333096057b3c10 #213 Hardware name: Generic R8A73A4 (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x88/0x98) [] (dump_stack) from [] (print_usage_bug+0x22c/0x2d8) [] (print_usage_bug) from [] (mark_lock+0x204/0x768) [] (mark_lock) from [] (__lock_acquire+0xa1c/0x215c) [] (__lock_acquire) from [] (lock_acquire+0xac/0x12c) [] (lock_acquire) from [] (_raw_spin_lock+0x30/0x40) [] (_raw_spin_lock) from [] (handle_fasteoi_irq+0x18/0x190) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x2c/0x3c) [] (generic_handle_irq) from [] (__handle_domain_irq+0x5c/0xb4) [] (__handle_domain_irq) from [] (gic_handle_irq+0x24/0x5c) [] (gic_handle_irq) from [] (__irq_svc+0x44/0x5c) Exception stack(0xc0583f58 to 0xc0583fa0) 3f40: 00000001 00000001 3f60: 00000000 c05887a8 c0582000 c05854fc c0585400 c05d6624 c0585498 c057d3c4 3f80: c041f220 00000000 01000000 c0583fa0 c0070a74 c0010c3c 20000113 ffffffff [] (__irq_svc) from [] (arch_cpu_idle+0x24/0x3c) [] (arch_cpu_idle) from [] (cpu_startup_entry+0x170/0x280) [] (cpu_startup_entry) from [] (start_kernel+0x358/0x364) [] (start_kernel) from [<4000807c>] (0x4000807c) ----->8----- > As for wakeup support, based on IRQ_WAKEUP_STATE being toggled in > irq_set_irq_wake() together with the irqd_is_wakeup_set() usage in > suspend_device_irqs() it looks like interrupts used for wakeup will > stay enabled once we use Runtime PM in ->irq_enable() and ->irq_disable() > and because of that the clock operations and custom ->irq_set_wake() > should not be necessary. > > I have boot tested this with some simple PHY link state change IRQs > on a Koelsch board, but I have not tried Suspend-to-RAM yet with wakeup > support. It would be useful to test this with Suspend-to-RAM on APE6EVM. Unfortunately pm_runtime_get_sync() doesn't protect against s2ram. pm_clk_suspend() will be called anyway, disabling the clock if it wasn't enabled explicitly. That's why I incremented the clock's enable count manually when wake-up is enabled. During the suspend phase, it does: -----8<----- sh_mobile_sdhi ee120000.sd: pm_clk_resume() sh_mobile_sdhi ee120000.sd: pm_clk_suspend() sh_mobile_sdhi ee120000.sd: pm_clk_resume() PM: Syncing filesystems ... done. PM: Preparing system for mem sleep Freezing user space processes ... (elapsed 0.001 seconds) done. Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. PM: Entering mem sleep i2c-sh_mobile e60b0000.i2c: pm_clk_resume() MSTP iic5 ON sh-dma-engine e6700020.dma-controller: pm_clk_resume() MSTP dmac ON sh_mobile_sdhi ee120000.sd: pm_clk_suspend() sh_mobile_sdhi ee100000.sd: pm_clk_suspend() PM: suspend of devices complete after 24.016 msecs PM: late suspend of devices complete after 7.605 msecs sh-dma-engine e6700020.dma-controller: pm_clk_suspend() MSTP dmac OFF simple-pm-bus fec10000.bus: pm_clk_suspend() sh_mmcif ee200000.mmc: pm_clk_suspend() MSTP mmcif0 OFF sh_mobile_sdhi ee120000.sd: pm_clk_suspend() sh_mobile_sdhi ee100000.sd: pm_clk_suspend() sh-sci e6c40000.serial: pm_clk_suspend() rcar_thermal e61f0000.thermal: pm_clk_suspend() MSTP thermal OFF sh-pfc e6050000.pfc: pm_clk_suspend() renesas_irqc e61c0200.interrupt-controller: pm_clk_suspend() renesas_irqc e61c0000.interrupt-controller: pm_clk_suspend() MSTP irqc OFF ----->8----- Woops, wake-up by IRQC won't work anymore as the IRQC clock is now disabled. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/