linux-stable.git/include/linux/pwm.h, branch v4.9.321

pwm: Fix deadlock warning when removing PWM device

2019-06-22T06:17:17+00:00

[ Upstream commit 347ab9480313737c0f1aaa08e8f2e1a791235535 ]

This patch fixes deadlock warning if removing PWM device
when CONFIG_PROVE_LOCKING is enabled.

This issue can be reproceduced by the following steps on
the R-Car H3 Salvator-X board if the backlight is disabled:

 # cd /sys/class/pwm/pwmchip0
 # echo 0 > export
 # ls
 device  export  npwm  power  pwm0  subsystem  uevent  unexport
 # cd device/driver
 # ls
 bind  e6e31000.pwm  uevent  unbind
 # echo e6e31000.pwm > unbind

[   87.659974] ======================================================
[   87.666149] WARNING: possible circular locking dependency detected
[   87.672327] 5.0.0 #7 Not tainted
[   87.675549] ------------------------------------------------------
[   87.681723] bash/2986 is trying to acquire lock:
[   87.686337] 000000005ea0e178 (kn->count#58){++++}, at: kernfs_remove_by_name_ns+0x50/0xa0
[   87.694528]
[   87.694528] but task is already holding lock:
[   87.700353] 000000006313b17c (pwm_lock){+.+.}, at: pwmchip_remove+0x28/0x13c
[   87.707405]
[   87.707405] which lock already depends on the new lock.
[   87.707405]
[   87.715574]
[   87.715574] the existing dependency chain (in reverse order) is:
[   87.723048]
[   87.723048] -> #1 (pwm_lock){+.+.}:
[   87.728017]        __mutex_lock+0x70/0x7e4
[   87.732108]        mutex_lock_nested+0x1c/0x24
[   87.736547]        pwm_request_from_chip.part.6+0x34/0x74
[   87.741940]        pwm_request_from_chip+0x20/0x40
[   87.746725]        export_store+0x6c/0x1f4
[   87.750820]        dev_attr_store+0x18/0x28
[   87.754998]        sysfs_kf_write+0x54/0x64
[   87.759175]        kernfs_fop_write+0xe4/0x1e8
[   87.763615]        __vfs_write+0x40/0x184
[   87.767619]        vfs_write+0xa8/0x19c
[   87.771448]        ksys_write+0x58/0xbc
[   87.775278]        __arm64_sys_write+0x18/0x20
[   87.779721]        el0_svc_common+0xd0/0x124
[   87.783986]        el0_svc_compat_handler+0x1c/0x24
[   87.788858]        el0_svc_compat+0x8/0x18
[   87.792947]
[   87.792947] -> #0 (kn->count#58){++++}:
[   87.798260]        lock_acquire+0xc4/0x22c
[   87.802353]        __kernfs_remove+0x258/0x2c4
[   87.806790]        kernfs_remove_by_name_ns+0x50/0xa0
[   87.811836]        remove_files.isra.1+0x38/0x78
[   87.816447]        sysfs_remove_group+0x48/0x98
[   87.820971]        sysfs_remove_groups+0x34/0x4c
[   87.825583]        device_remove_attrs+0x6c/0x7c
[   87.830197]        device_del+0x11c/0x33c
[   87.834201]        device_unregister+0x14/0x2c
[   87.838638]        pwmchip_sysfs_unexport+0x40/0x4c
[   87.843509]        pwmchip_remove+0xf4/0x13c
[   87.847773]        rcar_pwm_remove+0x28/0x34
[   87.852039]        platform_drv_remove+0x24/0x64
[   87.856651]        device_release_driver_internal+0x18c/0x21c
[   87.862391]        device_release_driver+0x14/0x1c
[   87.867175]        unbind_store+0xe0/0x124
[   87.871265]        drv_attr_store+0x20/0x30
[   87.875442]        sysfs_kf_write+0x54/0x64
[   87.879618]        kernfs_fop_write+0xe4/0x1e8
[   87.884055]        __vfs_write+0x40/0x184
[   87.888057]        vfs_write+0xa8/0x19c
[   87.891887]        ksys_write+0x58/0xbc
[   87.895716]        __arm64_sys_write+0x18/0x20
[   87.900154]        el0_svc_common+0xd0/0x124
[   87.904417]        el0_svc_compat_handler+0x1c/0x24
[   87.909289]        el0_svc_compat+0x8/0x18
[   87.913378]
[   87.913378] other info that might help us debug this:
[   87.913378]
[   87.921374]  Possible unsafe locking scenario:
[   87.921374]
[   87.927286]        CPU0                    CPU1
[   87.931808]        ----                    ----
[   87.936331]   lock(pwm_lock);
[   87.939293]                                lock(kn->count#58);
[   87.945120]                                lock(pwm_lock);
[   87.950599]   lock(kn->count#58);
[   87.953908]
[   87.953908]  *** DEADLOCK ***
[   87.953908]
[   87.959821] 4 locks held by bash/2986:
[   87.963563]  #0: 00000000ace7bc30 (sb_writers#6){.+.+}, at: vfs_write+0x188/0x19c
[   87.971044]  #1: 00000000287991b2 (&of->mutex){+.+.}, at: kernfs_fop_write+0xb4/0x1e8
[   87.978872]  #2: 00000000f739d016 (&dev->mutex){....}, at: device_release_driver_internal+0x40/0x21c
[   87.988001]  #3: 000000006313b17c (pwm_lock){+.+.}, at: pwmchip_remove+0x28/0x13c
[   87.995481]
[   87.995481] stack backtrace:
[   87.999836] CPU: 0 PID: 2986 Comm: bash Not tainted 5.0.0 #7
[   88.005489] Hardware name: Renesas Salvator-X board based on r8a7795 ES1.x (DT)
[   88.012791] Call trace:
[   88.015235]  dump_backtrace+0x0/0x190
[   88.018891]  show_stack+0x14/0x1c
[   88.022204]  dump_stack+0xb0/0xec
[   88.025514]  print_circular_bug.isra.32+0x1d0/0x2e0
[   88.030385]  __lock_acquire+0x1318/0x1864
[   88.034388]  lock_acquire+0xc4/0x22c
[   88.037958]  __kernfs_remove+0x258/0x2c4
[   88.041874]  kernfs_remove_by_name_ns+0x50/0xa0
[   88.046398]  remove_files.isra.1+0x38/0x78
[   88.050487]  sysfs_remove_group+0x48/0x98
[   88.054490]  sysfs_remove_groups+0x34/0x4c
[   88.058580]  device_remove_attrs+0x6c/0x7c
[   88.062671]  device_del+0x11c/0x33c
[   88.066154]  device_unregister+0x14/0x2c
[   88.070070]  pwmchip_sysfs_unexport+0x40/0x4c
[   88.074421]  pwmchip_remove+0xf4/0x13c
[   88.078163]  rcar_pwm_remove+0x28/0x34
[   88.081906]  platform_drv_remove+0x24/0x64
[   88.085996]  device_release_driver_internal+0x18c/0x21c
[   88.091215]  device_release_driver+0x14/0x1c
[   88.095478]  unbind_store+0xe0/0x124
[   88.099048]  drv_attr_store+0x20/0x30
[   88.102704]  sysfs_kf_write+0x54/0x64
[   88.106359]  kernfs_fop_write+0xe4/0x1e8
[   88.110275]  __vfs_write+0x40/0x184
[   88.113757]  vfs_write+0xa8/0x19c
[   88.117065]  ksys_write+0x58/0xbc
[   88.120374]  __arm64_sys_write+0x18/0x20
[   88.124291]  el0_svc_common+0xd0/0x124
[   88.128034]  el0_svc_compat_handler+0x1c/0x24
[   88.132384]  el0_svc_compat+0x8/0x18

The sysfs unexport in pwmchip_remove() is completely asymmetric
to what we do in pwmchip_add_with_polarity() and commit 0733424c9ba9
("pwm: Unexport children before chip removal") is a strong indication
that this was wrong to begin with. We should just move
pwmchip_sysfs_unexport() where it belongs, which is right after
pwmchip_sysfs_unexport_children(). In that case, we do not need
separate functions anymore either.

We also really want to remove sysfs irrespective of whether or not
the chip will be removed as a result of pwmchip_remove(). We can only
assume that the driver will be gone after that, so we shouldn't leave
any dangling sysfs files around.

This warning disappears if we move pwmchip_sysfs_unexport() to
the top of pwmchip_remove(), pwmchip_sysfs_unexport_children().
That way it is also outside of the pwm_lock section, which indeed
doesn't seem to be needed.

Moving the pwmchip_sysfs_export() call outside of that section also
seems fine and it'd be perfectly symmetric with pwmchip_remove() again.

So, this patch fixes them.

Signed-off-by: Phong Hoang 
[shimoda: revise the commit log and code]
Fixes: 76abbdde2d95 ("pwm: Add sysfs interface")
Fixes: 0733424c9ba9 ("pwm: Unexport children before chip removal")
Signed-off-by: Yoshihiro Shimoda 
Tested-by: Hoan Nguyen An 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Simon Horman 
Reviewed-by: Uwe Kleine-König 
Signed-off-by: Thierry Reding 
Signed-off-by: Sasha Levin

pwm: Unexport children before chip removal

2016-09-05T14:45:39+00:00

Exported pwm channels aren't removed before the pwmchip and are
leaked. This results in invalid sysfs files. This fix removes
all exported pwm channels before chip removal.

Signed-off-by: David Hsu 
Fixes: 76abbdde2d95 ("pwm: Add sysfs interface")
Signed-off-by: Thierry Reding

Merge branch 'for-4.8/capture' into for-next

2016-07-25T14:23:39+00:00

Merge branch 'for-4.8/core' into for-next

2016-07-25T14:23:37+00:00

pwm: Remove gratuitous blank line

2016-07-08T16:11:51+00:00

Commit 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic
updates") introduced this double blank line by mistake.

Signed-off-by: Thierry Reding

pwm: Add relative duty cycle manipulation helpers

2016-07-08T15:52:19+00:00

The PWM framework expects PWM users to configure the duty cycle in nano-
seconds, but many users want to express the duty cycle relatively to the
period value (i.e. duty_cycle = 33% of the period).

Add the pwm_{get,set}_relative_duty_cycle() helpers to ease this kind of
conversion.

Signed-off-by: Boris Brezillon 
Tested-by: Heiko Stuebner 
Signed-off-by: Thierry Reding

pwm: Add a helper to prepare a new PWM state

2016-07-08T15:52:18+00:00

The pwm_init_state() helper prepares a new state object containing the
current PWM state except for the polarity and period fields which are
set to the reference values (those in struct pwm_args).

This is particularly useful for PWM users who want to apply a new duty-
cycle expressed relatively to the reference period without changing the
enable state.

Signed-off-by: Boris Brezillon 
Tested-by: Heiko Stuebner 
Signed-off-by: Thierry Reding

pwm: Fix pwm_apply_args()

2016-06-23T16:37:05+00:00

Commit 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic
updates"), implemented pwm_disable() as a wrapper around
pwm_apply_state(), and then, commit ef2bf4997f7d ("pwm: Improve args
checking in pwm_apply_state()") added missing checks on the ->period
value in pwm_apply_state() to ensure we were not passing inappropriate
values to the ->config() or ->apply() methods.

The conjunction of these 2 commits led to a case where pwm_disable()
was no longer succeeding, thus preventing the polarity setting done
in pwm_apply_args().

Set a valid period in pwm_apply_args() to ensure polarity setting
won't be rejected.

Signed-off-by: Boris Brezillon 
Reported-by: Geert Uytterhoeven 
Suggested-by: Brian Norris 
Fixes: 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates")
Tested-by: Geert Uytterhoeven 
Reviewed-by: Brian Norris 
Signed-off-by: Thierry Reding

pwm: Add PWM capture support

2016-06-10T14:01:35+00:00

Supply a PWM capture callback op in order to pass back information
obtained by running analysis on a PWM signal. This would normally (at
least during testing) be called from the sysfs routines with a view to
printing out PWM capture data which has been encoded into a string.

Signed-off-by: Lee Jones 
[thierry.reding@gmail.com: make capture data unsigned int for symmetry]
Signed-off-by: Thierry Reding

pwm: Improve args checking in pwm_apply_state()

2016-06-10T12:21:00+00:00

It seems like in the process of refactoring pwm_config() to utilize the
newly-introduced pwm_apply_state() API, some args/bounds checking was
dropped.

In particular, I noted that we are now allowing invalid period
selections, e.g.:

  # echo 1 > /sys/class/pwm/pwmchip0/export
  # cat /sys/class/pwm/pwmchip0/pwm1/period
  100
  # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle
  [... driver may or may not reject the value, or trigger some logic bug ...]

It's better to see:

  # echo 1 > /sys/class/pwm/pwmchip0/export
  # cat /sys/class/pwm/pwmchip0/pwm1/period
  100
  # echo 101 > /sys/class/pwm/pwmchip0/pwm1/duty_cycle
  -bash: echo: write error: Invalid argument

This patch reintroduces some bounds checks in both pwm_config() (for its
signed parameters; we don't want to convert negative values into large
unsigned values) and in pwm_apply_state() (which fix the above described
behavior, as well as other potential API misuses).

Fixes: 5ec803edcb70 ("pwm: Add core infrastructure to allow atomic updates")
Signed-off-by: Brian Norris 
Acked-by: Boris Brezillon 
Signed-off-by: Thierry Reding