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

Hello Sangyun,

On Sun, Apr 19, 2026 at 05:08:38PM +0900, Sangyun Kim wrote:
> @@ -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);
> +

Only one concern left: clk_get_rate() should only be called on enabled
clocks. I don't know the architecture details and how expensive it is to
have .clk enabled (or if it's enabled anyhow).

If you're ok, I'd squash the following diff into your patch:

diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 1a2832f1ace2..3d30aeab507e 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -437,13 +437,17 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	tcbpwmc->channel = channel;
 	tcbpwmc->width = config->counter_width;
 
-	err = clk_prepare_enable(tcbpwmc->slow_clk);
+	err = clk_prepare_enable(tcbpwmc->clk);
 	if (err)
 		goto err_gclk;
 
+	err = clk_prepare_enable(tcbpwmc->slow_clk);
+	if (err)
+		goto err_disable_clk;;
+
 	err = clk_rate_exclusive_get(tcbpwmc->clk);
 	if (err)
-		goto err_disable_clk;
+		goto err_disable_slow_clk;
 
 	err = clk_rate_exclusive_get(tcbpwmc->slow_clk);
 	if (err)
@@ -469,6 +473,9 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
 	clk_rate_exclusive_put(tcbpwmc->clk);
 
 err_disable_clk:
+	clk_disable_unprepare(tcbpwmc->clk);
+
+err_disable_slow_clk:
 	clk_disable_unprepare(tcbpwmc->slow_clk);
 
 err_gclk:
@@ -492,6 +499,7 @@ static void atmel_tcb_pwm_remove(struct platform_device *pdev)
 
 	clk_rate_exclusive_put(tcbpwmc->slow_clk);
 	clk_rate_exclusive_put(tcbpwmc->clk);
+	clk_disable_unprepare(tcbpwmc->clk);
 	clk_disable_unprepare(tcbpwmc->slow_clk);
 	clk_put(tcbpwmc->gclk);
 	clk_put(tcbpwmc->clk);


This has the downside that clk is kept enabled the whole driver
lifetime, but that's the easiest way to make your fix honor the clk API
constraints. This allows to fast-track the patch fixing the sleeping
function called from invalid context issue and the optimisation can then
be addressed with more time during the next development cycles.

Best regards
Uwe

On Tue, Apr 21, 2026 at 01:40:55 PM +0200, Uwe Kleine-König wrote:
>Hello Sangyun,

Hi Uwe,
Thanks for the review.

>
>On Sun, Apr 19, 2026 at 05:08:38PM +0900, Sangyun Kim wrote:
>> @@ -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);
>> +
>
>Only one concern left: clk_get_rate() should only be called on enabled
>clocks. I don't know the architecture details and how expensive it is to
>have .clk enabled (or if it's enabled anyhow).
>
>If you're ok, I'd squash the following diff into your patch:

That makes sense. clk_get_rate() should indeed only be used on enabled
clocks, and your change is the simplest way to ensure correctness while
respecting the clk API constraints. I’m happy with squashing your diff
into my patch.

>
>diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
>index 1a2832f1ace2..3d30aeab507e 100644
>--- a/drivers/pwm/pwm-atmel-tcb.c
>+++ b/drivers/pwm/pwm-atmel-tcb.c
>@@ -437,13 +437,17 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
> 	tcbpwmc->channel = channel;
> 	tcbpwmc->width = config->counter_width;
>
>-	err = clk_prepare_enable(tcbpwmc->slow_clk);
>+	err = clk_prepare_enable(tcbpwmc->clk);
> 	if (err)
> 		goto err_gclk;
>
>+	err = clk_prepare_enable(tcbpwmc->slow_clk);
>+	if (err)
>+		goto err_disable_clk;;
>+
> 	err = clk_rate_exclusive_get(tcbpwmc->clk);
> 	if (err)
>-		goto err_disable_clk;
>+		goto err_disable_slow_clk;
>
> 	err = clk_rate_exclusive_get(tcbpwmc->slow_clk);
> 	if (err)
>@@ -469,6 +473,9 @@ static int atmel_tcb_pwm_probe(struct platform_device *pdev)
> 	clk_rate_exclusive_put(tcbpwmc->clk);
>
> err_disable_clk:
>+	clk_disable_unprepare(tcbpwmc->clk);
>+
>+err_disable_slow_clk:
> 	clk_disable_unprepare(tcbpwmc->slow_clk);
>
> err_gclk:
>@@ -492,6 +499,7 @@ static void atmel_tcb_pwm_remove(struct platform_device *pdev)
>
> 	clk_rate_exclusive_put(tcbpwmc->slow_clk);
> 	clk_rate_exclusive_put(tcbpwmc->clk);
>+	clk_disable_unprepare(tcbpwmc->clk);
> 	clk_disable_unprepare(tcbpwmc->slow_clk);
> 	clk_put(tcbpwmc->gclk);
> 	clk_put(tcbpwmc->clk);
>
>
>This has the downside that clk is kept enabled the whole driver
>lifetime, but that's the easiest way to make your fix honor the clk API
>constraints. This allows to fast-track the patch fixing the sleeping
>function called from invalid context issue and the optimisation can then
>be addressed with more time during the next development cycles.

Keeping the clock enabled for the driver lifetime is acceptable for now
to fast-track the fix, and we can revisit potential optimizations in a
follow-up patch.

Thanks again for the suggestion.

>
>Best regards
>Uwe

Best regards,
Sangyun

Hello Sangyun,

On Wed, Apr 22, 2026 at 01:49:40PM +0900, Sangyun Kim wrote:
> > On Sun, Apr 19, 2026 at 05:08:38PM +0900, Sangyun Kim wrote:
> > > @@ -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);
> > > +
> > 
> > Only one concern left: clk_get_rate() should only be called on enabled
> > clocks. I don't know the architecture details and how expensive it is to
> > have .clk enabled (or if it's enabled anyhow).
> > 
> > If you're ok, I'd squash the following diff into your patch:
> 
> That makes sense. clk_get_rate() should indeed only be used on enabled
> clocks, and your change is the simplest way to ensure correctness while
> respecting the clk API constraints. I’m happy with squashing your diff
> into my patch.

Just for the record: I did that, the result is already in Linus' tree
at https://git.kernel.org/linus/68637b68afcc3cb4d56aca14a3a1d1b47b879369
and will be part of v7.1-rc1.

Best regards
Uwe