[PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm.

[PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm.

[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(-)

[PATCH 1/3] pwm: core: Rearrange pwm lock.

[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

Re: [PATCH 1/3] pwm: core: Rearrange pwm lock.

[Thierry Reding]

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

[PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call.

[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

Re: [PATCH 2/3] driver: pwm: ti-ecap: Remove duplicate put_sync call.

[Thierry Reding]

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

[PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call.

[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

Re: [PATCH 3/3] driver: pwmss: Disable stop clk bit during enable clock call.

[Thierry Reding]

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

Re: [PATCH 0/3] pwm: ti: Miscellaneous Fixes and cleanup for pwm.

[sourav]

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..