Brunocor26 opened a new pull request, #18947:
URL: https://github.com/apache/nuttx/pull/18947

   ## 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 4df80e19 ("!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 4df80e19.
   * 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%:
   ```sh
   setup_period: PWM3 freq 25000 max 2288
   setup_period: PWM3 top 0x00001999 div 0x0000000E
   setup_pulse: PWM3 compare 0x0000FFFF flags 0x00000000
   ```
   
   Duty 50%:
   ```sh
   setup_period: PWM3 top 0x00001999 div 0x0000000E
   setup_pulse: PWM3 compare 0x0005000F flags 0x00000000
   ```
   
   Duty 100%:
   ```sh
   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):
   ```sh
   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):
   ```sh
   nsh> pwm -d 50
   pwm_start: PWM3
   setup_period: PWM3 freq=25000 top=5999 div=16
   setup_pulse: PWM3 compare=0x00000bb7 flags=0x00000000
   ```
   
   ```sh
     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
   
     * [x] This PR introduces only one functional change.
     * [x] I have updated all required description fields above.
     * [x] My PR adheres to Contributing Guidelines and Documentation.
     * [ ] My PR is still work in progress (not ready for review).
     * [x] My PR is ready for review and can be safely merged into a codebase.
       


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to