* [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more
@ 2026-10-16 16:12 Karl Mehltretter
2026-10-22 8:29 ` Uwe Kleine-König
2026-10-27 2:27 ` [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods Karl Mehltretter
0 siblings, 2 replies; 5+ messages in thread
From: Karl Mehltretter @ 2026-10-16 16:12 UTC (permalink / raw)
To: Uwe Kleine-König, Claudiu Beznea
Cc: Karl Mehltretter, Nicolas Ferre, Alexandre Belloni, linux-pwm,
linux-arm-kernel, linux-kernel
atmel_pwm_calculate_cprd_and_pres() keeps the period in clock cycles in
an unsigned long long but derives the prescaler from fls(cycles). fls()
takes an unsigned int, so the upper 32 bits are ignored.
On controllers that the driver configures with a 32-bit period register
(SAM9X60, SAM9X7) a request of 2^32 clock cycles or more therefore gets
prescaler 0 and a truncated CPRD. The request succeeds and sysfs
reports the requested period.
On a SAM9X75 (PWM clock 266.67 MHz) this affects periods from about
16.1 s on. Requesting 20 s with a 10 s duty cycle
# echo 2 > /sys/class/pwm/pwmchip0/export
# echo 20000000000 > /sys/class/pwm/pwmchip0/pwm2/period
# echo 10000000000 > /sys/class/pwm/pwmchip0/pwm2/duty_cycle
# echo 1 > /sys/class/pwm/pwmchip0/pwm2/enable
programs CPRE=0, CPRD=0x3de4355c and CDTY=0x9ef21aae. The channel runs
with a period of 3.9 s, measured from the period end flag in PWM_ISR.
With this duty cycle CDTY is above CPRD, so the output stays inactive
and an LED on it stays dark.
Use fls64() so that the prescaler is derived from the full 64-bit
value. The driver then programs CPRE=1, CPRD=0x9ef21aae and
CDTY=0x4f790d57. The measured period is 20.0 s and the LED is on for
10 s of every 20 s.
Controllers configured with a 16-bit period register are unaffected
for all representable periods. Some oversized requests of 2^32 clock
cycles or more now fail with -EINVAL instead of being programmed with
truncated values. Clamping such requests to the maximum possible
configuration is a pre-existing issue best addressed when converting
the driver to the waveform API.
The truncation was not reachable when it was introduced. The period
was an unsigned int in nanoseconds then, which limits it to about
4.29 s. Commit a9d887dc1c60 ("pwm: Convert period and duty cycle to
u64") made longer periods possible.
Fixes: a9d887dc1c60 ("pwm: Convert period and duty cycle to u64")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Changes in v2:
- Keep only the Fixes tag for the commit that exposed the bug (Uwe).
- Clarify the pre-existing handling of oversized requests (Uwe).
- Add Microchip's confirmation of the data sheet error.
- Rebase onto v7.3-rc7.
- No code changes from v1.
v1: https://lore.kernel.org/r/20261003083036.21584-1-kmehltretter@gmail.com/
Notes:
Found while testing a Rust port of this driver on a SAM9X75 Curiosity
board. The port uses a 64-bit fls. Its registers differed from the C
driver for long periods.
Tested on that board with a v7.3-rc5-based kernel built with clang
22.1.8. PC20 was routed to PWM2 (peripheral function C) in a local
device tree. The blue LED is on that pin.
- Without the patch there are 23 period ends in 90 s, 3.88 s to 3.93 s
apart. The LED stays dark.
- With the patch the period ends are 19.98 s to 20.01 s apart. The LED
is on for 10 s of every 20 s.
The SAM9X60 and SAM9X7 data sheets describe the channel counter as
16 bit. That does not match the hardware.
- The 20.0 s result needs CPRD=0x9ef21aae. All 32 bits are used.
- Commit 0285827d546d ("pwm: atmel: Add support for controllers with
32 bit counters") says 32 bit.
- Microchip's Harmony PWM example for this board programs
CPRD=133333333.
Microchip support has confirmed that the data sheets are incorrect and
will be corrected.
drivers/pwm/pwm-atmel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 86918523d821..574020d90a3b 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -195,7 +195,7 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
* So for each bit the number of clock cycles is wider divide the input
* clock frequency by two using pres and shift cprd accordingly.
*/
- shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
+ shift = fls64(cycles) - atmel_pwm->data->cfg.period_bits;
if (shift > PWM_MAX_PRES) {
dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more
2026-10-16 16:12 [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more Karl Mehltretter
@ 2026-10-22 8:29 ` Uwe Kleine-König
2026-10-24 7:16 ` Karl Mehltretter
2026-10-27 2:27 ` [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods Karl Mehltretter
1 sibling, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2026-10-22 8:29 UTC (permalink / raw)
To: Karl Mehltretter
Cc: Claudiu Beznea, Nicolas Ferre, Alexandre Belloni, linux-pwm,
linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5252 bytes --]
Hello Karl,
On Fri, Oct 16, 2026 at 06:12:21PM +0200, Karl Mehltretter wrote:
> atmel_pwm_calculate_cprd_and_pres() keeps the period in clock cycles in
> an unsigned long long but derives the prescaler from fls(cycles). fls()
> takes an unsigned int, so the upper 32 bits are ignored.
>
> On controllers that the driver configures with a 32-bit period register
> (SAM9X60, SAM9X7) a request of 2^32 clock cycles or more therefore gets
> prescaler 0 and a truncated CPRD. The request succeeds and sysfs
> reports the requested period.
>
> On a SAM9X75 (PWM clock 266.67 MHz) this affects periods from about
> 16.1 s on. Requesting 20 s with a 10 s duty cycle
>
> # echo 2 > /sys/class/pwm/pwmchip0/export
> # echo 20000000000 > /sys/class/pwm/pwmchip0/pwm2/period
> # echo 10000000000 > /sys/class/pwm/pwmchip0/pwm2/duty_cycle
> # echo 1 > /sys/class/pwm/pwmchip0/pwm2/enable
>
> programs CPRE=0, CPRD=0x3de4355c and CDTY=0x9ef21aae. The channel runs
> with a period of 3.9 s, measured from the period end flag in PWM_ISR.
> With this duty cycle CDTY is above CPRD, so the output stays inactive
> and an LED on it stays dark.
>
> Use fls64() so that the prescaler is derived from the full 64-bit
> value. The driver then programs CPRE=1, CPRD=0x9ef21aae and
> CDTY=0x4f790d57. The measured period is 20.0 s and the LED is on for
> 10 s of every 20 s.
>
> Controllers configured with a 16-bit period register are unaffected
> for all representable periods. Some oversized requests of 2^32 clock
> cycles or more now fail with -EINVAL instead of being programmed with
> truncated values. Clamping such requests to the maximum possible
> configuration is a pre-existing issue best addressed when converting
> the driver to the waveform API.
>
> The truncation was not reachable when it was introduced. The period
> was an unsigned int in nanoseconds then, which limits it to about
> 4.29 s. Commit a9d887dc1c60 ("pwm: Convert period and duty cycle to
> u64") made longer periods possible.
>
> Fixes: a9d887dc1c60 ("pwm: Convert period and duty cycle to u64")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Changes in v2:
> - Keep only the Fixes tag for the commit that exposed the bug (Uwe).
> - Clarify the pre-existing handling of oversized requests (Uwe).
> - Add Microchip's confirmation of the data sheet error.
> - Rebase onto v7.3-rc7.
> - No code changes from v1.
>
> v1: https://lore.kernel.org/r/20261003083036.21584-1-kmehltretter@gmail.com/
>
> Notes:
> Found while testing a Rust port of this driver on a SAM9X75 Curiosity
> board. The port uses a 64-bit fls. Its registers differed from the C
> driver for long periods.
>
> Tested on that board with a v7.3-rc5-based kernel built with clang
> 22.1.8. PC20 was routed to PWM2 (peripheral function C) in a local
> device tree. The blue LED is on that pin.
>
> - Without the patch there are 23 period ends in 90 s, 3.88 s to 3.93 s
> apart. The LED stays dark.
> - With the patch the period ends are 19.98 s to 20.01 s apart. The LED
> is on for 10 s of every 20 s.
>
> The SAM9X60 and SAM9X7 data sheets describe the channel counter as
> 16 bit. That does not match the hardware.
>
> - The 20.0 s result needs CPRD=0x9ef21aae. All 32 bits are used.
> - Commit 0285827d546d ("pwm: atmel: Add support for controllers with
> 32 bit counters") says 32 bit.
> - Microchip's Harmony PWM example for this board programs
> CPRD=133333333.
>
> Microchip support has confirmed that the data sheets are incorrect and
> will be corrected.
>
> drivers/pwm/pwm-atmel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 86918523d821..574020d90a3b 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -195,7 +195,7 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
> * So for each bit the number of clock cycles is wider divide the input
> * clock frequency by two using pres and shift cprd accordingly.
> */
> - shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
> + shift = fls64(cycles) - atmel_pwm->data->cfg.period_bits;
>
> if (shift > PWM_MAX_PRES) {
> dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
just above this code snippet there is:
unsigned long long cycles = state->period;
/* Calculate the period cycles and prescale value */
cycles *= clkrate;
do_div(cycles, NSEC_PER_SEC);
there is another overflow happening here if state->period * clkrate
doesn't fit into an u64. A complete fix converts that to:
cycles = mul_u64_u64_div_u64(state->period, clkrate, NSEC_PER_SEC);
which prevents an overflow if clkrate ≤ NSEC_PER_SEC.
The latter can be asserted by checking that at probe time after calling
clk_rate_exclusive_get().
The options now are I apply your patch anyhow and the above can be
addressed later, or you rework your patch to a complete fix. What is
your preference?
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more
2026-10-22 8:29 ` Uwe Kleine-König
@ 2026-10-24 7:16 ` Karl Mehltretter
0 siblings, 0 replies; 5+ messages in thread
From: Karl Mehltretter @ 2026-10-24 7:16 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Claudiu Beznea, Nicolas Ferre, Alexandre Belloni, linux-pwm,
linux-arm-kernel, linux-kernel
Hi Uwe,
On Thu, Oct 22, 2026 at 10:29:42AM +0100, Uwe Kleine-König wrote:
> >
> > if (shift > PWM_MAX_PRES) {
> > dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
>
> just above this code snippet there is:
>
> unsigned long long cycles = state->period;
>
> /* Calculate the period cycles and prescale value */
> cycles *= clkrate;
> do_div(cycles, NSEC_PER_SEC);
>
> there is another overflow happening here if state->period * clkrate
> doesn't fit into an u64. A complete fix converts that to:
>
> cycles = mul_u64_u64_div_u64(state->period, clkrate, NSEC_PER_SEC);
>
> which prevents an overflow if clkrate ≤ NSEC_PER_SEC.
> The latter can be asserted by checking that at probe time after calling
> clk_rate_exclusive_get().
>
> The options now are I apply your patch anyhow and the above can be
> addressed later, or you rework your patch to a complete fix. What is
> your preference?
>
Good point. I took a look at this and will send an improved patch soon.
Karl
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods
2026-10-16 16:12 [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more Karl Mehltretter
2026-10-22 8:29 ` Uwe Kleine-König
@ 2026-10-27 2:27 ` Karl Mehltretter
2026-10-27 13:42 ` David Laight
1 sibling, 1 reply; 5+ messages in thread
From: Karl Mehltretter @ 2026-10-27 2:27 UTC (permalink / raw)
To: Uwe Kleine-König, Claudiu Beznea
Cc: Karl Mehltretter, Nicolas Ferre, Alexandre Belloni, linux-pwm,
linux-arm-kernel, linux-kernel
atmel_pwm_calculate_cprd_and_pres() first multiplies the requested
period by the clock rate in a u64 and only then divides by
NSEC_PER_SEC. The product can overflow even when the quotient and the
resulting hardware configuration are representable.
For example, at 12 MHz a 1600 s period has 19.2 billion clock cycles
and fits with prescaler 3, but the intermediate product exceeds U64_MAX.
It wraps and the driver programs a period of about 62.8 s instead. This
remains broken after changing fls() to fls64(). The duty-cycle
calculation has the same overflow.
Use mul_u64_u64_div_u64() for both conversions and use fls64() when
deriving the prescaler. The multiply/divide result cannot overflow if
the clock rate is at most NSEC_PER_SEC, so hold the clock rate exclusive
and validate it at probe time.
The reverse conversion in atmel_pwm_get_state() can overflow too: it
multiplies CPRD or CDTY by NSEC_PER_SEC and then shifts the u64 by the
prescaler. Convert the scaled clock ticks with the overflow-safe,
round-up helper instead. Explicitly saturate values that cannot be
represented by the u64 nanosecond API, rather than relying on the
architecture-specific helper's quotient-overflow behaviour.
Fixes: a9d887dc1c60 ("pwm: Convert period and duty cycle to u64")
Cc: stable@vger.kernel.org
Reported-by: Uwe Kleine-König <ukleinek@kernel.org>
Link: https://lore.kernel.org/r/atnHHaOwTkI78_tc@monoceros
Suggested-by: Uwe Kleine-König <ukleinek@kernel.org>
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Changes in v3:
- Use mul_u64_u64_div_u64() for the period conversion as suggested by
Uwe, and for the analogous duty-cycle conversion.
- Fix the corresponding overflow in the get_state() conversion and
explicitly saturate results that cannot be represented in nanoseconds.
- Hold the clock rate exclusive and validate the rate bound required by
the arithmetic.
- Rebase onto v7.3-rc7.
Testing:
- Built an ARM zImage with clang 22.1.8 and W=1.
- Passed 14 host boundary tests, 53 oracle-checked SAM9X75 QEMU boots,
and all 20 PWM device-model qtests.
- Passed 89/89 checks on a physical SAM9X75, including overflow cases.
A 69.175-second period measured 69.210 seconds between interrupts; no
kernel errors were found.
drivers/pwm/pwm-atmel.c | 46 +++++++++++++++++++++++++++++------------
1 file changed, 33 insertions(+), 13 deletions(-)
diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 86918523d8..db7a14c66a 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -23,6 +23,7 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/math64.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -183,19 +184,18 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
unsigned long *cprd, u32 *pres)
{
struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
- unsigned long long cycles = state->period;
+ unsigned long long cycles;
int shift;
/* Calculate the period cycles and prescale value */
- cycles *= clkrate;
- do_div(cycles, NSEC_PER_SEC);
+ cycles = mul_u64_u64_div_u64(state->period, clkrate, NSEC_PER_SEC);
/*
* The register for the period length is cfg.period_bits bits wide.
* So for each bit the number of clock cycles is wider divide the input
* clock frequency by two using pres and shift cprd accordingly.
*/
- shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
+ shift = fls64(cycles) - atmel_pwm->data->cfg.period_bits;
if (shift > PWM_MAX_PRES) {
dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
@@ -218,12 +218,23 @@ static void atmel_pwm_calculate_cdty(const struct pwm_state *state,
{
unsigned long long cycles = state->duty_cycle;
- cycles *= clkrate;
- do_div(cycles, NSEC_PER_SEC);
+ cycles = mul_u64_u64_div_u64(cycles, clkrate, NSEC_PER_SEC);
cycles >>= pres;
+
*cdty = cprd - cycles;
}
+static u64 atmel_pwm_ticks_to_ns(u64 ticks, unsigned long rate)
+{
+ u64 max_ticks;
+
+ max_ticks = mul_u64_u64_div_u64(U64_MAX, rate, NSEC_PER_SEC);
+ if (ticks > max_ticks)
+ return U64_MAX;
+
+ return mul_u64_u64_div_u64_roundup(ticks, NSEC_PER_SEC, rate);
+}
+
static void atmel_pwm_update_cdty(struct pwm_chip *chip, struct pwm_device *pwm,
unsigned long cdty)
{
@@ -352,24 +363,22 @@ static int atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
if (sr & (1 << pwm->hwpwm)) {
unsigned long rate = clk_get_rate(atmel_pwm->clk);
u32 cdty, cprd, pres;
- u64 tmp;
+ u64 ticks;
pres = cmr & PWM_CMR_CPRE_MSK;
cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
atmel_pwm->data->regs.period);
- tmp = (u64)cprd * NSEC_PER_SEC;
- tmp <<= pres;
- state->period = DIV64_U64_ROUND_UP(tmp, rate);
+ ticks = (u64)cprd << pres;
+ state->period = atmel_pwm_ticks_to_ns(ticks, rate);
/* Wait for an updated duty_cycle queued in hardware */
atmel_pwm_wait_nonpending(atmel_pwm, pwm->hwpwm);
cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
atmel_pwm->data->regs.duty);
- tmp = (u64)(cprd - cdty) * NSEC_PER_SEC;
- tmp <<= pres;
- state->duty_cycle = DIV64_U64_ROUND_UP(tmp, rate);
+ ticks = (u64)(cprd - cdty) << pres;
+ state->duty_cycle = atmel_pwm_ticks_to_ns(ticks, rate);
state->enabled = true;
} else {
@@ -487,6 +496,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
{
struct atmel_pwm_chip *atmel_pwm;
struct pwm_chip *chip;
+ unsigned long rate;
int ret;
chip = devm_pwmchip_alloc(&pdev->dev, 4, sizeof(*atmel_pwm));
@@ -507,6 +517,16 @@ static int atmel_pwm_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(atmel_pwm->clk),
"failed to get prepared PWM clock\n");
+ ret = devm_clk_rate_exclusive_get(&pdev->dev, atmel_pwm->clk);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to get exclusive PWM clock rate\n");
+
+ rate = clk_get_rate(atmel_pwm->clk);
+ if (!rate || rate > NSEC_PER_SEC)
+ return dev_err_probe(&pdev->dev, -EINVAL,
+ "invalid PWM clock rate %lu Hz\n", rate);
+
chip->ops = &atmel_pwm_ops;
ret = atmel_pwm_enable_clk_if_on(chip, true);
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
--
2.39.5 (Apple Git-154)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods
2026-10-27 2:27 ` [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods Karl Mehltretter
@ 2026-10-27 13:42 ` David Laight
0 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2026-10-27 13:42 UTC (permalink / raw)
To: Karl Mehltretter
Cc: Uwe Kleine-König, Claudiu Beznea, Nicolas Ferre,
Alexandre Belloni, linux-pwm, linux-arm-kernel, linux-kernel
On Tue, 27 Oct 2026 03:27:55 +0100
Karl Mehltretter <kmehltretter@gmail.com> wrote:
> atmel_pwm_calculate_cprd_and_pres() first multiplies the requested
> period by the clock rate in a u64 and only then divides by
> NSEC_PER_SEC. The product can overflow even when the quotient and the
> resulting hardware configuration are representable.
>
> For example, at 12 MHz a 1600 s period has 19.2 billion clock cycles
> and fits with prescaler 3, but the intermediate product exceeds U64_MAX.
> It wraps and the driver programs a period of about 62.8 s instead. This
> remains broken after changing fls() to fls64(). The duty-cycle
> calculation has the same overflow.
>
> Use mul_u64_u64_div_u64() for both conversions and use fls64() when
> deriving the prescaler. The multiply/divide result cannot overflow if
> the clock rate is at most NSEC_PER_SEC, so hold the clock rate exclusive
> and validate it at probe time.
>
> The reverse conversion in atmel_pwm_get_state() can overflow too: it
> multiplies CPRD or CDTY by NSEC_PER_SEC and then shifts the u64 by the
> prescaler. Convert the scaled clock ticks with the overflow-safe,
> round-up helper instead. Explicitly saturate values that cannot be
> represented by the u64 nanosecond API, rather than relying on the
> architecture-specific helper's quotient-overflow behaviour.
Personally I think mul_u64_u64_div_u64() should return ~0ull on both
overflow and divide by zero.
While a valid result, any code that expects it can pretty certainly
also get 'a bit more' so including ~0ull in the 'overflow' values
doesn't really do any harm.
But someone insisted on in faulting - even though there has been
a patch to x86-64 to recover from the fault.
>
> Fixes: a9d887dc1c60 ("pwm: Convert period and duty cycle to u64")
> Cc: stable@vger.kernel.org
> Reported-by: Uwe Kleine-König <ukleinek@kernel.org>
> Link: https://lore.kernel.org/r/atnHHaOwTkI78_tc@monoceros
> Suggested-by: Uwe Kleine-König <ukleinek@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
> ---
> Changes in v3:
> - Use mul_u64_u64_div_u64() for the period conversion as suggested by
> Uwe, and for the analogous duty-cycle conversion.
> - Fix the corresponding overflow in the get_state() conversion and
> explicitly saturate results that cannot be represented in nanoseconds.
> - Hold the clock rate exclusive and validate the rate bound required by
> the arithmetic.
> - Rebase onto v7.3-rc7.
>
> Testing:
> - Built an ARM zImage with clang 22.1.8 and W=1.
> - Passed 14 host boundary tests, 53 oracle-checked SAM9X75 QEMU boots,
> and all 20 PWM device-model qtests.
> - Passed 89/89 checks on a physical SAM9X75, including overflow cases.
> A 69.175-second period measured 69.210 seconds between interrupts; no
> kernel errors were found.
>
> drivers/pwm/pwm-atmel.c | 46 +++++++++++++++++++++++++++++------------
> 1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
> index 86918523d8..db7a14c66a 100644
> --- a/drivers/pwm/pwm-atmel.c
> +++ b/drivers/pwm/pwm-atmel.c
> @@ -23,6 +23,7 @@
> #include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/io.h>
> +#include <linux/math64.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> @@ -183,19 +184,18 @@ static int atmel_pwm_calculate_cprd_and_pres(struct pwm_chip *chip,
> unsigned long *cprd, u32 *pres)
> {
> struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
> - unsigned long long cycles = state->period;
> + unsigned long long cycles;
> int shift;
>
> /* Calculate the period cycles and prescale value */
> - cycles *= clkrate;
> - do_div(cycles, NSEC_PER_SEC);
> + cycles = mul_u64_u64_div_u64(state->period, clkrate, NSEC_PER_SEC);
>
> /*
> * The register for the period length is cfg.period_bits bits wide.
> * So for each bit the number of clock cycles is wider divide the input
> * clock frequency by two using pres and shift cprd accordingly.
> */
> - shift = fls(cycles) - atmel_pwm->data->cfg.period_bits;
> + shift = fls64(cycles) - atmel_pwm->data->cfg.period_bits;
>
> if (shift > PWM_MAX_PRES) {
> dev_err(pwmchip_parent(chip), "pres exceeds the maximum value\n");
> @@ -218,12 +218,23 @@ static void atmel_pwm_calculate_cdty(const struct pwm_state *state,
> {
> unsigned long long cycles = state->duty_cycle;
>
> - cycles *= clkrate;
> - do_div(cycles, NSEC_PER_SEC);
> + cycles = mul_u64_u64_div_u64(cycles, clkrate, NSEC_PER_SEC);
> cycles >>= pres;
> +
> *cdty = cprd - cycles;
> }
>
> +static u64 atmel_pwm_ticks_to_ns(u64 ticks, unsigned long rate)
> +{
> + u64 max_ticks;
> +
> + max_ticks = mul_u64_u64_div_u64(U64_MAX, rate, NSEC_PER_SEC);
> + if (ticks > max_ticks)
> + return U64_MAX;
I'm not sure how often this is called, but it might be worth caching
max_ticks during the probe.
(and maybe even the clock rate itself - I doubt it can change.)
David
> +
> + return mul_u64_u64_div_u64_roundup(ticks, NSEC_PER_SEC, rate);
> +}
> +
> static void atmel_pwm_update_cdty(struct pwm_chip *chip, struct pwm_device *pwm,
> unsigned long cdty)
> {
> @@ -352,24 +363,22 @@ static int atmel_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> if (sr & (1 << pwm->hwpwm)) {
> unsigned long rate = clk_get_rate(atmel_pwm->clk);
> u32 cdty, cprd, pres;
> - u64 tmp;
> + u64 ticks;
>
> pres = cmr & PWM_CMR_CPRE_MSK;
>
> cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
> atmel_pwm->data->regs.period);
> - tmp = (u64)cprd * NSEC_PER_SEC;
> - tmp <<= pres;
> - state->period = DIV64_U64_ROUND_UP(tmp, rate);
> + ticks = (u64)cprd << pres;
> + state->period = atmel_pwm_ticks_to_ns(ticks, rate);
>
> /* Wait for an updated duty_cycle queued in hardware */
> atmel_pwm_wait_nonpending(atmel_pwm, pwm->hwpwm);
>
> cdty = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
> atmel_pwm->data->regs.duty);
> - tmp = (u64)(cprd - cdty) * NSEC_PER_SEC;
> - tmp <<= pres;
> - state->duty_cycle = DIV64_U64_ROUND_UP(tmp, rate);
> + ticks = (u64)(cprd - cdty) << pres;
> + state->duty_cycle = atmel_pwm_ticks_to_ns(ticks, rate);
>
> state->enabled = true;
> } else {
> @@ -487,6 +496,7 @@ static int atmel_pwm_probe(struct platform_device *pdev)
> {
> struct atmel_pwm_chip *atmel_pwm;
> struct pwm_chip *chip;
> + unsigned long rate;
> int ret;
>
> chip = devm_pwmchip_alloc(&pdev->dev, 4, sizeof(*atmel_pwm));
> @@ -507,6 +517,16 @@ static int atmel_pwm_probe(struct platform_device *pdev)
> return dev_err_probe(&pdev->dev, PTR_ERR(atmel_pwm->clk),
> "failed to get prepared PWM clock\n");
>
> + ret = devm_clk_rate_exclusive_get(&pdev->dev, atmel_pwm->clk);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to get exclusive PWM clock rate\n");
> +
> + rate = clk_get_rate(atmel_pwm->clk);
> + if (!rate || rate > NSEC_PER_SEC)
> + return dev_err_probe(&pdev->dev, -EINVAL,
> + "invalid PWM clock rate %lu Hz\n", rate);
> +
> chip->ops = &atmel_pwm_ops;
>
> ret = atmel_pwm_enable_clk_if_on(chip, true);
>
> base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-27 13:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 16:12 [PATCH v2] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more Karl Mehltretter
2026-10-22 8:29 ` Uwe Kleine-König
2026-10-24 7:16 ` Karl Mehltretter
2026-10-27 2:27 ` [PATCH v3] pwm: atmel: Fix arithmetic overflows for long periods Karl Mehltretter
2026-10-27 13:42 ` David Laight
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox