[PATCH] pwm: atmel-tcb: Fix sleeping function called from invalid context

[PATCH] pwm: atmel-tcb: Fix sleeping function called from invalid context

[Sangyun Kim]

atmel_tcb_pwm_apply() holds tcbpwmc->lock as a spinlock via
guard(spinlock)() and then calls atmel_tcb_pwm_config(), which calls
clk_get_rate() twice. clk_get_rate() acquires clk_prepare_lock (a
mutex), so this is a sleep-in-atomic-context violation.

On CONFIG_DEBUG_ATOMIC_SLEEP kernels every pwm_apply_state() that
enables or reconfigures the PWM triggers a "BUG: sleeping function
called from invalid context" warning.

All callers of tcbpwmc->lock (the .request and .apply callbacks) run in
process context and only need mutual exclusion against each other, so
use a mutex instead of a spinlock and allow the sleeping calls inside
atmel_tcb_pwm_config().

Fixes: 37f7707077f5 ("pwm: atmel-tcb: Fix race condition and convert to guards")
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
---
 drivers/pwm/pwm-atmel-tcb.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index f9ff78ba122d..6405e82d9f10 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -17,6 +17,7 @@
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 #include <linux/of.h>
@@ -47,7 +48,7 @@ struct atmel_tcb_channel {
 };
 
 struct atmel_tcb_pwm_chip {
-	spinlock_t lock;
+	struct mutex lock;
 	u8 channel;
 	u8 width;
 	struct regmap *regmap;
@@ -81,7 +82,7 @@ static int atmel_tcb_pwm_request(struct pwm_chip *chip,
 	tcbpwm->period = 0;
 	tcbpwm->div = 0;
 
-	guard(spinlock)(&tcbpwmc->lock);
+	guard(mutex)(&tcbpwmc->lock);
 
 	regmap_read(tcbpwmc->regmap, ATMEL_TC_REG(tcbpwmc->channel, CMR), &cmr);
 	/*
@@ -335,7 +336,7 @@ static int atmel_tcb_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	int duty_cycle, period;
 	int ret;
 
-	guard(spinlock)(&tcbpwmc->lock);
+	guard(mutex)(&tcbpwmc->lock);
 
 	if (!state->enabled) {
 		atmel_tcb_pwm_disable(chip, pwm, state->polarity);
@@ -438,7 +439,7 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	if (err)
 		goto err_gclk;
 
-	spin_lock_init(&tcbpwmc->lock);
+	mutex_init(&tcbpwmc->lock);
 
 	err = pwmchip_add(chip);
 	if (err < 0)
-- 
2.34.1

Re: [PATCH] pwm: atmel-tcb: Fix sleeping function called from invalid context

[Uwe Kleine-König]

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

Hello Sangyun (I hope this is the right part of your name to address
you, feel free to tell me, when I'm wrong),

On Wed, Apr 15, 2026 at 06:34:33PM +0900, Sangyun Kim wrote:
> atmel_tcb_pwm_apply() holds tcbpwmc->lock as a spinlock via
> guard(spinlock)() and then calls atmel_tcb_pwm_config(), which calls
> clk_get_rate() twice. clk_get_rate() acquires clk_prepare_lock (a
> mutex), so this is a sleep-in-atomic-context violation.
> 
> On CONFIG_DEBUG_ATOMIC_SLEEP kernels every pwm_apply_state() that
> enables or reconfigures the PWM triggers a "BUG: sleeping function
> called from invalid context" warning.
> 
> All callers of tcbpwmc->lock (the .request and .apply callbacks) run in
> process context and only need mutual exclusion against each other, so
> use a mutex instead of a spinlock and allow the sleeping calls inside
> atmel_tcb_pwm_config().
> 
> Fixes: 37f7707077f5 ("pwm: atmel-tcb: Fix race condition and convert to guards")
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>

The issue is real. I first thought the lock isn't needed at all and can
better be dropped, but the chip lock doesn't cover .request().

It would be great if you could rework the patch to keep the spinlock and
instead make use of clk_rate_exclusive_get() at probe time and then
store the rate in struct atmel_tcb_pwm_chip.

Or alternatively drop the lock and call guard(pwmchip)(chip) in
.request(). (This however would require to move the GUARD definition to
a header.)

Without the mutex we could then do:

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index f9ff78ba122d..70856be12517 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -431,6 +431,7 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	}
 
 	chip->ops = &atmel_tcb_pwm_ops;
+	chip->atomic = true;
 	tcbpwmc->channel = channel;
 	tcbpwmc->width = config->counter_width;
 
which is nice for some usages.

Best regards
Uwe

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

[PATCH v2] pwm: atmel-tcb: Cache clock rates and mark chip as atomic

[Sangyun Kim]

atmel_tcb_pwm_apply() holds tcbpwmc->lock as a spinlock via
guard(spinlock)() and then calls atmel_tcb_pwm_config(), which calls
clk_get_rate() twice. clk_get_rate() acquires clk_prepare_lock (a
mutex), so this is a sleep-in-atomic-context violation.

On CONFIG_DEBUG_ATOMIC_SLEEP kernels every pwm_apply_state() that
enables or reconfigures the PWM triggers a "BUG: sleeping function
called from invalid context" warning.

Acquire exclusive control over the clock rates with
clk_rate_exclusive_get() at probe time and cache the rates in struct
atmel_tcb_pwm_chip, then read the cached rates from
atmel_tcb_pwm_config(). This keeps the spinlock-based mutual exclusion
introduced in commit 37f7707077f5 ("pwm: atmel-tcb: Fix race condition
and convert to guards") and removes the sleeping calls from the atomic
section.

With no sleeping calls left in .apply() and the regmap-mmio bus already
running with fast_io=true, also mark the chip as atomic so consumers
can use pwm_apply_atomic() from atomic context.

Fixes: 37f7707077f5 ("pwm: atmel-tcb: Fix race condition and convert to guards")
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
---
Hi Uwe,

Thanks for the review! "Sangyun" is the right form to address me, no
worries.

Changes in v2:
 - Keep the spinlock instead of converting tcbpwmc->lock to a mutex.
 - Cache clk and slow_clk rates at probe via clk_rate_exclusive_get()
   so the .apply() path no longer calls clk_get_rate() under the
   spinlock.
 - Mark the chip as atomic now that .apply() has no sleeping calls.

Thanks,
Sangyun

 drivers/pwm/pwm-atmel-tcb.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index f9a9c12cbcdd..8d46ce28f736 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -50,6 +50,8 @@ struct atmel_tcb_pwm_chip {
 	spinlock_t lock;
 	u8 channel;
 	u8 width;
+	unsigned long rate;
+	unsigned long slow_rate;
 	struct regmap *regmap;
 	struct clk *clk;
 	struct clk *gclk;
@@ -266,7 +268,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	int slowclk = 0;
 	unsigned period;
 	unsigned duty;
-	unsigned rate = clk_get_rate(tcbpwmc->clk);
+	unsigned long rate = tcbpwmc->rate;
 	unsigned long long min;
 	unsigned long long max;
 
@@ -294,7 +296,7 @@ static int atmel_tcb_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 */
 	if (i == ARRAY_SIZE(atmel_tcb_divisors)) {
 		i = slowclk;
-		rate = clk_get_rate(tcbpwmc->slow_clk);
+		rate = tcbpwmc->slow_rate;
 		min = div_u64(NSEC_PER_SEC, rate);
 		max = min << tcbpwmc->width;
 
@@ -431,6 +433,7 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	}
 
 	chip->ops = &atmel_tcb_pwm_ops;
+	chip->atomic = true;
 	tcbpwmc->channel = channel;
 	tcbpwmc->width = config->counter_width;
 
@@ -438,16 +441,33 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	if (err)
 		goto err_gclk;
 
+	err = clk_rate_exclusive_get(tcbpwmc->clk);
+	if (err)
+		goto err_disable_clk;
+
+	err = clk_rate_exclusive_get(tcbpwmc->slow_clk);
+	if (err)
+		goto err_clk_unlock;
+
+	tcbpwmc->rate = clk_get_rate(tcbpwmc->clk);
+	tcbpwmc->slow_rate = clk_get_rate(tcbpwmc->slow_clk);
+
 	spin_lock_init(&tcbpwmc->lock);
 
 	err = pwmchip_add(chip);
 	if (err < 0)
-		goto err_disable_clk;
+		goto err_slow_clk_unlock;
 
 	platform_set_drvdata(pdev, chip);
 
 	return 0;
 
+err_slow_clk_unlock:
+	clk_rate_exclusive_put(tcbpwmc->slow_clk);
+
+err_clk_unlock:
+	clk_rate_exclusive_put(tcbpwmc->clk);
+
 err_disable_clk:
 	clk_disable_unprepare(tcbpwmc->slow_clk);
 
@@ -470,6 +490,8 @@ static void atmel_tcb_pwm_remove(struct platform_device *pdev)
 
 	pwmchip_remove(chip);
 
+	clk_rate_exclusive_put(tcbpwmc->slow_clk);
+	clk_rate_exclusive_put(tcbpwmc->clk);
 	clk_disable_unprepare(tcbpwmc->slow_clk);
 	clk_put(tcbpwmc->gclk);
 	clk_put(tcbpwmc->clk);
-- 
2.34.1