irqchip: renesas-irqc: Fine grained Runtime PM support

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

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.

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.

Thanks to Geert for hooking up APE6EVM interrupt wake support!

---
Written against next-20150421

 drivers/irqchip/irq-renesas-irqc.c |   61 +++++++++++++++---------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

From: Magnus Damm <[email protected]>

Hook up ->irq_enable() and ->irq_disable() together with ->irq_mask() and ->irq_unmask(). This should not adjust any code behaviour at all.

---

 drivers/irqchip/irq-renesas-irqc.c |   24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

@@ -76,24 +76,34 @@ static void irqc_dbg(struct irqc_irq *i,
 		str, i->requested_irq, i->hw_irq, i->domain_irq);
 }
 
-static void irqc_irq_enable(struct irq_data *d)
+static void irqc_irq_unmask(struct irq_data *d)
 {
 	struct irqc_priv *p = irq_data_get_irq_chip_data(d);
 	int hw_irq = irqd_to_hwirq(d);
 
-	irqc_dbg(&p->irq[hw_irq], "enable");
+	irqc_dbg(&p->irq[hw_irq], "unmask");
 	iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_SET);
 }
 
-static void irqc_irq_disable(struct irq_data *d)
+static void irqc_irq_mask(struct irq_data *d)
 {
 	struct irqc_priv *p = irq_data_get_irq_chip_data(d);
 	int hw_irq = irqd_to_hwirq(d);
 
-	irqc_dbg(&p->irq[hw_irq], "disable");
+	irqc_dbg(&p->irq[hw_irq], "mask");
 	iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_STS);
 }
 
+static void irqc_irq_enable(struct irq_data *d)
+{
+	irqc_irq_unmask(d);
+}
+
+static void irqc_irq_disable(struct irq_data *d)
+{
+	irqc_irq_mask(d);
+}
+
 static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {
 	[IRQ_TYPE_LEVEL_LOW]	= 0x01,
 	[IRQ_TYPE_LEVEL_HIGH]	= 0x02,

@@ -244,8 +254,10 @@ static int irqc_probe(struct platform_de
 
 	irq_chip = &p->irq_chip;
 	irq_chip->name = name;
-	irq_chip->irq_mask = irqc_irq_disable;
-	irq_chip->irq_unmask = irqc_irq_enable;
+	irq_chip->irq_enable = irqc_irq_enable;
+	irq_chip->irq_disable = irqc_irq_disable;
+	irq_chip->irq_mask = irqc_irq_mask;
+	irq_chip->irq_unmask = irqc_irq_unmask;
 	irq_chip->irq_set_type = irqc_irq_set_type;
 	irq_chip->irq_set_wake = irqc_irq_set_wake;
 	irq_chip->flags	= IRQCHIP_MASK_ON_SUSPEND;

From: Magnus Damm <[email protected]>

Convert Runtime PM support from coarse grained "always enabled" into more fine grained per-interrupt optional enablement.

---

 drivers/irqchip/irq-renesas-irqc.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

@@ -96,12 +96,18 @@ static void irqc_irq_mask(struct irq_dat
 
 static void irqc_irq_enable(struct irq_data *d)
 {
+	struct irqc_priv *p = irq_data_get_irq_chip_data(d);
+
+	pm_runtime_get_sync(&p->pdev->dev);
 	irqc_irq_unmask(d);
 }
 
 static void irqc_irq_disable(struct irq_data *d)
 {
+	struct irqc_priv *p = irq_data_get_irq_chip_data(d);
+
 	irqc_irq_mask(d);
+	pm_runtime_put(&p->pdev->dev);
 }
 
 static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {

@@ -124,10 +130,12 @@ static int irqc_irq_set_type(struct irq_
 	if (!value)
 		return -EINVAL;
 
+	pm_runtime_get_sync(&p->pdev->dev);
 	tmp = ioread32(p->iomem + IRQC_CONFIG(hw_irq));
 	tmp &= ~0x3f;
 	tmp |= value;
 	iowrite32(tmp, p->iomem + IRQC_CONFIG(hw_irq));
+	pm_runtime_put(&p->pdev->dev);
 	return 0;
 }
 

@@ -215,7 +223,6 @@ static int irqc_probe(struct platform_de
 	}
 
 	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 
 	/* get hold of manadatory IOMEM */
 	io = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -300,7 +307,6 @@ err3:
 err2:
 	iounmap(p->iomem);
 err1:
-	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	kfree(p);
 err0:

@@ -317,7 +323,6 @@ static int irqc_remove(struct platform_d
 
 	irq_domain_remove(p->irq_domain);
 	iounmap(p->iomem);
-	pm_runtime_put(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	kfree(p);
 	return 0;

From: Magnus Damm <[email protected]>

Get rid of the clock control code and rely on Runtime PM for wakeup handling. This more or less reverts the earlier commit "irqchip: renesas-irqc: Add wake-up support" however to work ->irq_enable() and ->irq_disable() need to manage Runtime PM state.

---

Needs testing before applying.

 drivers/irqchip/irq-renesas-irqc.c |   26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

@@ -17,7 +17,6 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include <linux/clk.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>

@@ -67,7 +66,6 @@ struct irqc_priv {
 	struct platform_device *pdev;
 	struct irq_chip irq_chip;
 	struct irq_domain *irq_domain;
-	struct clk *clk;
 };
 
 static void irqc_dbg(struct irqc_irq *i, char *str)

@@ -139,21 +137,6 @@ static int irqc_irq_set_type(struct irq_
 	return 0;
 }
 
-static int irqc_irq_set_wake(struct irq_data *d, unsigned int on)
-{
-	struct irqc_priv *p = irq_data_get_irq_chip_data(d);
-
-	if (!p->clk)
-		return 0;
-
-	if (on)
-		clk_enable(p->clk);
-	else
-		clk_disable(p->clk);
-
-	return 0;
-}
-
 static irqreturn_t irqc_irq_handler(int irq, void *dev_id)
 {
 	struct irqc_irq *i = dev_id;

@@ -216,12 +199,6 @@ static int irqc_probe(struct platform_de
 	p->pdev = pdev;
 	platform_set_drvdata(pdev, p);
 
-	p->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(p->clk)) {
-		dev_warn(&pdev->dev, "unable to get clock\n");
-		p->clk = NULL;
-	}
-
 	pm_runtime_enable(&pdev->dev);
 
 	/* get hold of manadatory IOMEM */

@@ -266,8 +243,7 @@ static int irqc_probe(struct platform_de
 	irq_chip->irq_mask = irqc_irq_mask;
 	irq_chip->irq_unmask = irqc_irq_unmask;
 	irq_chip->irq_set_type = irqc_irq_set_type;
-	irq_chip->irq_set_wake = irqc_irq_set_wake;
-	irq_chip->flags	= IRQCHIP_MASK_ON_SUSPEND;
+	irq_chip->flags	= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND;
 
 	p->irq_domain = irq_domain_add_simple(pdev->dev.of_node,
 					      p->number_of_irqs,

Hi Magnus,

Thanks for your patches!

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: [<c0088a34>] handle_fasteoi_irq+0x18/0x190
{HARDIRQ-ON-W} state was registered at:

  [<c041cf80>] _raw_spin_unlock_irq+0x24/0x2c
  [<c028a748>] __rpm_callback+0x50/0x60
  [<c028a778>] rpm_callback+0x20/0x80
  [<c028af24>] rpm_resume+0x3a4/0x5ec
  [<c028bc10>] __pm_runtime_resume+0x4c/0x64
  [<c024241c>] irqc_irq_set_type+0x34/0x9c
  [<c0086dc8>] __irq_set_trigger+0x54/0x11c
  [<c00883fc>] irq_set_irq_type+0x34/0x5c
  [<c008ad6c>] irq_create_of_mapping+0x114/0x168
  [<c02d6f5c>] irq_of_parse_and_map+0x24/0x2c
  [<c02d6f7c>] of_irq_to_resource+0x18/0xb8
  [<c02d7120>] of_irq_to_resource_table+0x3c/0x54
  [<c02d4c74>] of_device_alloc+0x104/0x170
  [<c02d4d28>] of_platform_device_create_pdata+0x48/0xac
  [<c02d4e20>] of_platform_bus_create+0x94/0x130
  [<c02d520c>] of_platform_populate+0x180/0x1c4
  [<c0242778>] simple_pm_bus_probe+0x30/0x38
  [<c028431c>] platform_drv_probe+0x44/0xa4
  [<c0282d10>] driver_probe_device+0x178/0x2bc
  [<c0282f2c>] __driver_attach+0x94/0x98
  [<c028143c>] bus_for_each_dev+0x6c/0xa0
  [<c0281dcc>] bus_add_driver+0x140/0x1e8
  [<c02837dc>] driver_register+0x78/0xf8
  [<c0540d40>] do_one_initcall+0x118/0x1c8
  [<c0540f34>] kernel_init_freeable+0x144/0x1e4
  [<c04148cc>] kernel_init+0x8/0xe8
  [<c0010210>] ret_from_fork+0x14/0x24
irq event stamp: 2304
hardirqs last  enabled at (2301): [<c0010c38>] arch_cpu_idle+0x20/0x3c
hardirqs last disabled at (2302): [<c0014734>] __irq_svc+0x34/0x5c
softirqs last  enabled at (2304): [<c002fa54>] irq_enter+0x68/0x84
softirqs last disabled at (2303): [<c002fa40>] irq_enter+0x54/0x84

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&irq_desc_lock_class);
  <Interrupt>
    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)
[<c0017280>] (unwind_backtrace) from [<c0013b90>] (show_stack+0x10/0x14)
[<c0013b90>] (show_stack) from [<c0417450>] (dump_stack+0x88/0x98)
[<c0417450>] (dump_stack) from [<c007007c>] (print_usage_bug+0x22c/0x2d8)
[<c007007c>] (print_usage_bug) from [<c007032c>] (mark_lock+0x204/0x768)
[<c007032c>] (mark_lock) from [<c0072b58>] (__lock_acquire+0xa1c/0x215c)
[<c0072b58>] (__lock_acquire) from [<c0074bf0>] (lock_acquire+0xac/0x12c)
[<c0074bf0>] (lock_acquire) from [<c041cdf0>] (_raw_spin_lock+0x30/0x40)
[<c041cdf0>] (_raw_spin_lock) from [<c0088a34>] (handle_fasteoi_irq+0x18/0x190)

[<c0088a34>] (handle_fasteoi_irq) from [<c0085394>] (generic_handle_irq+0x2c/0x3c)
[<c0085394>] (generic_handle_irq) from [<c0085400>] (__handle_domain_irq+0x5c/0xb4)
[<c0085400>] (__handle_domain_irq) from [<c000a3e4>] (gic_handle_irq+0x24/0x5c) [<c000a3e4>] (gic_handle_irq) from [<c0014744>] (__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 [<c0014744>] (__irq_svc) from [<c0010c3c>] (arch_cpu_idle+0x24/0x3c) [<c0010c3c>] (arch_cpu_idle) from [<c0068fb8>] (cpu_startup_entry+0x170/0x280) [<c0068fb8>] (cpu_startup_entry) from [<c0540c1c>] (start_kernel+0x358/0x364) [<c0540c1c>] (start_kernel) from [<4000807c>] (0x4000807c) ----->8-----

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 -- [email protected]

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

Hi Geert,

Thanks!

Ouch. I know the clock framework has prepare/enable separated with context, but with the irqchip callbacks I suppose no such separation is made...? Maybe it makes more sense to do power management from the online/offline hooks?

Yeah, the IRQC and GPIO interrupt bits should be pretty much the same. I considered IRQC to be a simpler test case, but I guess it is may be more complicated due to lack of wakeup sources.

Thanks, this looks troublesome. =) I can see it is on APE6EVM, but which patches are applied?

Is it possible to reproduce on R-Car Gen2 somehow?

Can you please clarify what you mean about "enabled explicitly"? Just enabling the clock may solve the issue for this particular driver, but as a general approach it seems to me that we probably need to control both power domain and clock. So PM domain handling must be necessary somehow I think.

Also, I honestly don't know what kind of special casing we need to do with the interrupt controller during Suspend-to-RAM. Ideally it would be nice if we could turn off all clocks / power domains that are not necessary for wakeup, but this seems quite similar to regular runtime operation IMO.

So the question is how to get runtime operation to work well...

So if there is any wakeup source needed shouldn't the Runtime PM device remain in "get" state?

Thanks,

/ magnus

Hi Magnus,

It's not the clock operations, but the pm_runtime operations that cannot be called from interrupt context.

Which online/offline hooks do you mean?

[snip]

I can reproduce it on renesas-drivers-2015-04-20-v4.0 with just your 3 patches. I was using my own .config.

With shmobile_defconfig + CONFIG_PROVE_LOCKING=y I see a different one:

[ INFO: inconsistent lock state ]
4.0.0-10356-g7156f022c08bb358 #231 Not tainted
---------------------------------
inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
swapper/0/1 [HC0[0]:SC0[0]:HE1:SE1] takes:
(&irq_desc_lock_class){?.-...}, at: [<c006ef0c>] __irq_get_desc_lock+0x78/0x94
{IN-HARDIRQ-W} state was registered at:

  [<c0066bd4>] lock_acquire+0x74/0x94
  [<c04fb728>] _raw_spin_lock+0x34/0x44
  [<c0072330>] handle_fasteoi_irq+0x20/0x128
  [<c006ed1c>] generic_handle_irq+0x28/0x38
  [<c006edc0>] __handle_domain_irq+0x94/0xbc
  [<c000a408>] gic_handle_irq+0x40/0x64
  [<c00140c4>] __irq_svc+0x44/0x5c
  [<c04fb8a8>] _raw_spin_unlock_irqrestore+0x48/0x4c
  [<c005d35c>] prepare_to_wait_event+0xd8/0xe8
  [<c03434b4>] sh_mobile_i2c_xfer+0x338/0x4f4
  [<c033f22c>] __i2c_transfer+0x1d0/0x220
  [<c033f2f8>] i2c_transfer+0x7c/0xa0
  [<c02a0c80>] regmap_i2c_read+0x50/0x6c
  [<c029d8a8>] _regmap_raw_read+0x9c/0xd4
  [<c029d90c>] _regmap_bus_read+0x2c/0x58
  [<c029c738>] _regmap_read+0xa4/0xe0
  [<c029e604>] regmap_read+0x48/0x68
  [<c022dcbc>] max8973_dcdc_get_voltage_sel+0x28/0x5c
  [<c022728c>] regulator_attr_is_visible+0x90/0x238
  [<c013bdf0>] internal_create_group+0x13c/0x254
  [<c013bf20>] sysfs_create_group+0x18/0x1c
  [<c013c014>] sysfs_create_groups+0x3c/0x68
  [<c02885d0>] device_add+0x224/0x514
  [<c02888dc>] device_register+0x1c/0x20
  [<c022a9b4>] regulator_register+0x290/0xd0c
  [<c022c268>] devm_regulator_register+0x3c/0x78
  [<c022e200>] max8973_probe+0x3b8/0x430
  [<c033e1c4>] i2c_device_probe+0xf8/0x120
  [<c028a964>] driver_probe_device+0x100/0x268
  [<c028aafc>] __device_attach+0x30/0x4c
  [<c0289614>] bus_for_each_drv+0x8c/0x9c
  [<c028ac24>] device_attach+0x70/0x94
  [<c02897a8>] bus_probe_device+0x30/0xa4
  [<c02887a8>] device_add+0x3fc/0x514
  [<c02888dc>] device_register+0x1c/0x20
  [<c033e4d8>] i2c_new_device+0x12c/0x1a0
  [<c033e984>] i2c_register_adapter+0x2f0/0x408
  [<c033eccc>] i2c_add_adapter+0x90/0xa8
  [<c033ed00>] i2c_add_numbered_adapter+0x1c/0x28
  [<c0342ce4>] sh_mobile_i2c_probe+0x3c0/0x454
  [<c028bdc4>] platform_drv_probe+0x50/0xa0
  [<c028a964>] driver_probe_device+0x100/0x268
  [<c028ab90>] __driver_attach+0x78/0x9c
  [<c0289218>] bus_for_each_dev+0x74/0x98
  [<c028ac68>] driver_attach+0x20/0x28
  [<c02899f0>] bus_add_driver+0xdc/0x1c4
  [<c028b378>] driver_register+0xa4/0xe8
  [<c028c798>] __platform_driver_register+0x50/0x64
  [<c06a54c8>] sh_mobile_i2c_adap_init+0x18/0x20
  [<c0687dfc>] do_one_initcall+0x108/0x1bc
  [<c0687fcc>] kernel_init_freeable+0x11c/0x1e4
  [<c04f30d4>] kernel_init+0x10/0xec
  [<c000fcb0>] ret_from_fork+0x14/0x24
irq event stamp: 162248
hardirqs last  enabled at (162247): [<c04f9b04>]
__mutex_unlock_slowpath+0x174/0x198
hardirqs last disabled at (162248): [<c04fb75c>]
_raw_spin_lock_irqsave+0x24/0x58
softirqs last  enabled at (160452): [<c002bda0>] __do_softirq+0x204/0x29c
softirqs last disabled at (160443): [<c002c134>] irq_exit+0x8c/0x118

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&irq_desc_lock_class);
  <Interrupt>
    lock(&irq_desc_lock_class);

*** DEADLOCK ***

3 locks held by swapper/0/1:

 #0:  (&dev->mutex){......}, at: [<c028ab54>] __driver_attach+0x3c/0x9c
 #1:  (&dev->mutex){......}, at: [<c028ab78>] __driver_attach+0x60/0x9c
 #2:  (&irq_desc_lock_class){?.-...}, at: [<c006ef0c>]
__irq_get_desc_lock+0x78/0x94

stack backtrace:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.0.0-10356-g7156f022c08bb358 #231
Hardware name: Generic R8A73A4 (Flattened Device Tree)
Backtrace:
[<c0013368>] (dump_backtrace) from [<c0013510>] (show_stack+0x18/0x1c)
 r6:c0ede83c r5:ee8433c0 r4:00000000 r3:00200140
[<c00134f8>] (show_stack) from [<c04f605c>] (dump_stack+0x78/0x94)
[<c04f5fe4>] (dump_stack) from [<c00629bc>] (print_usage_bug+0x254/0x2c4)
 r4:c08412f8 r3:00000002
[<c0062768>] (print_usage_bug) from [<c0062dd8>] (mark_lock+0x3ac/0x674)
 r9:c0061f18 r8:ee8433c0 r7:00001015 r6:ee843880 r5:00000002 r4:00000000
[<c0062a2c>] (mark_lock) from [<c0063108>] (mark_held_locks+0x68/0x84)
 r10:c06f6568 r9:00000018 r8:00000002 r7:00000003 r6:ee843880 r5:ee8433c0
 r4:00000002 r3:0000000c

[<c00630a0>] (mark_held_locks) from [<c0063280>] (trace_hardirqs_on_caller+0x15c/0x1dc)
r9:ee923ca0 r8:ee844000 r7:00000000 r6:c04fb8d8 r5:00000001 r4:ee8433c0
[<c0063124>] (trace_hardirqs_on_caller) from [<c0063314>] (trace_hardirqs_on+0x14/0x18)
r6:c0298cf8 r5:ee923c10 r4:ee923ca0 r3:ee8433c0
[<c0063300>] (trace_hardirqs_on) from [<c04fb8d8>] (_raw_spin_unlock_irq+0x2c/0x34)
[<c04fb8ac>] (_raw_spin_unlock_irq) from [<c0292368>] (__rpm_callback+0x34/0x64)
r4:ee923ca0 r3:00000000
[<c0292334>] (__rpm_callback) from [<c0292408>] (rpm_callback+0x70/0x88)
r6:c06f64d8 r5:ee923c10 r4:ee923c10 r3:00000000
[<c0292398>] (rpm_callback) from [<c0292b28>] (rpm_resume+0x350/0x3fc)
r5:00000000 r4:ee923c10
[<c02927d8>] (rpm_resume) from [<c0293390>] (__pm_runtime_resume+0x54/0x6c)
r10:eeb7b280 r9:00000078 r8:00000004 r7:600001d3 r6:00000004 r5:ee923ca0 r4:ee923c10
[<c029333c>] (__pm_runtime_resume) from [<c01f2d0c>] (irqc_irq_set_type+0x38/0xa4)
r7:ee923614 r6:00000002 r5:ee923400 r4:00000008
[<c01f2cd4>] (irqc_irq_set_type) from [<c00706c0>] (__irq_set_trigger+0x68/0x118)
r6:00000000 r5:ee933e00 r4:00000004 r3:c01f2cd4
[<c0070658>] (__irq_set_trigger) from [<c0071c74>] (irq_set_irq_type+0x40/0x60)
r9:00000001 r8:00000000 r7:0000001c r6:00000078 r5:ee933e00 r4:00000004
[<c0071c34>] (irq_set_irq_type) from [<c0074374>] (irq_create_of_mapping+0x144/0x15c)
r6:eefe0970 r5:00000004 r4:00000078
[<c0074230>] (irq_create_of_mapping) from [<c03a218c>] (irq_of_parse_and_map+0x2c/0x34)
r5:00000001 r4:eeb7b29c
[<c03a2160>] (irq_of_parse_and_map) from [<c03a21b4>] (of_irq_to_resource+0x20/0xb0)
[<c03a2194>] (of_irq_to_resource) from [<c03a2348>] (of_irq_to_resource_table+0x38/0x4c)
r8:eefe0970 r7:0000001c r6:eeb7b29c r5:00000001 r4:00000000
[<c03a2310>] (of_irq_to_resource_table) from [<c039fbf0>] (of_device_alloc+0xd8/0x130)
r8:0000001c r7:00000001 r6:eefe0970 r5:00000001 r4:eeb7a800 r3:00000000
[<c039fb18>] (of_device_alloc) from [<c039fc98>] (of_platform_device_create_pdata+0x50/0xb4)
r10:00000000 r9:00000001 r8:00000000 r7:ee944410 r6:00000000 r5:eefe09c0 r4:eefe0970
[<c039fc48>] (of_platform_device_create_pdata) from [<c039fde8>] (of_platform_bus_create+0xec/0x17c)
r8:00000000 r7:00000000 r6:ee944410 r5:00000000 r4:eefe0970 r3:ee944410
[<c039fcfc>] (of_platform_bus_create) from [<c039ffd0>] (of_platform_populate+0x70/0xac)
r10:00000001 r9:ee944410 r8:00000000 r7:00000000 r6:eefe0970 r5:eefe0704 r4:ee944410
[<c039ff60>] (of_platform_populate) from [<c01f3f48>] (simple_pm_bus_probe+0x38/0x40)
r10:00000000 r9:c06eb83c r8:00000000 r7:c0ee3100 r6:c06eb83c r5:eefe0704 r4:ee944410
[<c01f3f10>] (simple_pm_bus_probe) from [<c028bdc4>] (platform_drv_probe+0x50/0xa0)
r5:ee944410 r4:00000000
[<c028bd74>] (platform_drv_probe) from [<c028a964>] (driver_probe_device+0x100/0x268)
r6:c0ee30ec r5:00000000 r4:ee944410 r3:c028bd74
[<c028a864>] (driver_probe_device) from [<c028ab90>] (__driver_attach+0x78/0x9c)
r9:c0712000 r8:c06ba84c r7:c06f6700 r6:c06eb83c r5:ee944444 r4:ee944410
[<c028ab18>] (__driver_attach) from [<c0289218>] (bus_for_each_dev+0x74/0x98)
r6:c028ab18 r5:c06eb83c r4:00000000 r3:00000001
[<c02891a4>] (bus_for_each_dev) from [<c028ac68>] (driver_attach+0x20/0x28)
r6:ee82f140 r5:00000000 r4:c06eb83c
[<c028ac48>] (driver_attach) from [<c02899f0>] (bus_add_driver+0xdc/0x1c4) [<c0289914>] (bus_add_driver) from [<c028b378>] (driver_register+0xa4/0xe8)
r7:c06ba84c r6:00000000 r5:c069fed0 r4:c06eb83c
[<c028b2d4>] (driver_register) from [<c028c798>] (__platform_driver_register+0x50/0x64)
r5:c069fed0 r4:eeb7b580
[<c028c748>] (__platform_driver_register) from [<c069fee8>] (simple_pm_bus_driver_init+0x18/0x20)
[<c069fed0>] (simple_pm_bus_driver_init) from [<c0687dfc>] (do_one_initcall+0x108/0x1bc)
[<c0687cf4>] (do_one_initcall) from [<c0687fcc>] (kernel_init_freeable+0x11c/0x1e4)
r9:c0712000 r8:c0712000 r7:c06c3d40 r6:c06bb078 r5:000000b3 r4:00000006
[<c0687eb0>] (kernel_init_freeable) from [<c04f30d4>] (kernel_init+0x10/0xec)
r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c04f30c4 r4:00000000
[<c04f30c4>] (kernel_init) from [<c000fcb0>] (ret_from_fork+0x14/0x24)
r4:00000000 r3:00000000

Seems I had disabled CONFIG_PROVE_LOCKING in my koelsch .config, so I didn't see it there.
But you're lucky ;-) With shmobile_defconfig + CONFIG_PROVE_LOCKING=y:

[ INFO: inconsistent lock state ]
4.0.0-10356-g7156f022c08bb358 #232 Not tainted
---------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
swapper/0/1 [HC1[1]:SC0[0]:HE0:SE1] takes:
(&irq_desc_lock_class){?.+...}, at: [<c0072330>] handle_fasteoi_irq+0x20/0x128
{HARDIRQ-ON-W} state was registered at:

  [<c0063280>] trace_hardirqs_on_caller+0x15c/0x1dc
  [<c0063314>] trace_hardirqs_on+0x14/0x18
  [<c04fb8d8>] _raw_spin_unlock_irq+0x2c/0x34
  [<c0292368>] __rpm_callback+0x34/0x64
  [<c0292408>] rpm_callback+0x70/0x88
  [<c0292b28>] rpm_resume+0x350/0x3fc
  [<c0293390>] __pm_runtime_resume+0x54/0x6c
  [<c01f2d0c>] irqc_irq_set_type+0x38/0xa4
  [<c00706c0>] __irq_set_trigger+0x68/0x118
  [<c0071c74>] irq_set_irq_type+0x40/0x60
  [<c0074374>] irq_create_of_mapping+0x144/0x15c
  [<c03a2280>] of_irq_get+0x3c/0x44
  [<c033e104>] i2c_device_probe+0x38/0x120
  [<c028a964>] driver_probe_device+0x100/0x268
  [<c028ab90>] __driver_attach+0x78/0x9c
  [<c0289218>] bus_for_each_dev+0x74/0x98
  [<c028ac68>] driver_attach+0x20/0x28
  [<c02899f0>] bus_add_driver+0xdc/0x1c4
  [<c028b378>] driver_register+0xa4/0xe8
  [<c033ef78>] i2c_register_driver+0x48/0x7c
  [<c06a0e50>] da9210_regulator_driver_init+0x18/0x20
  [<c0687dfc>] do_one_initcall+0x108/0x1bc
  [<c0687fcc>] kernel_init_freeable+0x11c/0x1e4
  [<c04f30d4>] kernel_init+0x10/0xec
  [<c000fcb0>] ret_from_fork+0x14/0x24
irq event stamp: 223608
hardirqs last  enabled at (223607): [<c04fb8a4>]
_raw_spin_unlock_irqrestore+0x44/0x4c
hardirqs last disabled at (223608): [<c00140b4>] __irq_svc+0x34/0x5c
softirqs last  enabled at (214668): [<c002bda0>] __do_softirq+0x204/0x29c
softirqs last disabled at (214373): [<c002c134>] irq_exit+0x8c/0x118

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&irq_desc_lock_class);
  <Interrupt>
    lock(&irq_desc_lock_class);

*** DEADLOCK ***

3 locks held by swapper/0/1:

 #0:  (&dev->mutex){......}, at: [<c028ab54>] __driver_attach+0x3c/0x9c
 #1:  (&dev->mutex){......}, at: [<c028ab78>] __driver_attach+0x60/0x9c
 #2:  (&map->mutex){+.+...}, at: [<c029c304>] regmap_lock_mutex+0x14/0x18

stack backtrace:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.0.0-10356-g7156f022c08bb358 #232
Hardware name: Generic R8A7791 (Flattened Device Tree)
Backtrace:
[<c0013368>] (dump_backtrace) from [<c0013510>] (show_stack+0x18/0x1c)
 r6:c0ede83c r5:ee0533c0 r4:00000000 r3:00200140
[<c00134f8>] (show_stack) from [<c04f605c>] (dump_stack+0x78/0x94)
[<c04f5fe4>] (dump_stack) from [<c00629bc>] (print_usage_bug+0x254/0x2c4)
 r4:c08412f8 r3:00000002
[<c0062768>] (print_usage_bug) from [<c0062dd8>] (mark_lock+0x3ac/0x674)
 r9:c0061e08 r8:ee0533c0 r7:00001045 r6:ee053898 r5:00000000 r4:00000002
[<c0062a2c>] (mark_lock) from [<c00650cc>] (__lock_acquire+0x83c/0x1b64)
 r10:ee055c04 r9:c08412f8 r8:ee0533c0 r7:00000003 r6:00000001 r5:00000019
 r4:ee1afb70 r3:00000001
[<c0064890>] (__lock_acquire) from [<c0066bd4>] (lock_acquire+0x74/0x94)
 r10:ee055c04 r9:00000001 r8:ee006000 r7:00000067 r6:00000001 r5:60000193
 r4:00000000
[<c0066b60>] (lock_acquire) from [<c04fb728>] (_raw_spin_lock+0x34/0x44)
 r6:ee1afb60 r5:c06eb520 r4:ee1afb60
[<c04fb6f4>] (_raw_spin_lock) from [<c0072330>] (handle_fasteoi_irq+0x20/0x128)
 r4:ee1afb00

[<c0072310>] (handle_fasteoi_irq) from [<c006ed1c>] (generic_handle_irq+0x28/0x38)
r6:c06c6e38 r5:00000000 r4:00000067 r3:c0072310
[<c006ecf4>] (generic_handle_irq) from [<c006edc0>] (__handle_domain_irq+0x94/0xbc)
r4:00000000 r3:00000178
[<c006ed2c>] (__handle_domain_irq) from [<c000a408>] (gic_handle_irq+0x40/0x64)
r8:ed8fff74 r7:ee055ba4 r6:ee055b70 r5:c06cc784 r4:f0002000 r3:ee055b70
[<c000a3c8>] (gic_handle_irq) from [<c00140c4>] (__irq_svc+0x44/0x5c)

Exception stack(0xee055b70 to 0xee055bb8)

5b60: 00000001 00000110 00000000 ee0533c0
5b80: 60000113 ed8fff74 00000002 00000000 ed8fff74 00000001 ee055c04 ee055bcc 5ba0: ee055b60 ee055bb8 c0063108 c04fb8a8 20000113 ffffffff
r6:ffffffff r5:20000113 r4:c04fb8a8 r3:ee0533c0
[<c04fb860>] (_raw_spin_unlock_irqrestore) from [<c005d35c>] (prepare_to_wait_event+0xd8/0xe8)
r5:ed8fff74 r4:ee055c04
[<c005d284>] (prepare_to_wait_event) from [<c03434b4>] (sh_mobile_i2c_xfer+0x338/0x4f4)
r6:ee055c9c r5:000001f4 r4:ed8ffc10 r3:00000000
[<c034317c>] (sh_mobile_i2c_xfer) from [<c033f22c>] (__i2c_transfer+0x1d0/0x220)
r10:ffff8b09 r9:c06cc100 r8:00000001 r7:00000000 r6:c0ee5440 r5:ed8ffc18 r4:ee055c9c
[<c033f05c>] (__i2c_transfer) from [<c033f2f8>] (i2c_transfer+0x7c/0xa0)
r10:00000054 r9:ee3c9080 r8:ee3c9081 r7:ee055c9c r6:00000001 r5:ed8ffc28 r4:ed8ffc18 r3:00000000
[<c033f27c>] (i2c_transfer) from [<c033f360>] (i2c_master_send+0x44/0x54)
r7:ee3c9081 r6:00000001 r5:fffffdf4 r4:00000002
[<c033f31c>] (i2c_master_send) from [<c02a0d50>] (regmap_i2c_write+0x18/0x34)
r4:00000002
[<c02a0d38>] (regmap_i2c_write) from [<c029dd8c>] (_regmap_raw_write+0x454/0x534)
r4:ed934200 r3:c02a0d38
[<c029d938>] (_regmap_raw_write) from [<c029dee4>] (_regmap_bus_raw_write+0x78/0x90)
r10:00000000 r9:c06ee170 r8:00000000 r7:ed934200 r6:ffffffff r5:00000054 r4:ed934200
[<c029de6c>] (_regmap_bus_raw_write) from [<c029d5e4>] (_regmap_write+0x90/0x9c)
r6:ffffffff r5:00000054 r4:ed934200 r3:c029de6c
[<c029d554>] (_regmap_write) from [<c029e1e8>] (regmap_write+0x48/0x68)
r7:ed8ff400 r6:ffffffff r5:00000054 r4:ed934200
[<c029e1a0>] (regmap_write) from [<c022d34c>] (da9210_i2c_probe+0xb4/0x13c)
r6:ed8ff420 r5:ee3c9110 r4:ed934200 r3:ee7c15a0
[<c022d298>] (da9210_i2c_probe) from [<c033e1c4>] (i2c_device_probe+0xf8/0x120)
r8:ed8ff404 r7:c022d298 r6:ed8ff420 r5:c055496c r4:ed8ff400
[<c033e0cc>] (i2c_device_probe) from [<c028a964>] (driver_probe_device+0x100/0x268)
r8:00000000 r7:c0ee3100 r6:c0ee30ec r5:00000000 r4:ed8ff420 r3:c033e0cc
[<c028a864>] (driver_probe_device) from [<c028ab90>] (__driver_attach+0x78/0x9c)
r9:c0712000 r8:c06ba84c r7:c06fe6a4 r6:c06ee170 r5:ed8ff454 r4:ed8ff420
[<c028ab18>] (__driver_attach) from [<c0289218>] (bus_for_each_dev+0x74/0x98)
r6:c028ab18 r5:c06ee170 r4:00000000 r3:00000001
[<c02891a4>] (bus_for_each_dev) from [<c028ac68>] (driver_attach+0x20/0x28)
r6:ee1fc940 r5:00000000 r4:c06ee170
[<c028ac48>] (driver_attach) from [<c02899f0>] (bus_add_driver+0xdc/0x1c4) [<c0289914>] (bus_add_driver) from [<c028b378>] (driver_register+0xa4/0xe8)
r7:c06ba84c r6:00000000 r5:c06a0e38 r4:c06ee170
[<c028b2d4>] (driver_register) from [<c033ef78>] (i2c_register_driver+0x48/0x7c)
r5:c06a0e38 r4:c06ee154
[<c033ef30>] (i2c_register_driver) from [<c06a0e50>] (da9210_regulator_driver_init+0x18/0x20)
r5:c06a0e38 r4:ee3c9340
[<c06a0e38>] (da9210_regulator_driver_init) from [<c0687dfc>] (do_one_initcall+0x108/0x1bc)
[<c0687cf4>] (do_one_initcall) from [<c0687fcc>] (kernel_init_freeable+0x11c/0x1e4)
r9:c0712000 r8:c0712000 r7:c06c3d78 r6:c06bb078 r5:000000b3 r4:00000006
[<c0687eb0>] (kernel_init_freeable) from [<c04f30d4>] (kernel_init+0x10/0xec)
r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c04f30c4 r4:00000000
[<c04f30c4>] (kernel_init) from [<c000fcb0>] (ret_from_fork+0x14/0x24)
r4:00000000 r3:00000000

"enabled in the driver using clk_enable(p->clk) (in irqc_irq_set_wake())". As this increases the reference count, pm_clk_suspend() during suspend won't disable the clock.

Right...

Probably the IRQC driver needs to return -EBUSY in some callback, but we have to be careful that this doesn't prevent the whole system from going into suspend.

For current SoCs, it will work as-is, as the PM domains containing interrupt and gpio controllers won't be powered down:
- PFC (which provides gpio) is in the "always on" C5 power area on R-MoBile and SH-Mobile,
- GPIO is always on on R-Car,
- IRQC is in C4 on r8a73a4, but won't be powered down as the ARM subsystem is a child of C4.
- intc-irqpin is in A4S on r8a7740/sh73a0, but ARM etc. is a child,
- GIC is always on on R-Car.

(Note that on r8a73a4, the pfc is in the "always on" C5 power area, while
its interrupt lines are tied to irqc[01], which is in the C4 power area and thus can be powered down? Doesn't seem to make sense, even considering another CPU core can be in charge, as irqc[01] is not specific to the ARM core)....

Given the above, for current SoCs no PM domains can be powered down. So that leaves us with clocks only for saving some power.

[...]

No, Runtime PM is separate from system suspend: a device in "get" state can still be suspended during system suspend.

It's similar with PM Domains: to prevent them from being powered down, you need two precautions:
- use pm_domain_always_on_gov (against runtime suspend),
- let .power_off() return -EBUSY (against system suspend).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

In fact the pm_runtime operations _can_ be called from interrupt context, provided the driver has first invoked pm_runtime_irq_safe(). Of course, this requires that none of the runtime-PM callback routines ever sleep or perform a blocking operation.

This is all explained in Documentation/power/runtime_pm.txt (search for "irq_safe").

Alan Stern

Perhaps that can help. We'll have to give it a try...

I've always found this a bit strange when PM Domains are involved:
pm_runtime_irq_safe(dev) applies to device dev, while the actual callbacks belong to the PM Domain code (the device's driver doesn't have any).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Currently PM domains don't support IRQ-safe devices very well. Any irq_safe devices will prevent the entire PM domain from ever powering off. There is some on-going work for this but it requires reworking the genpd locking a bit.

Kevin