* [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm.
@ 2013-12-18 11:36 Sourav Poddar
2013-12-18 11:36 ` [PATCH 1/3] pwm: core: Rearrange pwm lock Sourav Poddar
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Sourav Poddar @ 2013-12-18 11:36 UTC (permalink / raw)
To: thierry.reding; +Cc: linux-pwm, linux-omap, balbi, Sourav Poddar
This patch series caters to the issue faced while using tiecap
as a module.
The patch fix lock dependency issue which leads to crash during rmmod.
Also fixes the clock control register setup values during CLK EN call.
Sourav Poddar (3):
pwm: core: Rearrange pwm lock usage.
driver: pwm: ti-ecap: Rmove duplicate put_sync call.
driver: pwmss: Disable stop during Enable clock call..
drivers/pwm/core.c | 4 +++-
drivers/pwm/pwm-tiecap.c | 1 -
drivers/pwm/pwm-tipwmss.c | 2 ++
3 files changed, 5 insertions(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] pwm: core: Rearrange pwm lock.
2013-12-18 11:36 [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm Sourav Poddar
@ 2013-12-18 11:36 ` Sourav Poddar
2014-01-23 13:54 ` Thierry Reding
2013-12-18 11:36 ` [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call Sourav Poddar
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Sourav Poddar @ 2013-12-18 11:36 UTC (permalink / raw)
To: thierry.reding; +Cc: linux-pwm, linux-omap, balbi, Sourav Poddar
When tiecap is used as a module, then while doing a rmmod I
get the following dump.
root@am437x-evm:/# rmmod pwm_tiecap
[ 219.539245]
[ 219.540771] ======================================================
[ 219.546936] [ INFO: possible circular locking dependency detected ]
[ 219.553192] 3.12.4-01557-g9921cde-dirty #134 Not tainted
[ 219.558471] -------------------------------------------------------
[ 219.564727] rmmod/1517 is trying to acquire lock:
[ 219.569427] (s_active#35){++++.+}, at: [<c017ab00>] sysfs_hash_and_remove+0x4c/0x8c
[ 219.577239]
[ 219.577239] but task is already holding lock:
[ 219.583068] (pwm_lock){+.+.+.}, at: [<c0303598>] pwmchip_remove+0x14/0xf8
[ 219.589996]
[ 219.589996] which lock already depends on the new lock.
[ 219.589996]
[ 219.598144]
[ 219.598144] the existing dependency chain (in reverse order) is:
[ 219.605590]
-> #1 (pwm_lock){+.+.+.}:
[ 219.609497] [<c00a2d1c>] lock_acquire+0x9c/0x128
[ 219.614746] [<c0639bc0>] mutex_lock_nested+0x50/0x3dc
[ 219.620391] [<c0303974>] pwm_request_from_chip+0x38/0x6c
[ 219.626312] [<c0303fe0>] pwm_export_store+0x50/0x140
[ 219.631896] [<c039aba8>] dev_attr_store+0x18/0x24
[ 219.637207] [<c017aff0>] sysfs_write_file+0x16c/0x1a0
[ 219.642883] [<c0119084>] vfs_write+0xb0/0x188
[ 219.647857] [<c0119478>] SyS_write+0x3c/0x70
[ 219.652770] [<c0014100>] ret_fast_syscall+0x0/0x48
[ 219.658172]
-> #0 (s_active#35){++++.+}:
[ 219.662353] [<c00a2778>] __lock_acquire+0x1b28/0x1b70
[ 219.667999] [<c00a2d1c>] lock_acquire+0x9c/0x128
[ 219.673248] [<c017c780>] sysfs_addrm_finish+0xe8/0x158
[ 219.678985] [<c017ab00>] sysfs_hash_and_remove+0x4c/0x8c
[ 219.684906] [<c017e224>] remove_files+0x38/0x74
[ 219.690063] [<c017e2a4>] sysfs_remove_group+0x44/0x108
[ 219.695800] [<c017e38c>] sysfs_remove_groups+0x24/0x34
[ 219.701538] [<c039bc2c>] device_del+0xec/0x178
[ 219.706604] [<c039bcc4>] device_unregister+0xc/0x18
[ 219.712097] [<c0303658>] pwmchip_remove+0xd4/0xf8
[ 219.717407] [<c039fdc4>] platform_drv_remove+0x18/0x1c
[ 219.723175] [<c039e6c4>] __device_release_driver+0x70/0xc8
[ 219.729248] [<c039eec8>] driver_detach+0xb4/0xb8
[ 219.734497] [<c039e4ec>] bus_remove_driver+0x8c/0xd0
[ 219.740081] [<c00abd2c>] SyS_delete_module+0x118/0x22c
[ 219.745819] [<c0014100>] ret_fast_syscall+0x0/0x48
[ 219.751220]
[ 219.751220] other info that might help us debug this:
[ 219.751220]
[ 219.759216] Possible unsafe locking scenario:
[ 219.759216]
[ 219.765106] CPU0 CPU1
[ 219.769622] ---- ----
[ 219.774139] lock(pwm_lock);
[ 219.777130] lock(s_active#35);
[ 219.782897] lock(pwm_lock);
[ 219.788391] lock(s_active#35);
[ 219.791656]
[ 219.791656] *** DEADLOCK ***
[ 219.791656]
[ 219.797546] 3 locks held by rmmod/1517:
[ 219.801391] #0: (&__lockdep_no_validate__){......}, at: [<c039ee58>] driver_detach+0x44/0xb8
[ 219.810028] #1: (&__lockdep_no_validate__){......}, at: [<c039ee64>] driver_detach+0x50/0xb8
[ 219.818695] #2: (pwm_lock){+.+.+.}, at: [<c0303598>] pwmchip_remove+0x14/0xf8
[ 219.826049]
[ 219.826049] stack backtrace:
[ 219.830413] CPU: 0 PID: 1517 Comm: rmmod Not tainted 3.12.4-01557-g9921cde-dirty #134
[ 219.838256] [<c001cc98>] (unwind_backtrace+0x0/0xf0) from [<c0018124>] (show_stack+0x10/0x14)
[ 219.846771] [<c0018124>] (show_stack+0x10/0x14) from [<c0636728>] (dump_stack+0x74/0xb4)
[ 219.854858] [<c0636728>] (dump_stack+0x74/0xb4) from [<c06344e4>] (print_circular_bug+0x284/0x2d8)
[ 219.863830] [<c06344e4>] (print_circular_bug+0x284/0x2d8) from [<c00a2778>] (__lock_acquire+0x1b28/0x1b70)
[ 219.873443] [<c00a2778>] (__lock_acquire+0x1b28/0x1b70) from [<c00a2d1c>] (lock_acquire+0x9c/0x128)
[ 219.882476] [<c00a2d1c>] (lock_acquire+0x9c/0x128) from [<c017c780>] (sysfs_addrm_finish+0xe8/0x158)
[ 219.891601] [<c017c780>] (sysfs_addrm_finish+0xe8/0x158) from [<c017ab00>] (sysfs_hash_and_remove+0x4c/0x8c)
[ 219.901397] [<c017ab00>] (sysfs_hash_and_remove+0x4c/0x8c) from [<c017e224>] (remove_files+0x38/0x74)
[ 219.910614] [<c017e224>] (remove_files+0x38/0x74) from [<c017e2a4>] (sysfs_remove_group+0x44/0x108)
[ 219.919647] [<c017e2a4>] (sysfs_remove_group+0x44/0x108) from [<c017e38c>] (sysfs_remove_groups+0x24/0x34)
[ 219.929260] [<c017e38c>] (sysfs_remove_groups+0x24/0x34) from [<c039bc2c>] (device_del+0xec/0x178)
[ 219.938201] [<c039bc2c>] (device_del+0xec/0x178) from [<c039bcc4>] (device_unregister+0xc/0x18)
[ 219.946899] [<c039bcc4>] (device_unregister+0xc/0x18) from [<c0303658>] (pwmchip_remove+0xd4/0xf8)
[ 219.955841] [<c0303658>] (pwmchip_remove+0xd4/0xf8) from [<c039fdc4>] (platform_drv_remove+0x18/0x1c)
[ 219.965057] [<c039fdc4>] (platform_drv_remove+0x18/0x1c) from [<c039e6c4>] (__device_release_driver+0x70/0xc8)
[ 219.975006] [<c039e6c4>] (__device_release_driver+0x70/0xc8) from [<c039eec8>] (driver_detach+0xb4/0xb8)
[ 219.984466] [<c039eec8>] (driver_detach+0xb4/0xb8) from [<c039e4ec>] (bus_remove_driver+0x8c/0xd0)
[ 219.993438] [<c039e4ec>] (bus_remove_driver+0x8c/0xd0) from [<c00abd2c>] (SyS_delete_module+0x118/0x22c)
[ 220.002899] [<c00abd2c>] (SyS_delete_module+0x118/0x22c) from [<c0014100>] (ret_fast_syscall+0x0/0x48)
Looks like s_active lock cannot be held while pwm lock is held.
The patch fixes the above issue by unlocking the pwm lock before acquiring the
sysfs lock.
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/pwm/core.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 2ca9504..3e1d499 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -300,6 +300,7 @@ int pwmchip_remove(struct pwm_chip *chip)
if (test_bit(PWMF_REQUESTED, &pwm->flags)) {
ret = -EBUSY;
+ mutex_unlock(&pwm_lock);
goto out;
}
}
@@ -311,10 +312,11 @@ int pwmchip_remove(struct pwm_chip *chip)
free_pwms(chip);
+ mutex_unlock(&pwm_lock);
+
pwmchip_sysfs_unexport(chip);
out:
- mutex_unlock(&pwm_lock);
return ret;
}
EXPORT_SYMBOL_GPL(pwmchip_remove);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call.
2013-12-18 11:36 [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm Sourav Poddar
2013-12-18 11:36 ` [PATCH 1/3] pwm: core: Rearrange pwm lock Sourav Poddar
@ 2013-12-18 11:36 ` Sourav Poddar
2014-01-23 14:19 ` Thierry Reding
2013-12-18 11:36 ` [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call Sourav Poddar
2014-01-08 6:47 ` [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm sourav
3 siblings, 1 reply; 8+ messages in thread
From: Sourav Poddar @ 2013-12-18 11:36 UTC (permalink / raw)
To: thierry.reding; +Cc: linux-pwm, linux-omap, balbi, Sourav Poddar
Remove duplicate 'pm_runtime_put_sync' in the remove path.
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/pwm/pwm-tiecap.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c
index 4e5c3d1..032092c 100644
--- a/drivers/pwm/pwm-tiecap.c
+++ b/drivers/pwm/pwm-tiecap.c
@@ -279,7 +279,6 @@ static int ecap_pwm_remove(struct platform_device *pdev)
pwmss_submodule_state_change(pdev->dev.parent, PWMSS_ECAPCLK_STOP_REQ);
pm_runtime_put_sync(&pdev->dev);
- pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
return pwmchip_remove(&pc->chip);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call.
2013-12-18 11:36 [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm Sourav Poddar
2013-12-18 11:36 ` [PATCH 1/3] pwm: core: Rearrange pwm lock Sourav Poddar
2013-12-18 11:36 ` [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call Sourav Poddar
@ 2013-12-18 11:36 ` Sourav Poddar
2014-01-23 14:17 ` Thierry Reding
2014-01-08 6:47 ` [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm sourav
3 siblings, 1 reply; 8+ messages in thread
From: Sourav Poddar @ 2013-12-18 11:36 UTC (permalink / raw)
To: thierry.reding; +Cc: linux-pwm, linux-omap, balbi, Sourav Poddar
Writing to ecap register on second insmod crashes with an external
abort. This happens becuase the STOP_CLK bit remains set(from rmmod)
during the second insmod thereby not allowing the clocks to get enabled.
So, we disable STOP clock bit while doing a clock enable.
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
drivers/pwm/pwm-tipwmss.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
index 3b119bc..4749866 100644
--- a/drivers/pwm/pwm-tipwmss.c
+++ b/drivers/pwm/pwm-tipwmss.c
@@ -40,6 +40,8 @@ u16 pwmss_submodule_state_change(struct device *dev, int set)
mutex_lock(&info->pwmss_lock);
val = readw(info->mmio_base + PWMSS_CLKCONFIG);
+ if (set == PWMSS_ECAPCLK_EN)
+ val &= ~PWMSS_ECAPCLK_STOP_REQ;
val |= set;
writew(val , info->mmio_base + PWMSS_CLKCONFIG);
mutex_unlock(&info->pwmss_lock);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm.
2013-12-18 11:36 [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm Sourav Poddar
` (2 preceding siblings ...)
2013-12-18 11:36 ` [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call Sourav Poddar
@ 2014-01-08 6:47 ` sourav
3 siblings, 0 replies; 8+ messages in thread
From: sourav @ 2014-01-08 6:47 UTC (permalink / raw)
To: Sourav Poddar; +Cc: thierry.reding, linux-pwm, linux-omap, balbi
On Wednesday 18 December 2013 05:06 PM, Sourav Poddar wrote:
> This patch series caters to the issue faced while using tiecap
> as a module.
>
> The patch fix lock dependency issue which leads to crash during rmmod.
> Also fixes the clock control register setup values during CLK EN call.
>
> Sourav Poddar (3):
> pwm: core: Rearrange pwm lock usage.
> driver: pwm: ti-ecap: Rmove duplicate put_sync call.
> driver: pwmss: Disable stop during Enable clock call..
>
> drivers/pwm/core.c | 4 +++-
> drivers/pwm/pwm-tiecap.c | 1 -
> drivers/pwm/pwm-tipwmss.c | 2 ++
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
Gentle ping on this..
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] pwm: core: Rearrange pwm lock.
2013-12-18 11:36 ` [PATCH 1/3] pwm: core: Rearrange pwm lock Sourav Poddar
@ 2014-01-23 13:54 ` Thierry Reding
0 siblings, 0 replies; 8+ messages in thread
From: Thierry Reding @ 2014-01-23 13:54 UTC (permalink / raw)
To: Sourav Poddar; +Cc: linux-pwm, linux-omap, balbi
[-- Attachment #1: Type: text/plain, Size: 6311 bytes --]
On Wed, Dec 18, 2013 at 05:06:53PM +0530, Sourav Poddar wrote:
> When tiecap is used as a module, then while doing a rmmod I
> get the following dump.
>
> root@am437x-evm:/# rmmod pwm_tiecap
> [ 219.539245]
> [ 219.540771] ======================================================
> [ 219.546936] [ INFO: possible circular locking dependency detected ]
> [ 219.553192] 3.12.4-01557-g9921cde-dirty #134 Not tainted
> [ 219.558471] -------------------------------------------------------
> [ 219.564727] rmmod/1517 is trying to acquire lock:
> [ 219.569427] (s_active#35){++++.+}, at: [<c017ab00>] sysfs_hash_and_remove+0x4c/0x8c
> [ 219.577239]
> [ 219.577239] but task is already holding lock:
> [ 219.583068] (pwm_lock){+.+.+.}, at: [<c0303598>] pwmchip_remove+0x14/0xf8
> [ 219.589996]
> [ 219.589996] which lock already depends on the new lock.
> [ 219.589996]
> [ 219.598144]
> [ 219.598144] the existing dependency chain (in reverse order) is:
> [ 219.605590]
> -> #1 (pwm_lock){+.+.+.}:
> [ 219.609497] [<c00a2d1c>] lock_acquire+0x9c/0x128
> [ 219.614746] [<c0639bc0>] mutex_lock_nested+0x50/0x3dc
> [ 219.620391] [<c0303974>] pwm_request_from_chip+0x38/0x6c
> [ 219.626312] [<c0303fe0>] pwm_export_store+0x50/0x140
> [ 219.631896] [<c039aba8>] dev_attr_store+0x18/0x24
> [ 219.637207] [<c017aff0>] sysfs_write_file+0x16c/0x1a0
> [ 219.642883] [<c0119084>] vfs_write+0xb0/0x188
> [ 219.647857] [<c0119478>] SyS_write+0x3c/0x70
> [ 219.652770] [<c0014100>] ret_fast_syscall+0x0/0x48
> [ 219.658172]
> -> #0 (s_active#35){++++.+}:
> [ 219.662353] [<c00a2778>] __lock_acquire+0x1b28/0x1b70
> [ 219.667999] [<c00a2d1c>] lock_acquire+0x9c/0x128
> [ 219.673248] [<c017c780>] sysfs_addrm_finish+0xe8/0x158
> [ 219.678985] [<c017ab00>] sysfs_hash_and_remove+0x4c/0x8c
> [ 219.684906] [<c017e224>] remove_files+0x38/0x74
> [ 219.690063] [<c017e2a4>] sysfs_remove_group+0x44/0x108
> [ 219.695800] [<c017e38c>] sysfs_remove_groups+0x24/0x34
> [ 219.701538] [<c039bc2c>] device_del+0xec/0x178
> [ 219.706604] [<c039bcc4>] device_unregister+0xc/0x18
> [ 219.712097] [<c0303658>] pwmchip_remove+0xd4/0xf8
> [ 219.717407] [<c039fdc4>] platform_drv_remove+0x18/0x1c
> [ 219.723175] [<c039e6c4>] __device_release_driver+0x70/0xc8
> [ 219.729248] [<c039eec8>] driver_detach+0xb4/0xb8
> [ 219.734497] [<c039e4ec>] bus_remove_driver+0x8c/0xd0
> [ 219.740081] [<c00abd2c>] SyS_delete_module+0x118/0x22c
> [ 219.745819] [<c0014100>] ret_fast_syscall+0x0/0x48
> [ 219.751220]
> [ 219.751220] other info that might help us debug this:
> [ 219.751220]
> [ 219.759216] Possible unsafe locking scenario:
> [ 219.759216]
> [ 219.765106] CPU0 CPU1
> [ 219.769622] ---- ----
> [ 219.774139] lock(pwm_lock);
> [ 219.777130] lock(s_active#35);
> [ 219.782897] lock(pwm_lock);
> [ 219.788391] lock(s_active#35);
> [ 219.791656]
> [ 219.791656] *** DEADLOCK ***
> [ 219.791656]
> [ 219.797546] 3 locks held by rmmod/1517:
> [ 219.801391] #0: (&__lockdep_no_validate__){......}, at: [<c039ee58>] driver_detach+0x44/0xb8
> [ 219.810028] #1: (&__lockdep_no_validate__){......}, at: [<c039ee64>] driver_detach+0x50/0xb8
> [ 219.818695] #2: (pwm_lock){+.+.+.}, at: [<c0303598>] pwmchip_remove+0x14/0xf8
> [ 219.826049]
> [ 219.826049] stack backtrace:
> [ 219.830413] CPU: 0 PID: 1517 Comm: rmmod Not tainted 3.12.4-01557-g9921cde-dirty #134
> [ 219.838256] [<c001cc98>] (unwind_backtrace+0x0/0xf0) from [<c0018124>] (show_stack+0x10/0x14)
> [ 219.846771] [<c0018124>] (show_stack+0x10/0x14) from [<c0636728>] (dump_stack+0x74/0xb4)
> [ 219.854858] [<c0636728>] (dump_stack+0x74/0xb4) from [<c06344e4>] (print_circular_bug+0x284/0x2d8)
> [ 219.863830] [<c06344e4>] (print_circular_bug+0x284/0x2d8) from [<c00a2778>] (__lock_acquire+0x1b28/0x1b70)
> [ 219.873443] [<c00a2778>] (__lock_acquire+0x1b28/0x1b70) from [<c00a2d1c>] (lock_acquire+0x9c/0x128)
> [ 219.882476] [<c00a2d1c>] (lock_acquire+0x9c/0x128) from [<c017c780>] (sysfs_addrm_finish+0xe8/0x158)
> [ 219.891601] [<c017c780>] (sysfs_addrm_finish+0xe8/0x158) from [<c017ab00>] (sysfs_hash_and_remove+0x4c/0x8c)
> [ 219.901397] [<c017ab00>] (sysfs_hash_and_remove+0x4c/0x8c) from [<c017e224>] (remove_files+0x38/0x74)
> [ 219.910614] [<c017e224>] (remove_files+0x38/0x74) from [<c017e2a4>] (sysfs_remove_group+0x44/0x108)
> [ 219.919647] [<c017e2a4>] (sysfs_remove_group+0x44/0x108) from [<c017e38c>] (sysfs_remove_groups+0x24/0x34)
> [ 219.929260] [<c017e38c>] (sysfs_remove_groups+0x24/0x34) from [<c039bc2c>] (device_del+0xec/0x178)
> [ 219.938201] [<c039bc2c>] (device_del+0xec/0x178) from [<c039bcc4>] (device_unregister+0xc/0x18)
> [ 219.946899] [<c039bcc4>] (device_unregister+0xc/0x18) from [<c0303658>] (pwmchip_remove+0xd4/0xf8)
> [ 219.955841] [<c0303658>] (pwmchip_remove+0xd4/0xf8) from [<c039fdc4>] (platform_drv_remove+0x18/0x1c)
> [ 219.965057] [<c039fdc4>] (platform_drv_remove+0x18/0x1c) from [<c039e6c4>] (__device_release_driver+0x70/0xc8)
> [ 219.975006] [<c039e6c4>] (__device_release_driver+0x70/0xc8) from [<c039eec8>] (driver_detach+0xb4/0xb8)
> [ 219.984466] [<c039eec8>] (driver_detach+0xb4/0xb8) from [<c039e4ec>] (bus_remove_driver+0x8c/0xd0)
> [ 219.993438] [<c039e4ec>] (bus_remove_driver+0x8c/0xd0) from [<c00abd2c>] (SyS_delete_module+0x118/0x22c)
> [ 220.002899] [<c00abd2c>] (SyS_delete_module+0x118/0x22c) from [<c0014100>] (ret_fast_syscall+0x0/0x48)
>
> Looks like s_active lock cannot be held while pwm lock is held.
> The patch fixes the above issue by unlocking the pwm lock before acquiring the
> sysfs lock.
I've been trying to reproduce this, but I can't. I've enabled LOCKDEP
and PROVE_LOCKING in Kconfig, booted a Tegra-based board and did a
couple of modprobe pwm-tegra && modprobe -r pwm-tegra. But I never saw
LOCKDEP complain.
Can you reproduce the issue on latest linux-next? Or is there something
else I should be doing to trigger this?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call.
2013-12-18 11:36 ` [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call Sourav Poddar
@ 2014-01-23 14:17 ` Thierry Reding
0 siblings, 0 replies; 8+ messages in thread
From: Thierry Reding @ 2014-01-23 14:17 UTC (permalink / raw)
To: Sourav Poddar; +Cc: linux-pwm, linux-omap, balbi
[-- Attachment #1: Type: text/plain, Size: 3244 bytes --]
On Wed, Dec 18, 2013 at 05:06:55PM +0530, Sourav Poddar wrote:
> Writing to ecap register on second insmod crashes with an external
> abort. This happens becuase the STOP_CLK bit remains set(from rmmod)
> during the second insmod thereby not allowing the clocks to get enabled.
>
> So, we disable STOP clock bit while doing a clock enable.
>
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> drivers/pwm/pwm-tipwmss.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pwm/pwm-tipwmss.c b/drivers/pwm/pwm-tipwmss.c
> index 3b119bc..4749866 100644
> --- a/drivers/pwm/pwm-tipwmss.c
> +++ b/drivers/pwm/pwm-tipwmss.c
> @@ -40,6 +40,8 @@ u16 pwmss_submodule_state_change(struct device *dev, int set)
>
> mutex_lock(&info->pwmss_lock);
> val = readw(info->mmio_base + PWMSS_CLKCONFIG);
> + if (set == PWMSS_ECAPCLK_EN)
> + val &= ~PWMSS_ECAPCLK_STOP_REQ;
Should this be done for set == PWMSS_EPWMCLK_EN as well? Also how does
this behave if somebody goes and passes:
PWMSS_ECAPCLK_EN | PWMSS_ECAPCLK_STOP_REQ
as the "set" parameter.
I think that perhaps the pwmss_submodule_state_change() API should be
rethought. Instead of taking a value that's directly written into the
register, perhaps it should abstract away what this does.
From my understanding this is used to enable (or disable) the clock for
a specific submodule (ECAP or EHRPWM). Perhaps an interface like the
following would be more intuitive:
bool pwmss_module_enable(struct device *dev, enum pwmss_module module)
{
struct pwmss_info *info = dev_get_drvdata(dev);
u16 value, mask, state, ack;
switch (module) {
case PWMSS_MODULE_ECAP:
mask = PWMSS_ECAPCLK_EN | PWMSS_ECAPCLK_STOP_REQ;
state = PWMSS_ECAPCLK_EN;
ack = PWMSS_ECAPCLK_EN_ACK;
break;
case PWMSS_MODULE_EPWM:
mask = PWMSS_EPWMCLK_EN | PWMSS_EPWMCLK_STOP_REQ;
state = PWMSS_EPWMCLK_EN;
ack = PWMSS_ECAPCLK_EN_ACK;
break;
default:
return false;
}
mutex_lock(&info->pwmss_lock);
value = readw(info->mmio + PWMSS_CLKCONFIG);
value &= ~mask;
value |= state;
writew(value, info->mmio + PWMSS_CLKCONFIG);
mutex_unlock(&info->pwmss_lock);
value = readw(info->mmio + PWMSS_CLKSTATUS);
return (value & ack) != 0;
}
void pwmss_module_disable(struct device *dev, enum pwmss_module module)
{
struct pwmss_info *info = dev_get_drvdata(dev);
u16 value, mask, state;
switch (module) {
case PWMSS_MODULE_ECAP:
mask = PWMSS_ECAPCLK_EN | PWMSS_ECAPCLK_STOP_REQ;
state = PWMSS_ECAPCLK_STOP_REQ;
break;
case PWMSS_MODULE_EPWM:
mask = PWMSS_EPWMCLK_EN | PWMSS_EPWMCLK_STOP_REQ;
state = PWMSS_EPWMCLK_STOP_REQ;
break;
default:
return false;
}
mutex_lock(&info->pwmss_lock);
value = readw(info->mmio + PWMSS_CLKCONFIG);
value &= ~mask;
value |= state;
writew(value, info->mmio + PWMSS_CLKCONFIG);
mutex_unlock(&info->pwmss_lock);
}
One possible other interesting alternative would be to export this
functionality via the common clock framework, since you're essentially
exposing clk_enable() and clk_disable(). That's probably overkill,
though.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call.
2013-12-18 11:36 ` [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call Sourav Poddar
@ 2014-01-23 14:19 ` Thierry Reding
0 siblings, 0 replies; 8+ messages in thread
From: Thierry Reding @ 2014-01-23 14:19 UTC (permalink / raw)
To: Sourav Poddar; +Cc: linux-pwm, linux-omap, balbi
[-- Attachment #1: Type: text/plain, Size: 314 bytes --]
On Wed, Dec 18, 2013 at 05:06:54PM +0530, Sourav Poddar wrote:
> Remove duplicate 'pm_runtime_put_sync' in the remove path.
>
> Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
> ---
> drivers/pwm/pwm-tiecap.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
Applied, thanks.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-01-23 14:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18 11:36 [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm Sourav Poddar
2013-12-18 11:36 ` [PATCH 1/3] pwm: core: Rearrange pwm lock Sourav Poddar
2014-01-23 13:54 ` Thierry Reding
2013-12-18 11:36 ` [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call Sourav Poddar
2014-01-23 14:19 ` Thierry Reding
2013-12-18 11:36 ` [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call Sourav Poddar
2014-01-23 14:17 ` Thierry Reding
2014-01-08 6:47 ` [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm sourav
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.