[1/2] gpio: omap: move pm runtime in irq_chip.irq_bus_lock/sync_unlock
diff mbox

Message ID 1443209283-20781-2-git-send-email-grygorii.strashko@ti.com
State New
Headers show

Commit Message

grygorii Sept. 25, 2015, 7:28 p.m. UTC
The PM runtime API can't be used in atomic contex on -RT even if
it's configured as irqsafe. As result, below error report can
be seen when PM runtime API called from IRQ chip's callbacks
irq_startup/irq_shutdown/irq_set_type, because they are
protected by RAW spinlock:

BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:917
in_atomic(): 1, irqs_disabled(): 128, pid: 96, name: insmod
3 locks held by insmod/96:
 #0:  (&dev->mutex){......}, at: [<c04752c8>] __driver_attach+0x54/0xa0
 #1:  (&dev->mutex){......}, at: [<c04752d4>] __driver_attach+0x60/0xa0
 #2:  (class){......}, at: [<c00a408c>] __irq_get_desc_lock+0x60/0xa4
irq event stamp: 1834
hardirqs last  enabled at (1833): [<c06ab2a4>] _raw_spin_unlock_irqrestore+0x88/0x90
hardirqs last disabled at (1834): [<c06ab068>] _raw_spin_lock_irqsave+0x2c/0x64
softirqs last  enabled at (0): [<c003d220>] copy_process.part.52+0x410/0x19d8
softirqs last disabled at (0): [<  (null)>]   (null)
Preemption disabled at:[<  (null)>]   (null)

CPU: 1 PID: 96 Comm: insmod Tainted: G        W  O    4.1.3-rt3-00618-g57e2387-dirty #184
Hardware name: Generic DRA74X (Flattened Device Tree)
[<c00190f4>] (unwind_backtrace) from [<c0014734>] (show_stack+0x20/0x24)
[<c0014734>] (show_stack) from [<c06a62ec>] (dump_stack+0x88/0xdc)
[<c06a62ec>] (dump_stack) from [<c006ca44>] (___might_sleep+0x198/0x2a8)
[<c006ca44>] (___might_sleep) from [<c06ab6d4>] (rt_spin_lock+0x30/0x70)
[<c06ab6d4>] (rt_spin_lock) from [<c04815ac>] (__pm_runtime_resume+0x68/0xa4)
[<c04815ac>] (__pm_runtime_resume) from [<c04123f4>] (omap_gpio_irq_type+0x188/0x1d8)
[<c04123f4>] (omap_gpio_irq_type) from [<c00a64e4>] (__irq_set_trigger+0x68/0x130)
[<c00a64e4>] (__irq_set_trigger) from [<c00a7bc4>] (irq_set_irq_type+0x44/0x6c)
[<c00a7bc4>] (irq_set_irq_type) from [<c00abbf8>] (irq_create_of_mapping+0x120/0x174)
[<c00abbf8>] (irq_create_of_mapping) from [<c0577b74>] (of_irq_get+0x48/0x58)
[<c0577b74>] (of_irq_get) from [<c0540a14>] (i2c_device_probe+0x54/0x15c)
[<c0540a14>] (i2c_device_probe) from [<c04750dc>] (driver_probe_device+0x184/0x2c8)
[<c04750dc>] (driver_probe_device) from [<c0475310>] (__driver_attach+0x9c/0xa0)
[<c0475310>] (__driver_attach) from [<c0473238>] (bus_for_each_dev+0x7c/0xb0)
[<c0473238>] (bus_for_each_dev) from [<c0474af4>] (driver_attach+0x28/0x30)
[<c0474af4>] (driver_attach) from [<c0474760>] (bus_add_driver+0x154/0x200)
[<c0474760>] (bus_add_driver) from [<c0476348>] (driver_register+0x88/0x108)
[<c0476348>] (driver_register) from [<c0541600>] (i2c_register_driver+0x3c/0x90)
[<c0541600>] (i2c_register_driver) from [<bf003018>] (pcf857x_init+0x18/0x24 [gpio_pcf857x])
[<bf003018>] (pcf857x_init [gpio_pcf857x]) from [<c000998c>] (do_one_initcall+0x128/0x1e8)
[<c000998c>] (do_one_initcall) from [<c06a4220>] (do_init_module+0x6c/0x1bc)
[<c06a4220>] (do_init_module) from [<c00dd0c8>] (load_module+0x18e8/0x21c4)
[<c00dd0c8>] (load_module) from [<c00ddaa0>] (SyS_init_module+0xfc/0x158)
[<c00ddaa0>] (SyS_init_module) from [<c000ff40>] (ret_fast_syscall+0x0/0x54)

The IRQ chip interface defines only two callbacks which are executed in
non-atomic contex - irq_bus_lock/irq_bus_sync_unlock, so lets move
PM runtime calls there.

Tested-by: Tony Lindgren <tony@atomide.com>
Tested-by: Austin Schuh <austin@peloton-tech.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-omap.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Comments

Linus Walleij Oct. 2, 2015, 12:22 p.m. UTC | #1
On Fri, Sep 25, 2015 at 12:28 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> The PM runtime API can't be used in atomic contex on -RT even if
> it's configured as irqsafe. As result, below error report can
> be seen when PM runtime API called from IRQ chip's callbacks
> irq_startup/irq_shutdown/irq_set_type, because they are
> protected by RAW spinlock:
>
> BUG: sleeping function called from invalid context at kernel/locking/rtmutex.c:917
> in_atomic(): 1, irqs_disabled(): 128, pid: 96, name: insmod
> 3 locks held by insmod/96:
>  #0:  (&dev->mutex){......}, at: [<c04752c8>] __driver_attach+0x54/0xa0
>  #1:  (&dev->mutex){......}, at: [<c04752d4>] __driver_attach+0x60/0xa0
>  #2:  (class){......}, at: [<c00a408c>] __irq_get_desc_lock+0x60/0xa4
> irq event stamp: 1834
> hardirqs last  enabled at (1833): [<c06ab2a4>] _raw_spin_unlock_irqrestore+0x88/0x90
> hardirqs last disabled at (1834): [<c06ab068>] _raw_spin_lock_irqsave+0x2c/0x64
> softirqs last  enabled at (0): [<c003d220>] copy_process.part.52+0x410/0x19d8
> softirqs last disabled at (0): [<  (null)>]   (null)
> Preemption disabled at:[<  (null)>]   (null)
>
> CPU: 1 PID: 96 Comm: insmod Tainted: G        W  O    4.1.3-rt3-00618-g57e2387-dirty #184
> Hardware name: Generic DRA74X (Flattened Device Tree)
> [<c00190f4>] (unwind_backtrace) from [<c0014734>] (show_stack+0x20/0x24)
> [<c0014734>] (show_stack) from [<c06a62ec>] (dump_stack+0x88/0xdc)
> [<c06a62ec>] (dump_stack) from [<c006ca44>] (___might_sleep+0x198/0x2a8)
> [<c006ca44>] (___might_sleep) from [<c06ab6d4>] (rt_spin_lock+0x30/0x70)
> [<c06ab6d4>] (rt_spin_lock) from [<c04815ac>] (__pm_runtime_resume+0x68/0xa4)
> [<c04815ac>] (__pm_runtime_resume) from [<c04123f4>] (omap_gpio_irq_type+0x188/0x1d8)
> [<c04123f4>] (omap_gpio_irq_type) from [<c00a64e4>] (__irq_set_trigger+0x68/0x130)
> [<c00a64e4>] (__irq_set_trigger) from [<c00a7bc4>] (irq_set_irq_type+0x44/0x6c)
> [<c00a7bc4>] (irq_set_irq_type) from [<c00abbf8>] (irq_create_of_mapping+0x120/0x174)
> [<c00abbf8>] (irq_create_of_mapping) from [<c0577b74>] (of_irq_get+0x48/0x58)
> [<c0577b74>] (of_irq_get) from [<c0540a14>] (i2c_device_probe+0x54/0x15c)
> [<c0540a14>] (i2c_device_probe) from [<c04750dc>] (driver_probe_device+0x184/0x2c8)
> [<c04750dc>] (driver_probe_device) from [<c0475310>] (__driver_attach+0x9c/0xa0)
> [<c0475310>] (__driver_attach) from [<c0473238>] (bus_for_each_dev+0x7c/0xb0)
> [<c0473238>] (bus_for_each_dev) from [<c0474af4>] (driver_attach+0x28/0x30)
> [<c0474af4>] (driver_attach) from [<c0474760>] (bus_add_driver+0x154/0x200)
> [<c0474760>] (bus_add_driver) from [<c0476348>] (driver_register+0x88/0x108)
> [<c0476348>] (driver_register) from [<c0541600>] (i2c_register_driver+0x3c/0x90)
> [<c0541600>] (i2c_register_driver) from [<bf003018>] (pcf857x_init+0x18/0x24 [gpio_pcf857x])
> [<bf003018>] (pcf857x_init [gpio_pcf857x]) from [<c000998c>] (do_one_initcall+0x128/0x1e8)
> [<c000998c>] (do_one_initcall) from [<c06a4220>] (do_init_module+0x6c/0x1bc)
> [<c06a4220>] (do_init_module) from [<c00dd0c8>] (load_module+0x18e8/0x21c4)
> [<c00dd0c8>] (load_module) from [<c00ddaa0>] (SyS_init_module+0xfc/0x158)
> [<c00ddaa0>] (SyS_init_module) from [<c000ff40>] (ret_fast_syscall+0x0/0x54)
>
> The IRQ chip interface defines only two callbacks which are executed in
> non-atomic contex - irq_bus_lock/irq_bus_sync_unlock, so lets move
> PM runtime calls there.
>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Tested-by: Austin Schuh <austin@peloton-tech.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Patch applied with Santosh's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 5236db1..a254691 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -496,9 +496,6 @@  static int omap_gpio_irq_type(struct irq_data *d, unsigned type)
 		(type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
 		return -EINVAL;
 
-	if (!BANK_USED(bank))
-		pm_runtime_get_sync(bank->dev);
-
 	raw_spin_lock_irqsave(&bank->lock, flags);
 	retval = omap_set_gpio_triggering(bank, offset, type);
 	if (retval) {
@@ -521,8 +518,6 @@  static int omap_gpio_irq_type(struct irq_data *d, unsigned type)
 	return 0;
 
 error:
-	if (!BANK_USED(bank))
-		pm_runtime_put(bank->dev);
 	return retval;
 }
 
@@ -797,9 +792,6 @@  static unsigned int omap_gpio_irq_startup(struct irq_data *d)
 	unsigned long flags;
 	unsigned offset = d->hwirq;
 
-	if (!BANK_USED(bank))
-		pm_runtime_get_sync(bank->dev);
-
 	raw_spin_lock_irqsave(&bank->lock, flags);
 
 	if (!LINE_USED(bank->mod_usage, offset))
@@ -815,8 +807,6 @@  static unsigned int omap_gpio_irq_startup(struct irq_data *d)
 	return 0;
 err:
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
-	if (!BANK_USED(bank))
-		pm_runtime_put(bank->dev);
 	return -EINVAL;
 }
 
@@ -835,6 +825,19 @@  static void omap_gpio_irq_shutdown(struct irq_data *d)
 		omap_clear_gpio_debounce(bank, offset);
 	omap_disable_gpio_module(bank, offset);
 	raw_spin_unlock_irqrestore(&bank->lock, flags);
+}
+
+static void omap_gpio_irq_bus_lock(struct irq_data *data)
+{
+	struct gpio_bank *bank = omap_irq_data_get_bank(data);
+
+	if (!BANK_USED(bank))
+		pm_runtime_get_sync(bank->dev);
+}
+
+static void gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+	struct gpio_bank *bank = omap_irq_data_get_bank(data);
 
 	/*
 	 * If this is the last IRQ to be freed in the bank,
@@ -1183,6 +1186,8 @@  static int omap_gpio_probe(struct platform_device *pdev)
 	irqc->irq_unmask = omap_gpio_unmask_irq,
 	irqc->irq_set_type = omap_gpio_irq_type,
 	irqc->irq_set_wake = omap_gpio_wake_enable,
+	irqc->irq_bus_lock = omap_gpio_irq_bus_lock,
+	irqc->irq_bus_sync_unlock = gpio_irq_bus_sync_unlock,
 	irqc->name = dev_name(&pdev->dev);
 
 	bank->irq = platform_get_irq(pdev, 0);