[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 --]