RP2040: fix off-by-one PWM top issue#4192
Conversation
|
Tested it, this works perfectly now, thank you @dhalbert |
tannewt
left a comment
There was a problem hiding this comment.
I've gotta look at this further when I'm fresh. See section 4.5.2.2 in the datasheet. My understanding was that CC needs to be TOP+1 in order to not go low at all.
|
I see what you mean in the datasheet. I will look at the output with the Saleae. We may need to use 65534 as the TOP to set instead of 65535. |
|
@tannewt I read the datasheet and did a lot of boundary testing with the Saleae, checking both the low and high frequency cases at the boundary , and much higher and lower frequencies. I made sure |
Fixes #4189.
The
pwmio_pwmout_obj_ttopwas 32 bits, but it's passed to pico-sdk as 16 bits. Make it 16 bits so the compiler can catch values that are too large. Then needed to force some arithmetic to be 32 bit.Maintain top as a 0 to n-1 value. It was getting set to 65536 in one case. Remove
self-> top -1computations.Comparison check for low vs high frequency had a boundary problem at
.frequency = 1907. Fix by using<=comparison instead of<.Add some bounds checking to make sure values fit in 16 bits.
Unrelated, missed getting into a previous PR: Remove extraneous include in
shared-bindings/adafruit_bus_device/SPIDevice.c.@jedgarpark Could you test this after the artifact builds? It's more complicated than the version I gave you earlier to test.