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

["Uwe Kleine-König" <ukleinek@kernel.org>]

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