[PATCH 0/8] pwm: Drop local locking in several drivers

[PATCH 0/8] pwm: Drop local locking in several drivers

[Uwe Kleine-König]

Hello,

some time ago the pwm core implemented additional locking to protect
lowlevel driver callbacks against driver removal. A side effect is that
.apply() and .get_state() are serialized. This allows to drop some
locking that is now superfluous due to the core's locking.

I identified a few drivers that are affected; these are cleaned up
accordingly here.

Best regards
Uwe

Uwe Kleine-König (8):
  pwm: atmel: Drop driver local locking
  pwm: clps711x: Drop driver local locking
  pwm: fsl-ftm: Drop driver local locking
  pwm: lpc18xx-sct: Drop driver local locking
  pwm: microchip-core: Drop driver local locking
  pwm: sti: Drop driver local locking
  pwm: sun4i: Drop driver local locking
  pwm: twl-led: Drop driver local locking

 drivers/pwm/pwm-atmel.c          | 12 --------
 drivers/pwm/pwm-clps711x.c       |  8 ------
 drivers/pwm/pwm-fsl-ftm.c        | 28 +++++-------------
 drivers/pwm/pwm-lpc18xx-sct.c    | 14 ---------
 drivers/pwm/pwm-microchip-core.c | 17 +----------
 drivers/pwm/pwm-sti.c            | 23 ++++-----------
 drivers/pwm/pwm-sun4i.c          | 10 -------
 drivers/pwm/pwm-twl-led.c        | 49 +++++---------------------------
 8 files changed, 21 insertions(+), 140 deletions(-)

base-commit: f817b6dd2b62d921a6cdc0a3ac599cd1851f343c
-- 
2.49.0

[PATCH 1/8] pwm: atmel: Drop driver local locking

[Uwe Kleine-König]

The two functions making use of the lock are only called transitively from
.apply(). Calls to .apply() are already serialized by the pwm core so the
lock in the driver has no effect and can safely be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-atmel.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index b2f0abbbad63..06d22d0f7b26 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -91,9 +91,6 @@ struct atmel_pwm_chip {
 	 * hardware.
 	 */
 	u32 update_pending;
-
-	/* Protects .update_pending */
-	spinlock_t lock;
 };
 
 static inline struct atmel_pwm_chip *to_atmel_pwm_chip(struct pwm_chip *chip)
@@ -145,8 +142,6 @@ static void atmel_pwm_update_pending(struct atmel_pwm_chip *chip)
 
 static void atmel_pwm_set_pending(struct atmel_pwm_chip *chip, unsigned int ch)
 {
-	spin_lock(&chip->lock);
-
 	/*
 	 * Clear pending flags in hardware because otherwise there might still
 	 * be a stale flag in ISR.
@@ -154,16 +149,12 @@ static void atmel_pwm_set_pending(struct atmel_pwm_chip *chip, unsigned int ch)
 	atmel_pwm_update_pending(chip);
 
 	chip->update_pending |= (1 << ch);
-
-	spin_unlock(&chip->lock);
 }
 
 static int atmel_pwm_test_pending(struct atmel_pwm_chip *chip, unsigned int ch)
 {
 	int ret = 0;
 
-	spin_lock(&chip->lock);
-
 	if (chip->update_pending & (1 << ch)) {
 		atmel_pwm_update_pending(chip);
 
@@ -171,8 +162,6 @@ static int atmel_pwm_test_pending(struct atmel_pwm_chip *chip, unsigned int ch)
 			ret = 1;
 	}
 
-	spin_unlock(&chip->lock);
-
 	return ret;
 }
 
@@ -509,7 +498,6 @@ static int atmel_pwm_probe(struct platform_device *pdev)
 	atmel_pwm->data = of_device_get_match_data(&pdev->dev);
 
 	atmel_pwm->update_pending = 0;
-	spin_lock_init(&atmel_pwm->lock);
 
 	atmel_pwm->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(atmel_pwm->base))
-- 
2.49.0

[PATCH 2/8] pwm: clps711x: Drop driver local locking

[Uwe Kleine-König]

The pwm core serializes calls to .apply(), so the spinlock adds no
additional protection. Disabling the irq is also irrelevant as the driver
isn't an atomic one and so the callbacks cannot be called from atomic
context.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-clps711x.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 04559a9de718..2c92ce754872 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -14,7 +14,6 @@
 struct clps711x_chip {
 	void __iomem *pmpcon;
 	struct clk *clk;
-	spinlock_t lock;
 };
 
 static inline struct clps711x_chip *to_clps711x_chip(struct pwm_chip *chip)
@@ -42,7 +41,6 @@ static int clps711x_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct clps711x_chip *priv = to_clps711x_chip(chip);
 	/* PWM0 - bits 4..7, PWM1 - bits 8..11 */
 	u32 shift = (pwm->hwpwm + 1) * 4;
-	unsigned long flags;
 	u32 pmpcon, val;
 
 	if (state->polarity != PWM_POLARITY_NORMAL)
@@ -56,15 +54,11 @@ static int clps711x_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	else
 		val = 0;
 
-	spin_lock_irqsave(&priv->lock, flags);
-
 	pmpcon = readl(priv->pmpcon);
 	pmpcon &= ~(0xf << shift);
 	pmpcon |= val << shift;
 	writel(pmpcon, priv->pmpcon);
 
-	spin_unlock_irqrestore(&priv->lock, flags);
-
 	return 0;
 }
 
@@ -93,8 +87,6 @@ static int clps711x_pwm_probe(struct platform_device *pdev)
 
 	chip->ops = &clps711x_pwm_ops;
 
-	spin_lock_init(&priv->lock);
-
 	return devm_pwmchip_add(&pdev->dev, chip);
 }
 
-- 
2.49.0

[PATCH 3/8] pwm: fsl-ftm: Drop driver local locking

[Uwe Kleine-König]

The pwm core serializes calls to .apply(), so the driver local lock isn't
needed for that. It only has the effect to serialize .apply() with
.request() and .free() for a different PWM, and .request() and .free
against each other. But given that .request and .free() only do a single
regmap operation under the lock and regmap itself serializes register
accesses, it might happen without the lock that the calls are interleaved
now, but affecting different PWMs, so nothing bad can happen.

So the mutex has no effect and can be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-fsl-ftm.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index c45a5fca4cbb..6683931872fc 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -10,7 +10,6 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
@@ -40,7 +39,6 @@ struct fsl_pwm_periodcfg {
 };
 
 struct fsl_pwm_chip {
-	struct mutex lock;
 	struct regmap *regmap;
 
 	/* This value is valid iff a pwm is running */
@@ -89,11 +87,8 @@ static int fsl_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
 
 	ret = clk_prepare_enable(fpc->ipg_clk);
-	if (!ret && fpc->soc->has_enable_bits) {
-		mutex_lock(&fpc->lock);
+	if (!ret && fpc->soc->has_enable_bits)
 		regmap_set_bits(fpc->regmap, FTM_SC, BIT(pwm->hwpwm + 16));
-		mutex_unlock(&fpc->lock);
-	}
 
 	return ret;
 }
@@ -102,11 +97,8 @@ static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
 
-	if (fpc->soc->has_enable_bits) {
-		mutex_lock(&fpc->lock);
+	if (fpc->soc->has_enable_bits)
 		regmap_clear_bits(fpc->regmap, FTM_SC, BIT(pwm->hwpwm + 16));
-		mutex_unlock(&fpc->lock);
-	}
 
 	clk_disable_unprepare(fpc->ipg_clk);
 }
@@ -304,7 +296,7 @@ static int fsl_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 {
 	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
 	struct pwm_state *oldstate = &pwm->state;
-	int ret = 0;
+	int ret;
 
 	/*
 	 * oldstate to newstate : action
@@ -315,8 +307,6 @@ static int fsl_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * disabled to enabled : update settings + enable
 	 */
 
-	mutex_lock(&fpc->lock);
-
 	if (!newstate->enabled) {
 		if (oldstate->enabled) {
 			regmap_set_bits(fpc->regmap, FTM_OUTMASK,
@@ -325,30 +315,28 @@ static int fsl_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
 		}
 
-		goto end_mutex;
+		return 0;
 	}
 
 	ret = fsl_pwm_apply_config(chip, pwm, newstate);
 	if (ret)
-		goto end_mutex;
+		return ret;
 
 	/* check if need to enable */
 	if (!oldstate->enabled) {
 		ret = clk_prepare_enable(fpc->clk[fpc->period.clk_select]);
 		if (ret)
-			goto end_mutex;
+			return ret;
 
 		ret = clk_prepare_enable(fpc->clk[FSL_PWM_CLK_CNTEN]);
 		if (ret) {
 			clk_disable_unprepare(fpc->clk[fpc->period.clk_select]);
-			goto end_mutex;
+			return ret;
 		}
 
 		regmap_clear_bits(fpc->regmap, FTM_OUTMASK, BIT(pwm->hwpwm));
 	}
 
-end_mutex:
-	mutex_unlock(&fpc->lock);
 	return ret;
 }
 
@@ -408,8 +396,6 @@ static int fsl_pwm_probe(struct platform_device *pdev)
 		return PTR_ERR(chip);
 	fpc = to_fsl_chip(chip);
 
-	mutex_init(&fpc->lock);
-
 	fpc->soc = of_device_get_match_data(&pdev->dev);
 
 	base = devm_platform_ioremap_resource(pdev, 0);
-- 
2.49.0

[PATCH 4/8] pwm: lpc18xx-sct: Drop driver local locking

[Uwe Kleine-König]

Both mutexes are only used in one function each. These functions are only
called by the .apply() callback. As the .apply() calls are serialized by
the core since commit 1cc2e1faafb3 ("pwm: Add more locking") the mutexes
have no effect apart from runtime overhead. Drop them.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-lpc18xx-sct.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/pwm/pwm-lpc18xx-sct.c b/drivers/pwm/pwm-lpc18xx-sct.c
index f351baa63453..1e614b2a0227 100644
--- a/drivers/pwm/pwm-lpc18xx-sct.c
+++ b/drivers/pwm/pwm-lpc18xx-sct.c
@@ -100,8 +100,6 @@ struct lpc18xx_pwm_chip {
 	u64 max_period_ns;
 	unsigned int period_event;
 	unsigned long event_map;
-	struct mutex res_lock;
-	struct mutex period_lock;
 	struct lpc18xx_pwm_data channeldata[LPC18XX_NUM_PWMS];
 };
 
@@ -129,8 +127,6 @@ static void lpc18xx_pwm_set_conflict_res(struct lpc18xx_pwm_chip *lpc18xx_pwm,
 {
 	u32 val;
 
-	mutex_lock(&lpc18xx_pwm->res_lock);
-
 	/*
 	 * Simultaneous set and clear may happen on an output, that is the case
 	 * when duty_ns == period_ns. LPC18xx SCT allows to set a conflict
@@ -140,8 +136,6 @@ static void lpc18xx_pwm_set_conflict_res(struct lpc18xx_pwm_chip *lpc18xx_pwm,
 	val &= ~LPC18XX_PWM_RES_MASK(pwm->hwpwm);
 	val |= LPC18XX_PWM_RES(pwm->hwpwm, action);
 	lpc18xx_pwm_writel(lpc18xx_pwm, LPC18XX_PWM_RES_BASE, val);
-
-	mutex_unlock(&lpc18xx_pwm->res_lock);
 }
 
 static void lpc18xx_pwm_config_period(struct pwm_chip *chip, u64 period_ns)
@@ -200,8 +194,6 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return -ERANGE;
 	}
 
-	mutex_lock(&lpc18xx_pwm->period_lock);
-
 	requested_events = bitmap_weight(&lpc18xx_pwm->event_map,
 					 LPC18XX_PWM_EVENT_MAX);
 
@@ -214,7 +206,6 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	    lpc18xx_pwm->period_ns) {
 		dev_err(pwmchip_parent(chip), "conflicting period requested for PWM %u\n",
 			pwm->hwpwm);
-		mutex_unlock(&lpc18xx_pwm->period_lock);
 		return -EBUSY;
 	}
 
@@ -224,8 +215,6 @@ static int lpc18xx_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		lpc18xx_pwm_config_period(chip, period_ns);
 	}
 
-	mutex_unlock(&lpc18xx_pwm->period_lock);
-
 	lpc18xx_pwm_config_duty(chip, pwm, duty_ns);
 
 	return 0;
@@ -377,9 +366,6 @@ static int lpc18xx_pwm_probe(struct platform_device *pdev)
 	if (lpc18xx_pwm->clk_rate > NSEC_PER_SEC)
 		return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock to fast\n");
 
-	mutex_init(&lpc18xx_pwm->res_lock);
-	mutex_init(&lpc18xx_pwm->period_lock);
-
 	lpc18xx_pwm->max_period_ns =
 		mul_u64_u64_div_u64(NSEC_PER_SEC, LPC18XX_PWM_TIMER_MAX, lpc18xx_pwm->clk_rate);
 
-- 
2.49.0

Re: [PATCH 4/8] pwm: lpc18xx-sct: Drop driver local locking

[Vladimir Zapolskiy]

On 6/24/25 21:15, Uwe Kleine-König wrote:
> Both mutexes are only used in one function each. These functions are only
> called by the .apply() callback. As the .apply() calls are serialized by
> the core since commit 1cc2e1faafb3 ("pwm: Add more locking") the mutexes
> have no effect apart from runtime overhead. Drop them.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

Thank you for the change.

-- 
Best wishes,
Vladimir

[PATCH 5/8] pwm: microchip-core: Drop driver local locking

[Uwe Kleine-König]

The pwm core already serializes .apply() and .get_state(), so the driver
local lock is always free and adds no protection.

Drop it.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-microchip-core.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/pwm/pwm-microchip-core.c b/drivers/pwm/pwm-microchip-core.c
index 12821b4bbf97..4ff32bb4c205 100644
--- a/drivers/pwm/pwm-microchip-core.c
+++ b/drivers/pwm/pwm-microchip-core.c
@@ -36,7 +36,6 @@
 #include <linux/ktime.h>
 #include <linux/math.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
@@ -56,7 +55,6 @@
 struct mchp_core_pwm_chip {
 	struct clk *clk;
 	void __iomem *base;
-	struct mutex lock; /* protects the shared period */
 	ktime_t update_timestamp;
 	u32 sync_update_mask;
 	u16 channel_enabled;
@@ -360,17 +358,10 @@ static int mchp_core_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			       const struct pwm_state *state)
 {
 	struct mchp_core_pwm_chip *mchp_core_pwm = to_mchp_core_pwm(chip);
-	int ret;
-
-	mutex_lock(&mchp_core_pwm->lock);
 
 	mchp_core_pwm_wait_for_sync_update(mchp_core_pwm, pwm->hwpwm);
 
-	ret = mchp_core_pwm_apply_locked(chip, pwm, state);
-
-	mutex_unlock(&mchp_core_pwm->lock);
-
-	return ret;
+	return mchp_core_pwm_apply_locked(chip, pwm, state);
 }
 
 static int mchp_core_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -381,8 +372,6 @@ static int mchp_core_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm
 	u16 prescale, period_steps;
 	u8 duty_steps, posedge, negedge;
 
-	mutex_lock(&mchp_core_pwm->lock);
-
 	mchp_core_pwm_wait_for_sync_update(mchp_core_pwm, pwm->hwpwm);
 
 	if (mchp_core_pwm->channel_enabled & (1 << pwm->hwpwm))
@@ -415,8 +404,6 @@ static int mchp_core_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm
 	posedge = readb_relaxed(mchp_core_pwm->base + MCHPCOREPWM_POSEDGE(pwm->hwpwm));
 	negedge = readb_relaxed(mchp_core_pwm->base + MCHPCOREPWM_NEGEDGE(pwm->hwpwm));
 
-	mutex_unlock(&mchp_core_pwm->lock);
-
 	if (negedge == posedge) {
 		state->duty_cycle = state->period;
 		state->period *= 2;
@@ -469,8 +456,6 @@ static int mchp_core_pwm_probe(struct platform_device *pdev)
 				 &mchp_core_pwm->sync_update_mask))
 		mchp_core_pwm->sync_update_mask = 0;
 
-	mutex_init(&mchp_core_pwm->lock);
-
 	chip->ops = &mchp_core_pwm_ops;
 
 	mchp_core_pwm->channel_enabled = readb_relaxed(mchp_core_pwm->base + MCHPCOREPWM_EN(0));
-- 
2.49.0

[PATCH 6/8] pwm: sti: Drop driver local locking

[Uwe Kleine-König]

The pwm core already serializes calls to .apply(), so the driver local
mutex adds no protection and can be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-sti.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
index 396b52672ce0..3b702b8f0c7f 100644
--- a/drivers/pwm/pwm-sti.c
+++ b/drivers/pwm/pwm-sti.c
@@ -92,7 +92,6 @@ struct sti_pwm_chip {
 	struct pwm_device *cur;
 	unsigned long configured;
 	unsigned int en_count;
-	struct mutex sti_pwm_lock; /* To sync between enable/disable calls */
 	void __iomem *mmio;
 };
 
@@ -244,55 +243,46 @@ static int sti_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct sti_pwm_chip *pc = to_sti_pwmchip(chip);
 	struct device *dev = pc->dev;
-	int ret = 0;
+	int ret;
 
 	/*
 	 * Since we have a common enable for all PWM devices, do not enable if
 	 * already enabled.
 	 */
-	mutex_lock(&pc->sti_pwm_lock);
 
 	if (!pc->en_count) {
 		ret = clk_enable(pc->pwm_clk);
 		if (ret)
-			goto out;
+			return ret;
 
 		ret = clk_enable(pc->cpt_clk);
 		if (ret)
-			goto out;
+			return ret;
 
 		ret = regmap_field_write(pc->pwm_out_en, 1);
 		if (ret) {
 			dev_err(dev, "failed to enable PWM device %u: %d\n",
 				pwm->hwpwm, ret);
-			goto out;
+			return ret;
 		}
 	}
 
 	pc->en_count++;
 
-out:
-	mutex_unlock(&pc->sti_pwm_lock);
-	return ret;
+	return 0;
 }
 
 static void sti_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct sti_pwm_chip *pc = to_sti_pwmchip(chip);
 
-	mutex_lock(&pc->sti_pwm_lock);
-
-	if (--pc->en_count) {
-		mutex_unlock(&pc->sti_pwm_lock);
+	if (--pc->en_count)
 		return;
-	}
 
 	regmap_field_write(pc->pwm_out_en, 0);
 
 	clk_disable(pc->pwm_clk);
 	clk_disable(pc->cpt_clk);
-
-	mutex_unlock(&pc->sti_pwm_lock);
 }
 
 static void sti_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -594,7 +584,6 @@ static int sti_pwm_probe(struct platform_device *pdev)
 
 	pc->dev = dev;
 	pc->en_count = 0;
-	mutex_init(&pc->sti_pwm_lock);
 
 	ret = sti_pwm_probe_regmap(pc);
 	if (ret)
-- 
2.49.0

[PATCH 7/8] pwm: sun4i: Drop driver local locking

[Uwe Kleine-König]

The pwm core serializes calls to .apply(), so the driver lock doesn't
add any protection and can safely be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-sun4i.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
index e60dc7d6b591..6c5591ca868b 100644
--- a/drivers/pwm/pwm-sun4i.c
+++ b/drivers/pwm/pwm-sun4i.c
@@ -21,7 +21,6 @@
 #include <linux/pwm.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
-#include <linux/spinlock.h>
 #include <linux/time.h>
 
 #define PWM_CTRL_REG		0x0
@@ -85,7 +84,6 @@ struct sun4i_pwm_chip {
 	struct clk *clk;
 	struct reset_control *rst;
 	void __iomem *base;
-	spinlock_t ctrl_lock;
 	const struct sun4i_pwm_data *data;
 };
 
@@ -258,7 +256,6 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		return ret;
 	}
 
-	spin_lock(&sun4ichip->ctrl_lock);
 	ctrl = sun4i_pwm_readl(sun4ichip, PWM_CTRL_REG);
 
 	if (sun4ichip->data->has_direct_mod_clk_output) {
@@ -266,7 +263,6 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			ctrl |= BIT_CH(PWM_BYPASS, pwm->hwpwm);
 			/* We can skip other parameter */
 			sun4i_pwm_writel(sun4ichip, ctrl, PWM_CTRL_REG);
-			spin_unlock(&sun4ichip->ctrl_lock);
 			return 0;
 		}
 
@@ -297,8 +293,6 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	sun4i_pwm_writel(sun4ichip, ctrl, PWM_CTRL_REG);
 
-	spin_unlock(&sun4ichip->ctrl_lock);
-
 	if (state->enabled)
 		return 0;
 
@@ -309,12 +303,10 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	else
 		usleep_range(delay_us, delay_us * 2);
 
-	spin_lock(&sun4ichip->ctrl_lock);
 	ctrl = sun4i_pwm_readl(sun4ichip, PWM_CTRL_REG);
 	ctrl &= ~BIT_CH(PWM_CLK_GATING, pwm->hwpwm);
 	ctrl &= ~BIT_CH(PWM_EN, pwm->hwpwm);
 	sun4i_pwm_writel(sun4ichip, ctrl, PWM_CTRL_REG);
-	spin_unlock(&sun4ichip->ctrl_lock);
 
 	clk_disable_unprepare(sun4ichip->clk);
 
@@ -456,8 +448,6 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
 
 	chip->ops = &sun4i_pwm_ops;
 
-	spin_lock_init(&sun4ichip->ctrl_lock);
-
 	ret = pwmchip_add(chip);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
-- 
2.49.0

[PATCH 8/8] pwm: twl-led: Drop driver local locking

[Uwe Kleine-König]

The pwm core already serializes .apply(). twl6030's .request() and .free()
are also already serialized against .apply() because there is only a single
PWM. So the mutex doesn't add any additional protection and can be dropped.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/pwm/pwm-twl-led.c | 49 ++++++---------------------------------
 1 file changed, 7 insertions(+), 42 deletions(-)

diff --git a/drivers/pwm/pwm-twl-led.c b/drivers/pwm/pwm-twl-led.c
index 4b10a8dab312..a555cc3be4b3 100644
--- a/drivers/pwm/pwm-twl-led.c
+++ b/drivers/pwm/pwm-twl-led.c
@@ -61,10 +61,6 @@
 #define TWL6040_LED_MODE_OFF	0x02
 #define TWL6040_LED_MODE_MASK	0x03
 
-struct twl_pwmled_chip {
-	struct mutex mutex;
-};
-
 static inline struct twl_pwmled_chip *to_twl(struct pwm_chip *chip)
 {
 	return pwmchip_get_drvdata(chip);
@@ -106,15 +102,13 @@ static int twl4030_pwmled_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int twl4030_pwmled_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL4030_MODULE_LED, &val, TWL4030_LEDEN_REG);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read LEDEN\n", pwm->label);
-		goto out;
+		return ret;
 	}
 
 	val |= TWL4030_LED_TOGGLE(pwm->hwpwm, TWL4030_LED_PINS);
@@ -123,23 +117,19 @@ static int twl4030_pwmled_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to enable PWM\n", pwm->label);
 
-out:
-	mutex_unlock(&twl->mutex);
 	return ret;
 }
 
 static void twl4030_pwmled_disable(struct pwm_chip *chip,
 				   struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL4030_MODULE_LED, &val, TWL4030_LEDEN_REG);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read LEDEN\n", pwm->label);
-		goto out;
+		return;
 	}
 
 	val &= ~TWL4030_LED_TOGGLE(pwm->hwpwm, TWL4030_LED_PINS);
@@ -147,9 +137,6 @@ static void twl4030_pwmled_disable(struct pwm_chip *chip,
 	ret = twl_i2c_write_u8(TWL4030_MODULE_LED, val, TWL4030_LEDEN_REG);
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to disable PWM\n", pwm->label);
-
-out:
-	mutex_unlock(&twl->mutex);
 }
 
 static int twl4030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -209,16 +196,14 @@ static int twl6030_pwmled_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int twl6030_pwmled_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read PWM_CTRL2\n",
 			pwm->label);
-		goto out;
+		return ret;
 	}
 
 	val &= ~TWL6040_LED_MODE_MASK;
@@ -228,24 +213,20 @@ static int twl6030_pwmled_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to enable PWM\n", pwm->label);
 
-out:
-	mutex_unlock(&twl->mutex);
 	return ret;
 }
 
 static void twl6030_pwmled_disable(struct pwm_chip *chip,
 				   struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read PWM_CTRL2\n",
 			pwm->label);
-		goto out;
+		return;
 	}
 
 	val &= ~TWL6040_LED_MODE_MASK;
@@ -254,9 +235,6 @@ static void twl6030_pwmled_disable(struct pwm_chip *chip,
 	ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to disable PWM\n", pwm->label);
-
-out:
-	mutex_unlock(&twl->mutex);
 }
 
 static int twl6030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -287,16 +265,14 @@ static int twl6030_pwmled_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int twl6030_pwmled_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read PWM_CTRL2\n",
 			pwm->label);
-		goto out;
+		return ret;
 	}
 
 	val &= ~TWL6040_LED_MODE_MASK;
@@ -306,23 +282,19 @@ static int twl6030_pwmled_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to request PWM\n", pwm->label);
 
-out:
-	mutex_unlock(&twl->mutex);
 	return ret;
 }
 
 static void twl6030_pwmled_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct twl_pwmled_chip *twl = to_twl(chip);
 	int ret;
 	u8 val;
 
-	mutex_lock(&twl->mutex);
 	ret = twl_i2c_read_u8(TWL6030_MODULE_ID1, &val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0) {
 		dev_err(pwmchip_parent(chip), "%s: Failed to read PWM_CTRL2\n",
 			pwm->label);
-		goto out;
+		return;
 	}
 
 	val &= ~TWL6040_LED_MODE_MASK;
@@ -331,9 +303,6 @@ static void twl6030_pwmled_free(struct pwm_chip *chip, struct pwm_device *pwm)
 	ret = twl_i2c_write_u8(TWL6030_MODULE_ID1, val, TWL6030_LED_PWM_CTRL2);
 	if (ret < 0)
 		dev_err(pwmchip_parent(chip), "%s: Failed to free PWM\n", pwm->label);
-
-out:
-	mutex_unlock(&twl->mutex);
 }
 
 static const struct pwm_ops twl6030_pwmled_ops = {
@@ -345,7 +314,6 @@ static const struct pwm_ops twl6030_pwmled_ops = {
 static int twl_pwmled_probe(struct platform_device *pdev)
 {
 	struct pwm_chip *chip;
-	struct twl_pwmled_chip *twl;
 	unsigned int npwm;
 	const struct pwm_ops *ops;
 
@@ -357,15 +325,12 @@ static int twl_pwmled_probe(struct platform_device *pdev)
 		npwm = 1;
 	}
 
-	chip = devm_pwmchip_alloc(&pdev->dev, npwm, sizeof(*twl));
+	chip = devm_pwmchip_alloc(&pdev->dev, npwm, 0);
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
-	twl = to_twl(chip);
 
 	chip->ops = ops;
 
-	mutex_init(&twl->mutex);
-
 	return devm_pwmchip_add(&pdev->dev, chip);
 }
 
-- 
2.49.0

Re: [PATCH 0/8] pwm: Drop local locking in several drivers

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

Hello,

On Tue, Jun 24, 2025 at 08:15:36PM +0200, Uwe Kleine-König wrote:
> some time ago the pwm core implemented additional locking to protect
> lowlevel driver callbacks against driver removal. A side effect is that
> .apply() and .get_state() are serialized. This allows to drop some
> locking that is now superfluous due to the core's locking.
> 
> I identified a few drivers that are affected; these are cleaned up
> accordingly here.

I applied these patches to
https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next
now.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/8] pwm: Drop local locking in several drivers

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Uwe Kleine-König <ukleinek@kernel.org>:

On Tue, 24 Jun 2025 20:15:36 +0200 you wrote:
> Hello,
> 
> some time ago the pwm core implemented additional locking to protect
> lowlevel driver callbacks against driver removal. A side effect is that
> .apply() and .get_state() are serialized. This allows to drop some
> locking that is now superfluous due to the core's locking.
> 
> [...]

Here is the summary with links:
  - [5/8] pwm: microchip-core: Drop driver local locking
    https://git.kernel.org/riscv/c/9470e7d11fe2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html