Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Karl Mehltretter <kmehltretter@gmail.com>
To: "Uwe Kleine-König" <ukleinek@kernel.org>,
	"Claudiu Beznea" <claudiu.beznea@tuxon.dev>
Cc: Karl Mehltretter <kmehltretter@gmail.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] pwm: atmel: Fix prescaler for periods of 2^32 clock cycles or more
Date: Sat,  3 Oct 2026 10:30:35 +0200	[thread overview]
Message-ID: <20261003083036.21584-1-kmehltretter@gmail.com> (raw)

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 out-of-range requests of 2^32 clock
cycles or more were accepted and programmed with truncated values. They
now fail with -EINVAL.

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: 2101c878f767 ("pwm: atmel: Replace loop in prescale calculation by ad-hoc calculation")
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>
---

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.
    
    I will report the data sheet text to Microchip.

 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: e767a4ea70a3992c37ed604157d32f0dfbf9b1e3
-- 
2.39.5 (Apple Git-154)



                 reply	other threads:[~2026-10-03  8:31 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261003083036.21584-1-kmehltretter@gmail.com \
    --to=kmehltretter@gmail.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=ukleinek@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox