linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context
@ 2013-12-26  7:23 Dong Aisheng
  2013-12-26  7:23 ` [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function Dong Aisheng
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dong Aisheng @ 2013-12-26  7:23 UTC (permalink / raw)
  To: linux-arm-kernel

Sometimes we may meet the following lockdep issue.
The root cause is .set_clock callback is executed with spin_lock_irqsave
in sdhci_do_set_ios. However, the IMX set_clock callback will try to access
clk_get_rate which is using a mutex lock.

The fix avoids access mutex in .set_clock callback by initializing the
pltfm_host->clock at probe time and use it later instead of calling
clk_get_rate again in atomic context.

[ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]
3.13.0-rc1+ #285 Not tainted
------------------------------------------------------
kworker/u8:1/29 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
 (prepare_lock){+.+...}, at: [<80480b08>] clk_prepare_lock+0x44/0xe4

and this task is already holding:
 (&(&host->lock)->rlock#2){-.-...}, at: [<804611f4>] sdhci_do_set_ios+0x20/0x720
which would create a new lock dependency:
 (&(&host->lock)->rlock#2){-.-...} -> (prepare_lock){+.+...}

but this new dependency connects a HARDIRQ-irq-safe lock:
 (&(&host->lock)->rlock#2){-.-...}
... which became HARDIRQ-irq-safe at:
  [<8005f030>] mark_lock+0x140/0x6ac
  [<80060760>] __lock_acquire+0xb30/0x1cbc
  [<800620d0>] lock_acquire+0x70/0x84
  [<8061d2f0>] _raw_spin_lock+0x30/0x40
  [<80460668>] sdhci_irq+0x24/0xa68
  [<8006b1d4>] handle_irq_event_percpu+0x54/0x18c
  [<8006b350>] handle_irq_event+0x44/0x64
  [<8006e50c>] handle_fasteoi_irq+0xa0/0x170
  [<8006a8f0>] generic_handle_irq+0x30/0x44
  [<8000f238>] handle_IRQ+0x54/0xbc
  [<8000864c>] gic_handle_irq+0x30/0x64
  [<80013024>] __irq_svc+0x44/0x5c
  [<80614c58>] printk+0x38/0x40
  [<804622a8>] sdhci_add_host+0x844/0xbcc
  [<80464948>] sdhci_esdhc_imx_probe+0x378/0x67c
  [<8032ee88>] platform_drv_probe+0x20/0x50
  [<8032d48c>] driver_probe_device+0x118/0x234
  [<8032d690>] __driver_attach+0x9c/0xa0
  [<8032b89c>] bus_for_each_dev+0x68/0x9c
  [<8032cf44>] driver_attach+0x20/0x28
  [<8032cbc8>] bus_add_driver+0x148/0x1f4
  [<8032dce0>] driver_register+0x80/0x100
  [<8032ee54>] __platform_driver_register+0x50/0x64
  [<8084b094>] sdhci_esdhc_imx_driver_init+0x18/0x20
  [<80008980>] do_one_initcall+0x108/0x16c
  [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
  [<80611c50>] kernel_init+0x10/0x120
  [<8000e9c8>] ret_from_fork+0x14/0x2c

to a HARDIRQ-irq-unsafe lock:
 (prepare_lock){+.+...}
... which became HARDIRQ-irq-unsafe at:
...  [<8005f030>] mark_lock+0x140/0x6ac
  [<8005f604>] mark_held_locks+0x68/0x12c
  [<8005f780>] trace_hardirqs_on_caller+0xb8/0x1d8
  [<8005f8b4>] trace_hardirqs_on+0x14/0x18
  [<8061a130>] mutex_trylock+0x180/0x20c
  [<80480ad8>] clk_prepare_lock+0x14/0xe4
  [<804816a4>] clk_notifier_register+0x28/0xf0
  [<80015120>] twd_clk_init+0x50/0x68
  [<80008980>] do_one_initcall+0x108/0x16c
  [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
  [<80611c50>] kernel_init+0x10/0x120
  [<8000e9c8>] ret_from_fork+0x14/0x2c

other info that might help us debug this:

 Possible interrupt unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(prepare_lock);
                               local_irq_disable();
                               lock(&(&host->lock)->rlock#2);
                               lock(prepare_lock);
  <Interrupt>
    lock(&(&host->lock)->rlock#2);

 *** DEADLOCK ***

3 locks held by kworker/u8:1/29:
 #0:  (kmmcd){.+.+.+}, at: [<8003db18>] process_one_work+0x128/0x468
 #1:  ((&(&host->detect)->work)){+.+.+.}, at: [<8003db18>] process_one_work+0x128/0x468
 #2:  (&(&host->lock)->rlock#2){-.-...}, at: [<804611f4>] sdhci_do_set_ios+0x20/0x720

the dependencies between HARDIRQ-irq-safe lock and the holding lock:
-> (&(&host->lock)->rlock#2){-.-...} ops: 330 {
   IN-HARDIRQ-W at:
                    [<8005f030>] mark_lock+0x140/0x6ac
                    [<80060760>] __lock_acquire+0xb30/0x1cbc
                    [<800620d0>] lock_acquire+0x70/0x84
                    [<8061d2f0>] _raw_spin_lock+0x30/0x40
                    [<80460668>] sdhci_irq+0x24/0xa68
                    [<8006b1d4>] handle_irq_event_percpu+0x54/0x18c
                    [<8006b350>] handle_irq_event+0x44/0x64
                    [<8006e50c>] handle_fasteoi_irq+0xa0/0x170
                    [<8006a8f0>] generic_handle_irq+0x30/0x44
                    [<8000f238>] handle_IRQ+0x54/0xbc
                    [<8000864c>] gic_handle_irq+0x30/0x64
                    [<80013024>] __irq_svc+0x44/0x5c
                    [<80614c58>] printk+0x38/0x40
                    [<804622a8>] sdhci_add_host+0x844/0xbcc
                    [<80464948>] sdhci_esdhc_imx_probe+0x378/0x67c
                    [<8032ee88>] platform_drv_probe+0x20/0x50
                    [<8032d48c>] driver_probe_device+0x118/0x234
                    [<8032d690>] __driver_attach+0x9c/0xa0
                    [<8032b89c>] bus_for_each_dev+0x68/0x9c
                    [<8032cf44>] driver_attach+0x20/0x28
                    [<8032cbc8>] bus_add_driver+0x148/0x1f4
                    [<8032dce0>] driver_register+0x80/0x100
                    [<8032ee54>] __platform_driver_register+0x50/0x64
                    [<8084b094>] sdhci_esdhc_imx_driver_init+0x18/0x20
                    [<80008980>] do_one_initcall+0x108/0x16c
                    [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
                    [<80611c50>] kernel_init+0x10/0x120
                    [<8000e9c8>] ret_from_fork+0x14/0x2c
   IN-SOFTIRQ-W at:
                    [<8005f030>] mark_lock+0x140/0x6ac
                    [<80060204>] __lock_acquire+0x5d4/0x1cbc
                    [<800620d0>] lock_acquire+0x70/0x84
                    [<8061d40c>] _raw_spin_lock_irqsave+0x40/0x54
                    [<8045e4a4>] sdhci_tasklet_finish+0x1c/0x120
                    [<8002b538>] tasklet_action+0xa0/0x15c
                    [<8002b778>] __do_softirq+0x118/0x290
                    [<8002bcf4>] irq_exit+0xb4/0x10c
                    [<8000f240>] handle_IRQ+0x5c/0xbc
                    [<8000864c>] gic_handle_irq+0x30/0x64
                    [<80013024>] __irq_svc+0x44/0x5c
                    [<80614c58>] printk+0x38/0x40
                    [<804622a8>] sdhci_add_host+0x844/0xbcc
                    [<80464948>] sdhci_esdhc_imx_probe+0x378/0x67c
                    [<8032ee88>] platform_drv_probe+0x20/0x50
                    [<8032d48c>] driver_probe_device+0x118/0x234
                    [<8032d690>] __driver_attach+0x9c/0xa0
                    [<8032b89c>] bus_for_each_dev+0x68/0x9c
                    [<8032cf44>] driver_attach+0x20/0x28
                    [<8032cbc8>] bus_add_driver+0x148/0x1f4
                    [<8032dce0>] driver_register+0x80/0x100
                    [<8032ee54>] __platform_driver_register+0x50/0x64
                    [<8084b094>] sdhci_esdhc_imx_driver_init+0x18/0x20
                    [<80008980>] do_one_initcall+0x108/0x16c
                    [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
                    [<80611c50>] kernel_init+0x10/0x120
                    [<8000e9c8>] ret_from_fork+0x14/0x2c
   INITIAL USE at:
                   [<8005f030>] mark_lock+0x140/0x6ac
                   [<8005ff0c>] __lock_acquire+0x2dc/0x1cbc
                   [<800620d0>] lock_acquire+0x70/0x84
                   [<8061d40c>] _raw_spin_lock_irqsave+0x40/0x54
                   [<804611f4>] sdhci_do_set_ios+0x20/0x720
                   [<80461924>] sdhci_set_ios+0x30/0x3c
                   [<8044cea0>] mmc_power_up+0x6c/0xd0
                   [<8044dac4>] mmc_start_host+0x60/0x70
                   [<8044eb3c>] mmc_add_host+0x60/0x88
                   [<8046225c>] sdhci_add_host+0x7f8/0xbcc
                   [<80464948>] sdhci_esdhc_imx_probe+0x378/0x67c
                   [<8032ee88>] platform_drv_probe+0x20/0x50
                   [<8032d48c>] driver_probe_device+0x118/0x234
                   [<8032d690>] __driver_attach+0x9c/0xa0
                   [<8032b89c>] bus_for_each_dev+0x68/0x9c
                   [<8032cf44>] driver_attach+0x20/0x28
                   [<8032cbc8>] bus_add_driver+0x148/0x1f4
                   [<8032dce0>] driver_register+0x80/0x100
                   [<8032ee54>] __platform_driver_register+0x50/0x64
                   [<8084b094>] sdhci_esdhc_imx_driver_init+0x18/0x20
                   [<80008980>] do_one_initcall+0x108/0x16c
                   [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
                   [<80611c50>] kernel_init+0x10/0x120
                   [<8000e9c8>] ret_from_fork+0x14/0x2c
 }
 ... key      at: [<80e040e8>] __key.26952+0x0/0x8
 ... acquired at:
   [<8005eb60>] check_usage+0x3d0/0x5c0
   [<8005edac>] check_irq_usage+0x5c/0xb8
   [<80060d38>] __lock_acquire+0x1108/0x1cbc
   [<800620d0>] lock_acquire+0x70/0x84
   [<8061a210>] mutex_lock_nested+0x54/0x3c0
   [<80480b08>] clk_prepare_lock+0x44/0xe4
   [<8048188c>] clk_get_rate+0x14/0x64
   [<8046374c>] esdhc_pltfm_set_clock+0x20/0x2a4
   [<8045d70c>] sdhci_set_clock+0x4c/0x498
   [<80461518>] sdhci_do_set_ios+0x344/0x720
   [<80461924>] sdhci_set_ios+0x30/0x3c
   [<8044c390>] __mmc_set_clock+0x44/0x60
   [<8044cd4c>] mmc_set_clock+0x10/0x14
   [<8044f8f4>] mmc_init_card+0x1b4/0x1520
   [<80450f00>] mmc_attach_mmc+0xb4/0x194
   [<8044da08>] mmc_rescan+0x294/0x2f0
   [<8003db94>] process_one_work+0x1a4/0x468
   [<8003e850>] worker_thread+0x118/0x3e0
   [<80044de0>] kthread+0xd4/0xf0
   [<8000e9c8>] ret_from_fork+0x14/0x2c

the dependencies between the lock to be acquired and HARDIRQ-irq-unsafe lock:
-> (prepare_lock){+.+...} ops: 395 {
   HARDIRQ-ON-W at:
                    [<8005f030>] mark_lock+0x140/0x6ac
                    [<8005f604>] mark_held_locks+0x68/0x12c
                    [<8005f780>] trace_hardirqs_on_caller+0xb8/0x1d8
                    [<8005f8b4>] trace_hardirqs_on+0x14/0x18
                    [<8061a130>] mutex_trylock+0x180/0x20c
                    [<80480ad8>] clk_prepare_lock+0x14/0xe4
                    [<804816a4>] clk_notifier_register+0x28/0xf0
                    [<80015120>] twd_clk_init+0x50/0x68
                    [<80008980>] do_one_initcall+0x108/0x16c
                    [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
                    [<80611c50>] kernel_init+0x10/0x120
                    [<8000e9c8>] ret_from_fork+0x14/0x2c
   SOFTIRQ-ON-W at:
                    [<8005f030>] mark_lock+0x140/0x6ac
                    [<8005f604>] mark_held_locks+0x68/0x12c
                    [<8005f7c8>] trace_hardirqs_on_caller+0x100/0x1d8
                    [<8005f8b4>] trace_hardirqs_on+0x14/0x18
                    [<8061a130>] mutex_trylock+0x180/0x20c
                    [<80480ad8>] clk_prepare_lock+0x14/0xe4
                    [<804816a4>] clk_notifier_register+0x28/0xf0
                    [<80015120>] twd_clk_init+0x50/0x68
                    [<80008980>] do_one_initcall+0x108/0x16c
                    [<8081cca4>] kernel_init_freeable+0x10c/0x1d0
                    [<80611c50>] kernel_init+0x10/0x120
                    [<8000e9c8>] ret_from_fork+0x14/0x2c
   INITIAL USE at:
                   [<8005f030>] mark_lock+0x140/0x6ac
                   [<8005ff0c>] __lock_acquire+0x2dc/0x1cbc
                   [<800620d0>] lock_acquire+0x70/0x84
                   [<8061a0c8>] mutex_trylock+0x118/0x20c
                   [<80480ad8>] clk_prepare_lock+0x14/0xe4
                   [<80482af8>] __clk_init+0x1c/0x45c
                   [<8048306c>] _clk_register+0xd0/0x170
                   [<80483148>] clk_register+0x3c/0x7c
                   [<80483b4c>] clk_register_fixed_rate+0x88/0xd8
                   [<80483c04>] of_fixed_clk_setup+0x68/0x94
                   [<8084c6fc>] of_clk_init+0x44/0x68
                   [<808202b0>] time_init+0x2c/0x38
                   [<8081ca14>] start_kernel+0x1e4/0x368
                   [<10008074>] 0x10008074
 }
 ... key      at: [<808afebc>] prepare_lock+0x38/0x48
 ... acquired at:
   [<8005eb94>] check_usage+0x404/0x5c0
   [<8005edac>] check_irq_usage+0x5c/0xb8
   [<80060d38>] __lock_acquire+0x1108/0x1cbc
   [<800620d0>] lock_acquire+0x70/0x84
   [<8061a210>] mutex_lock_nested+0x54/0x3c0
   [<80480b08>] clk_prepare_lock+0x44/0xe4
   [<8048188c>] clk_get_rate+0x14/0x64
   [<8046374c>] esdhc_pltfm_set_clock+0x20/0x2a4
   [<8045d70c>] sdhci_set_clock+0x4c/0x498
   [<80461518>] sdhci_do_set_ios+0x344/0x720
   [<80461924>] sdhci_set_ios+0x30/0x3c
   [<8044c390>] __mmc_set_clock+0x44/0x60
   [<8044cd4c>] mmc_set_clock+0x10/0x14
   [<8044f8f4>] mmc_init_card+0x1b4/0x1520
   [<80450f00>] mmc_attach_mmc+0xb4/0x194
   [<8044da08>] mmc_rescan+0x294/0x2f0
   [<8003db94>] process_one_work+0x1a4/0x468
   [<8003e850>] worker_thread+0x118/0x3e0
   [<80044de0>] kthread+0xd4/0xf0
   [<8000e9c8>] ret_from_fork+0x14/0x2c

stack backtrace:
CPU: 2 PID: 29 Comm: kworker/u8:1 Not tainted 3.13.0-rc1+ #285
Workqueue: kmmcd mmc_rescan
Backtrace:
[<80012160>] (dump_backtrace+0x0/0x10c) from [<80012438>] (show_stack+0x18/0x1c)
 r6:00000000 r5:00000000 r4:8088ecc8 r3:bfa11200
[<80012420>] (show_stack+0x0/0x1c) from [<80616b14>] (dump_stack+0x84/0x9c)
[<80616a90>] (dump_stack+0x0/0x9c) from [<8005ebb4>] (check_usage+0x424/0x5c0)
 r5:80979940 r4:bfa29b44
[<8005e790>] (check_usage+0x0/0x5c0) from [<8005edac>] (check_irq_usage+0x5c/0xb8)
[<8005ed50>] (check_irq_usage+0x0/0xb8) from [<80060d38>] (__lock_acquire+0x1108/0x1cbc)
 r8:bfa115e8 r7:80df9884 r6:80dafa9c r5:00000003 r4:bfa115d0
[<8005fc30>] (__lock_acquire+0x0/0x1cbc) from [<800620d0>] (lock_acquire+0x70/0x84)
[<80062060>] (lock_acquire+0x0/0x84) from [<8061a210>] (mutex_lock_nested+0x54/0x3c0)
 r7:bfa11200 r6:80dafa9c r5:00000000 r4:80480b08
[<8061a1bc>] (mutex_lock_nested+0x0/0x3c0) from [<80480b08>] (clk_prepare_lock+0x44/0xe4)
[<80480ac4>] (clk_prepare_lock+0x0/0xe4) from [<8048188c>] (clk_get_rate+0x14/0x64)
 r6:03197500 r5:bf0e9aa8 r4:bf827400 r3:808ae128
[<80481878>] (clk_get_rate+0x0/0x64) from [<8046374c>] (esdhc_pltfm_set_clock+0x20/0x2a4)
 r5:bf0e9aa8 r4:bf0e9c40
[<8046372c>] (esdhc_pltfm_set_clock+0x0/0x2a4) from [<8045d70c>] (sdhci_set_clock+0x4c/0x498)
[<8045d6c0>] (sdhci_set_clock+0x0/0x498) from [<80461518>] (sdhci_do_set_ios+0x344/0x720)
 r8:0000003b r7:20000113 r6:bf0e9d68 r5:bf0e9aa8 r4:bf0e9c40
r3:00000000
[<804611d4>] (sdhci_do_set_ios+0x0/0x720) from [<80461924>] (sdhci_set_ios+0x30/0x3c)
 r9:00000004 r8:bf131000 r7:bf131048 r6:00000000 r5:bf0e9aa8
r4:bf0e9800
[<804618f4>] (sdhci_set_ios+0x0/0x3c) from [<8044c390>] (__mmc_set_clock+0x44/0x60)
 r5:03197500 r4:bf0e9800
[<8044c34c>] (__mmc_set_clock+0x0/0x60) from [<8044cd4c>] (mmc_set_clock+0x10/0x14)
 r5:00000000 r4:bf0e9800
[<8044cd3c>] (mmc_set_clock+0x0/0x14) from [<8044f8f4>] (mmc_init_card+0x1b4/0x1520)
[<8044f740>] (mmc_init_card+0x0/0x1520) from [<80450f00>] (mmc_attach_mmc+0xb4/0x194)
[<80450e4c>] (mmc_attach_mmc+0x0/0x194) from [<8044da08>] (mmc_rescan+0x294/0x2f0)
 r5:8065f358 r4:bf0e9af8
[<8044d774>] (mmc_rescan+0x0/0x2f0) from [<8003db94>] (process_one_work+0x1a4/0x468)
 r8:00000000 r7:bfa29eb0 r6:bf80dc00 r5:bf0e9af8 r4:bf9e3f00
r3:8044d774
[<8003d9f0>] (process_one_work+0x0/0x468) from [<8003e850>] (worker_thread+0x118/0x3e0)
[<8003e738>] (worker_thread+0x0/0x3e0) from [<80044de0>] (kthread+0xd4/0xf0)
[<80044d0c>] (kthread+0x0/0xf0) from [<8000e9c8>] (ret_from_fork+0x14/0x2c)
 r7:00000000 r6:00000000 r5:80044d0c r4:bf9e7f00

Cc: Chris Ball <cjb@laptop.org>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Fixes: 0ddf03c mmc: esdhc-imx: parse max-frequency from devicetree
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
ChangeLog:
no changes
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 2a04847..745ee1e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -576,19 +576,17 @@ static unsigned int esdhc_pltfm_get_max_clock(struct sdhci_host *host)
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
 	struct esdhc_platform_data *boarddata = &imx_data->boarddata;
 
-	u32 f_host = clk_get_rate(pltfm_host->clk);
-
-	if (boarddata->f_max && (boarddata->f_max < f_host))
+	if (boarddata->f_max && (boarddata->f_max < pltfm_host->clock))
 		return boarddata->f_max;
 	else
-		return f_host;
+		return pltfm_host->clock;
 }
 
 static unsigned int esdhc_pltfm_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 
-	return clk_get_rate(pltfm_host->clk) / 256 / 16;
+	return pltfm_host->clock / 256 / 16;
 }
 
 static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
@@ -596,7 +594,7 @@ static inline void esdhc_pltfm_set_clock(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = pltfm_host->priv;
-	unsigned int host_clock = clk_get_rate(pltfm_host->clk);
+	unsigned int host_clock = pltfm_host->clock;
 	int pre_div = 2;
 	int div = 1;
 	u32 temp, val;
@@ -1017,7 +1015,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 	}
 
 	pltfm_host->clk = imx_data->clk_per;
-
+	pltfm_host->clock = clk_get_rate(pltfm_host->clk);
 	clk_prepare_enable(imx_data->clk_per);
 	clk_prepare_enable(imx_data->clk_ipg);
 	clk_prepare_enable(imx_data->clk_ahb);
-- 
1.7.2.rc3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function
  2013-12-26  7:23 [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Dong Aisheng
@ 2013-12-26  7:23 ` Dong Aisheng
  2014-01-13 18:40   ` Chris Ball
  2013-12-31  5:24 ` [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Shawn Guo
  2014-01-13 18:41 ` Chris Ball
  2 siblings, 1 reply; 6+ messages in thread
From: Dong Aisheng @ 2013-12-26  7:23 UTC (permalink / raw)
  To: linux-arm-kernel

Since the clock is managed by runtime pm currently, we do not need
disable it again during driver remove function, or it will cause
clock disable count mismatch issue since the clocks have already been disabled.

The issue can be simply reproduced by unbind the devices via sysfs.
mx6slevk:/sys/bus/platform/drivers/sdhci-esdhc-imx# echo 2194000.usdhc > unbind
mmc1: card aaaa removed
------------[ cut here ]------------
WARNING: CPU: 0 PID: 657 at drivers/clk/clk.c:842 __clk_disable+0x68/0x88()
Modules linked in:
CPU: 0 PID: 657 Comm: sh Not tainted 3.13.0-rc1+ #285
Backtrace:
[<80012160>] (dump_backtrace+0x0/0x10c) from [<80012438>] (show_stack+0x18/0x1c)
 r6:80481370 r5:00000000 r4:8088ecc8 r3:00000000
[<80012420>] (show_stack+0x0/0x1c) from [<80616b14>] (dump_stack+0x84/0x9c)
[<80616a90>] (dump_stack+0x0/0x9c) from [<80027158>] (warn_slowpath_common+0x70/0x94)
 r5:00000009 r4:00000000
[<800270e8>] (warn_slowpath_common+0x0/0x94) from [<80027220>] (warn_slowpath_null+0x24/0x2c)
 r8:bec4ff78 r7:0000000e r6:bf91d800 r5:bf81d080 r4:bf81d080
[<800271fc>] (warn_slowpath_null+0x0/0x2c) from [<80481370>] (__clk_disable+0x68/0x88)
[<80481308>] (__clk_disable+0x0/0x88) from [<8048148c>] (clk_disable+0x20/0x2c)
 r4:200f0113 r3:bf95ec00
[<8048146c>] (clk_disable+0x0/0x2c) from [<80463bd8>] (sdhci_esdhc_imx_remove+0x64/0xa4)
 r5:bf81d080 r4:bfabb010
[<80463b74>] (sdhci_esdhc_imx_remove+0x0/0xa4) from [<8032e82c>] (platform_drv_remove+0x20/0x24)
 r6:808ae0e0 r5:808ae0e0 r4:bf91d810 r3:80463b74
[<8032e80c>] (platform_drv_remove+0x0/0x24) from [<8032d010>] (__device_release_driver+0x78/0xd0)
[<8032cf98>] (__device_release_driver+0x0/0xd0) from [<8032d090>] (device_release_driver+0x28/0x34)
 r5:bf91d810 r4:bf91d844
[<8032d068>] (device_release_driver+0x0/0x34) from [<8032c0c8>] (unbind_store+0x80/0xc4)
 r5:bf91d810 r4:80899ba0
[<8032c048>] (unbind_store+0x0/0xc4) from [<8032b648>] (drv_attr_store+0x28/0x34)
 r7:bed73100 r6:0000000e r5:00000000 r4:8032b620
[<8032b620>] (drv_attr_store+0x0/0x34) from [<80140580>] (sysfs_write_file+0x1b0/0x1e4)
[<801403d0>] (sysfs_write_file+0x0/0x1e4) from [<800dcda0>] (vfs_write+0xb4/0x190)
[<800dccec>] (vfs_write+0x0/0x190) from [<800dd3e4>] (SyS_write+0x44/0x80)
 r9:0000000e r8:00000000 r7:01a00408 r6:bf3b1c00 r5:00000000
r4:00000000
[<800dd3a0>] (SyS_write+0x0/0x80) from [<8000e900>] (ret_fast_syscall+0x0/0x48)
 r9:bec4e000 r8:8000eac4 r7:00000004 r6:76f5fb40 r5:01a00408
r4:0000000e
---[ end trace a0897d268e6233b2 ]---

If without runtime pm, we just run as before to match the clock enable
in probe function.

Cc: Chris Ball <cjb@laptop.org>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
ChangeLog:
v1->v2: cover the !CONFIG_PM_RUNTIME case as suggested by Shawn
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 745ee1e..452171f 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1169,9 +1169,11 @@ static int sdhci_esdhc_imx_remove(struct platform_device *pdev)
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-	clk_disable_unprepare(imx_data->clk_per);
-	clk_disable_unprepare(imx_data->clk_ipg);
-	clk_disable_unprepare(imx_data->clk_ahb);
+	if (!IS_ENABLED(CONFIG_PM_RUNTIME)) {
+		clk_disable_unprepare(imx_data->clk_per);
+		clk_disable_unprepare(imx_data->clk_ipg);
+		clk_disable_unprepare(imx_data->clk_ahb);
+	}
 
 	sdhci_pltfm_free(pdev);
 
-- 
1.7.2.rc3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context
  2013-12-26  7:23 [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Dong Aisheng
  2013-12-26  7:23 ` [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function Dong Aisheng
@ 2013-12-31  5:24 ` Shawn Guo
  2014-01-13 10:52   ` Dong Aisheng
  2014-01-13 18:41 ` Chris Ball
  2 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2013-12-31  5:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 26, 2013 at 03:23:53PM +0800, Dong Aisheng wrote:
> Sometimes we may meet the following lockdep issue.
> The root cause is .set_clock callback is executed with spin_lock_irqsave
> in sdhci_do_set_ios. However, the IMX set_clock callback will try to access
> clk_get_rate which is using a mutex lock.
> 
> The fix avoids access mutex in .set_clock callback by initializing the
> pltfm_host->clock at probe time and use it later instead of calling
> clk_get_rate again in atomic context.

<snip>

> Cc: Chris Ball <cjb@laptop.org>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>

For both,

Acked-by: Shawn Guo <shawn.guo@linaro.org>

> Fixes: 0ddf03c mmc: esdhc-imx: parse max-frequency from devicetree
> Signed-off-by: Dong Aisheng <b29396@freescale.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context
  2013-12-31  5:24 ` [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Shawn Guo
@ 2014-01-13 10:52   ` Dong Aisheng
  0 siblings, 0 replies; 6+ messages in thread
From: Dong Aisheng @ 2014-01-13 10:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Chris,

On Tue, Dec 31, 2013 at 1:24 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Thu, Dec 26, 2013 at 03:23:53PM +0800, Dong Aisheng wrote:
>> Sometimes we may meet the following lockdep issue.
>> The root cause is .set_clock callback is executed with spin_lock_irqsave
>> in sdhci_do_set_ios. However, the IMX set_clock callback will try to access
>> clk_get_rate which is using a mutex lock.
>>
>> The fix avoids access mutex in .set_clock callback by initializing the
>> pltfm_host->clock at probe time and use it later instead of calling
>> clk_get_rate again in atomic context.
>
> <snip>
>
>> Cc: Chris Ball <cjb@laptop.org>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>
> For both,
>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
>
>> Fixes: 0ddf03c mmc: esdhc-imx: parse max-frequency from devicetree
>> Signed-off-by: Dong Aisheng <b29396@freescale.com>
>
>

Can we make patch 1 go into 3.13 final since the issue exists
since 3.13 rc1.

Patch 2 in that series is the fix for sdhci-esdhc-imx runtime pm which is
newly added in chris/mmc-next and seems target for 3.14.

BTW, pls add:
Tested-by: Philippe De Muyter <phdm@macqel.be>

See:
https://lkml.org/lkml/2014/1/13/77

Regards
Dong Aisheng

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function
  2013-12-26  7:23 ` [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function Dong Aisheng
@ 2014-01-13 18:40   ` Chris Ball
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Ball @ 2014-01-13 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Dec 26 2013, Dong Aisheng wrote:
> Since the clock is managed by runtime pm currently, we do not need
> disable it again during driver remove function, or it will cause
> clock disable count mismatch issue since the clocks have already been disabled.
>
> The issue can be simply reproduced by unbind the devices via sysfs.
> mx6slevk:/sys/bus/platform/drivers/sdhci-esdhc-imx# echo 2194000.usdhc > unbind
> mmc1: card aaaa removed
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 657 at drivers/clk/clk.c:842 __clk_disable+0x68/0x88()

Thanks, pushed to mmc-next.

- Chris.
-- 
Chris Ball   <chris@printf.net>   <http://printf.net/>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context
  2013-12-26  7:23 [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Dong Aisheng
  2013-12-26  7:23 ` [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function Dong Aisheng
  2013-12-31  5:24 ` [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Shawn Guo
@ 2014-01-13 18:41 ` Chris Ball
  2 siblings, 0 replies; 6+ messages in thread
From: Chris Ball @ 2014-01-13 18:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Dec 26 2013, Dong Aisheng wrote:
> Sometimes we may meet the following lockdep issue.
> The root cause is .set_clock callback is executed with spin_lock_irqsave
> in sdhci_do_set_ios. However, the IMX set_clock callback will try to access
> clk_get_rate which is using a mutex lock.
>
> The fix avoids access mutex in .set_clock callback by initializing the
> pltfm_host->clock at probe time and use it later instead of calling
> clk_get_rate again in atomic context.
>
> [ INFO: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected ]

Thanks, pushed to mmc-next.

- Chris.
-- 
Chris Ball   <chris@printf.net>   <http://printf.net/>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-13 18:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-26  7:23 [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Dong Aisheng
2013-12-26  7:23 ` [PATCH V2 2/2] mmc: sdhci-esdhc-imx: fix warning during module remove function Dong Aisheng
2014-01-13 18:40   ` Chris Ball
2013-12-31  5:24 ` [PATCH V2 1/2] mmc: sdhci-esdhc-imx: fix access hardirq-unsafe lock in atomic context Shawn Guo
2014-01-13 10:52   ` Dong Aisheng
2014-01-13 18:41 ` Chris Ball

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).