linux icon indicating copy to clipboard operation
linux copied to clipboard

pwm-pio fails for duty cycles close to 0 or configured period

Open platinum95 opened this issue 1 month ago • 1 comments

Describe the bug

Attempting to set a duty cycle in the range (0,15) or (period-15,period) on a PWM device configured using the pwm-pio-rp1 driver fails, with the driver returning -EINVAL.

It looks like this is intentional behaviour since driver has a resolution limit of 15ns, so the ..._apply function early-exits if the requested duty cycle is within a single resolution step of the upper or lower limit. The values 0 and period are allowed, however. All valid values are floor'd to the prior integer multiple of the resolution. drivers/pwm/pwm-pio-rp1.c#L104

I've possibly missed a more fundamental reason but I'm not entirely sure if this is a necessary guard, especially since it prevents some other drivers (such as pwm-fan) from building on top of this one. I've tested a change locally that removes the restriction, which seems to work fine. Change is here for reference.

Steps to reproduce the behaviour

  1. Enable a PIO PWM instance, e.g dtoverlay=pwm-pio.
  2. Enable & configure the PWM device, e.g.:
  • cd /sys/class/pwm/pwmchip1
  • `echo "0" > export
  • echo "40000" > pwm0/period
  • echo "20000" > pwm0/duty_cycle
  • echo "1" > pwm0/enable
  1. Attempt to set a duty cycle between (0,15) and (39985,40000)

Observe that the write attempt fails with -EINVAL and the PWM output does not change:

$ echo "1" > duty_cycle 
-bash: echo: write error: Invalid argument

Device (s)

Raspberry Pi 5

System

$ cat /etc/rpi-issue 
Raspberry Pi reference 2024-11-19
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 891df1e21ed2b6099a2e6a13e26c91dea44b34d4, stage2

$ vcgencmd version
2025/11/05 17:37:18 
Copyright (c) 2012 Broadcom
version 57db150d (release) (embedded)

$ uname -a
Linux moodercade 6.12.57-v8-16k+ #1920 SMP PREEMPT Tue Nov  4 10:17:18 GMT 2025 aarch64 GNU/Linux

Logs

No response

Additional context

No response

platinum95 avatar Nov 10 '25 22:11 platinum95

First of all, it's nice to hear that somebody else is using this - it was intended to be as much of a proof-of-concept as a real driver.

I don't have strong feelings about whether or not it is better to return an error or just tweak the supplied values to make them acceptable. I added the driver in November last year to rpi-6.6.y, at which time the documentation for the get_state method of pwm_ops said that it was only called at registration time; that has since changed. This makes me happier to massage the given values to make them acceptable, and provide a get_state implementation so the actual values can be queried.

I accept that your code is just of proof of concept, but I wouldn't want to accept it as-is because it allows an encoded period (*) of zero to be set, which then wraps and causes and causes an effectively infinite period. Thinking about the implementation some more, it should be possible to support 0% and 100% duty cycles by pausing the state machine, but I'm not proposing to do that now - just the clamp and report change.

For the record, here are the properties and limitations of the PIO state machine:

  • The basic tick is 15ns, which is (1s/200MHz) * 3 PIO cycles.
  • The minimum actual period is 2 ticks. However, the first of these ticks is actually 4 PIO cycles, so the resulting clock is actually slower than expected and has a higher duty cycle.
  • The period values sent to the state machine (what I called and encoded value (*) above) are actually one lower than the resulting cycle length, so the requested period should be adjusted before being pushed to the FIFO.
  • The maximum duty cycle is currently (actual period - 1) - it is not possible to get 100% without occasional glitches. As mentioned above, this could be worked around by pausing the state machine to indefinitely hold a fixed high or low value.

pelwell avatar Nov 11 '25 12:11 pelwell