Fix bugs in kona pwm driver and pwm core
v10 cover Fix bugs in kona pwm driver and pwm core
Fixes a bug in the pwm core where the enabled state was incorrect on failed calls to enable.
Changes from v9:
- pwm_set_polarity() enabled check is protected using the same mutex as pwm_enable() in pwm core. Implemented as suggested by Thierry.
Changes from v8:
- Accepted patches not included. Patch to introduce debug statements to config in kona pwm driver not included - will be addressed later.
- Added mutex to pwm core enable function to prevent potential concurrency issue, as suggested by Thierry.
- Fixed commit message for kona pwm changes. Also changed wording in comments from enable to trigger.
Changes from v7:
- Polarity changes take effect immediately instead of being deferred until enable is called.
Changes from v6:
- Move new debugging info for duty cycle and period in config function into its own commit.
- Add kona_pwmc_prepare_for_settings() function to remove duplicated code.
Changes from v5:
- Address Dmitry's comment on code cleanup of pwm_enable() in pwm core.
- Including all patches from different contributors in this patchset. Some were left out in v4.
Changes from v4:
- Rebased to Tim Kryger's patch that adds support in pwm core to add driver with inversed polarity.
- Removed patch 2 that resolved difference between hardware default polarity and pwm framework default polarity. The default polarity is set properly now when the pwm driver is registered with the pwm framework.
Jonathan Richardson (1):
pwm: core: Set enable state properly on failed call to enable
drivers/pwm/core.c | 33 ++++++++++++++++++++++++++------- include/linux/pwm.h | 2 ++ 2 files changed, 28 insertions(+), 7 deletions(-)
Signature · 1 line
1.9.1
PATCH v10 1/1 pwm: core: Set enable state properly on failed call to enable
The pwm_enable function didn't clear the enabled bit if a call to a clients enable function returned an error. The result was that the state of the pwm core was wrong. Clearing the bit when enable returns an error ensures the state is properly set.
--- drivers/pwm/core.c | 33 ++++++++++++++++++++++++++------- include/linux/pwm.h | 2 ++ 2 files changed, 28 insertions(+), 7 deletions(-)
drivers/pwm/core.c +26 -7
@@ -269,6 +269,7 @@ int pwmchip_add_with_polarity(struct pwm_chip *chip, pwm->pwm = chip->base + i; pwm->hwpwm = i; pwm->polarity = polarity; +mutex_init (&pwm->lock);radix_tree_insert (&pwm_tree, pwm->pwm, pwm); }
@@ -473,16 +474,22 @@ int pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity)if (!pwm->chip->ops->set_polarity)return -ENOSYS ; -if (pwm_is_enabled (pwm)) -return -EBUSY ; +mutex_lock (&pwm->lock); + +if (pwm_is_enabled (pwm)) { + err = -EBUSY ; + goto unlock; + } err = pwm->chip->ops->set_polarity (pwm->chip, pwm, polarity);if (err) -return err; + goto unlock; pwm->polarity = polarity; -return 0 ; +unlock: +mutex_unlock (&pwm->lock); +return err; }EXPORT_SYMBOL_GPL (pwm_set_polarity);
@@ -494,10 +501,22 @@ EXPORT_SYMBOL_GPL(pwm_set_polarity);*/ int pwm_enable (struct pwm_device *pwm) { -if (pwm && !test_and_set_bit (PWMF_ENABLED , &pwm->flags)) -return pwm->chip->ops->enable (pwm->chip, pwm); +int err =0 ; + +if (!pwm) +return -EINVAL ; + +mutex_lock (&pwm->lock); + +if (!test_and_set_bit (PWMF_ENABLED , &pwm->flags)) { + err = pwm->chip->ops->enable (pwm->chip, pwm); +if (err) +clear_bit (PWMF_ENABLED , &pwm->flags); + } + +mutex_unlock (&pwm->lock); -return pwm ?0 : -EINVAL ; +return err; }EXPORT_SYMBOL_GPL (pwm_enable);
include/linux/pwm.h +2 -0
@@ -3,6 +3,7 @@#include <linux/err.h> #include <linux/of.h> +#include <linux/mutex.h> struct pwm_device ;struct seq_file ;
@@ -98,6 +99,7 @@ struct pwm_device {unsigned int pwm;struct pwm_chip *chip;void *chip_data; +struct mutex lock;unsigned int period;unsigned int duty_cycle;
Signature · 1 line
1.9.1
Thierry Reding
AppliedRe: [PATCH v10 1/1] pwm: core: Set enable state properly on failed call to enable
Quoted 5 lines · Jonathan Richardson
The
pwm_enablefunction didn't clear the enabled bit if a call to a clients enable function returned an error. The result was that the state of the pwm core was wrong. Clearing the bit when enable returns an error ensures the state is properly set.
Quoted 7 lines
Tested-by Jonathan Richardson <[email protected]>Reviewed-by Dmitry Torokhov <[email protected]>Signed-off-by Jonathan Richardson <[email protected]>--- drivers/pwm/core.c | 33 ++++++++++++++++++++++++++------- include/linux/pwm.h | 2 ++ 2 files changed, 28 insertions(+), 7 deletions(-)
Applied, thanks.
Thierry