Skip to content

arch/arm/rp23xx: Fix PWM frequency, duty cycle and API migration.#18947

Merged
lupyuen merged 1 commit into
apache:masterfrom
Brunocor26:fix/rp23xx-pwm
May 25, 2026
Merged

arch/arm/rp23xx: Fix PWM frequency, duty cycle and API migration.#18947
lupyuen merged 1 commit into
apache:masterfrom
Brunocor26:fix/rp23xx-pwm

Conversation

@Brunocor26
Copy link
Copy Markdown
Contributor

@Brunocor26 Brunocor26 commented May 24, 2026

Summary

Fix three bugs in the RP23XX PWM driver that produced incorrect frequency, duty cycle output, and build failure on real hardware:

  • setup_period: The previous divisor calculation used integer arithmetic that caused overflow and loss of precision. The divider is now computed using 64-bit arithmetic and clamped to the valid hardware range (0x10 to 0xFFF).

  • setup_pulse: The compare value was incorrectly scaled by TOP instead of 65535, producing wrong duty cycles for all values. The corrected formula is: compare = (duty * (top + 1)) / 65535, with an overflow guard.

  • pwm_start: The driver was not updated as part of the breaking change introduced in commit 4df80e1 ("!drivers/pwm: remove PWM_MULTICHAN option"), which migrated the single-channel API from info->XXX to info->channels[0].XXX. This caused the driver to fail to compile
    with CONFIG_PWM=y and CONFIG_PWM_NCHANNELS=1.

    The fix was derived from the RP2350 PWM formula as per the RP2350 datasheet
    (section 12.5.2.6, "Configuring PWM period"):

    f_pwm = f_sys / ((TOP + 1) * (CSR_PH_CORRECT + 1) * (DIV_INT + DIV_FRAC / 16))

Reference: https://pip-assets.raspberrypi.com/categories/1214-rp2350/documents/RP-008373-DS-2-rp2350-datasheet.pdf

Impact

  • Is new feature added? Is existing feature changed?
    NO. Bug fix only.
  • Impact on user (will user need to adapt to change)?
    NO. The fix corrects silent hardware misconfiguration.
  • Impact on build (will build process change)?
    YES. The driver previously failed to compile with CONFIG_PWM=y and CONFIG_PWM_NCHANNELS=1 due to missing API migration from commit 4df80e1.
  • Impact on hardware (will arch(s) / board(s) / driver(s) change)?
    YES. Affects all RP23XX boards using the PWM driver.
  • Impact on documentation (is update required / provided)?
    NO.
  • Impact on security (any sort of implications)?
    NO.
  • Impact on compatibility (backward/forward/interoperability)?
    NO.

Testing

I confirm that changes are verified on local setup and works as intended:

  • Build Host: Linux, GCC
  • Target: ARM Cortex-M33, raspberrypi-pico-2:nsh, PWM3 pin 6

Verification at 25 kHz (f_sys = 150 MHz):
Expected: TOP = 5999, DIV = 16 (1.0), f_pwm = 150e6 / (6000 * 1.0) = 25 kHz

Testing logs before change (original driver, f_sys=150MHz, f_pwm=25kHz):

Duty 10%:

setup_period: PWM3 freq 25000 max 2288
setup_period: PWM3 top 0x00001999 div 0x0000000E
setup_pulse: PWM3 compare 0x0000FFFF flags 0x00000000

Duty 50%:

setup_period: PWM3 top 0x00001999 div 0x0000000E
setup_pulse: PWM3 compare 0x0005000F flags 0x00000000

Duty 100%:

setup_period: PWM3 top 0x00001999 div 0x0000000E
setup_pulse: PWM3 compare 0x000A0028 flags 0x00000000

The original code produces incorrect results:

  • TOP=0x1999 (6553) instead of 5999: wrong frequency
  • DIV=0x0E (0.875) instead of 0x10 (1.0): further frequency error
  • compare at 10% duty = 0xFFFF (65535): equals 100% output
  • compare at 50% and 100%: overflow, values exceed TOP

Testing logs after change:

Duty 10% (duty=0x1999=6553, compare expected=0x0257=599):

nsh> pwm -d 10
pwm_start: PWM3
setup_period: PWM3 freq=25000 top=5999 div=16
setup_pulse: PWM3 compare=0x00000257 flags=0x00000000

Duty 50% (duty=0x7fff=32767, compare expected=0x0BB7=2999):

nsh> pwm -d 50
pwm_start: PWM3
setup_period: PWM3 freq=25000 top=5999 div=16
setup_pulse: PWM3 compare=0x00000bb7 flags=0x00000000

Duty 100% (duty=0xffff=65535, compare expected=0x176F=5999):

nsh> pwm -d 100
pwm_start: PWM3
setup_period: PWM3 freq=25000 top=5999 div=16
setup_pulse: PWM3 compare=0x0000176f flags=0x00000000

All compare values match the expected hardware calculations.

PR verification Self-Check

  • This PR introduces only one functional change.
  • I have updated all required description fields above.
  • My PR adheres to Contributing Guidelines and Documentation.
  • My PR is still work in progress (not ready for review).
  • My PR is ready for review and can be safely merged into a codebase.

Fixed three bugs in the RP23XX PWM driver:

* setup_period: The previous divisor calculation used integer arithmetic
  that caused overflow and loss of precision. The divider is now computed
  as a 16-bit fixed-point value (div16) using 64-bit arithmetic, and
  clamped to the valid hardware range (0x10 to 0xFFF).

* setup_pulse: The compare value was incorrectly scaled by TOP instead
  of 65535, producing wrong duty cycles. The formula is now corrected
  to ((duty * (top + 1)) / 65535) with an overflow guard.

* pwm_start: The driver was not updated as part of the breaking change
  introduced in commit 4df80e1 ("!drivers/pwm: remove PWM_MULTICHAN
  option"). Access to single channel API is now info->channels[0].duty
  instead of info[0].duty.

Signed-off-by: Brunocor26 <bruno.correia@ubi.pt>
@Brunocor26 Brunocor26 requested a review from jerpelea as a code owner May 24, 2026 01:42
@github-actions github-actions Bot added Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium labels May 24, 2026
@lupyuen lupyuen merged commit 53ef7c0 into apache:master May 25, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants