Skip to contentSkip to conversation

Fix bugs in kona pwm driver and pwm core

PATCH Applied Jonathan Richardson 2 participants 3 messages Oct 17, 2015 – Nov 6, 2015 PWM

lore
Share & tools
lore
#0 lore raw

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
#1 lore raw

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
index 3f9df3e..b8f6c30 100644
@@ -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
index d681f68..dee7d65 100644
@@ -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
#2 lore raw

Re: [PATCH v10 1/1] pwm: core: Set enable state properly on failed call to enable

Quoted 5 lines · Jonathan Richardson

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.

Quoted 7 lines
---
 drivers/pwm/core.c  | 33 ++++++++++++++++++++++++++-------
 include/linux/pwm.h |  2 ++
 2 files changed, 28 insertions(+), 7 deletions(-)

Applied, thanks.

Thierry