raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.25k stars 838 forks source link

Update dma.h (Fixes #1677) (Inline docs only) #1678

Closed myklemykle closed 1 month ago

myklemykle commented 3 months ago

I think It's important to be clear that how DMA timers are scaled on RP2040 is the opposite of how PWM clocks are scaled.

Specifically, pwm_config_set_clkdiv_int_frac() lets the user specifiy a fractional divider, but dma_timer_set_fraction() lets the user specify a fractional multiplier. So for instance the PWM divider is only effective if it's more than 1, while the DMA multiplier is only effective if it's less than 1.

This patch updates the inline doc of dma_timer_set_fraction() to reflect that it specifies a multiplier, not a divider, of clk_sys.

lurch commented 3 months ago

I think if this was called a "multiplier", that would imply that the DMA timer can run faster than the system-clock, but it can't, so IMHO this is still a "divider". Page 118 of https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf says "The pacing timer produces TREQ assertions at a rate set by ((X/Y) * sys_clk). This equation is evaluated every sys_clk cycles and therefore can only generate TREQs at a rate of 1 per sys_clk (i.e. permanent TREQ) or less."

OTOH, perhaps it makes sense for dma_timer_set_fraction to assert that numerator <= denominator ? :shrug:

myklemykle commented 3 months ago

OK, I get now that those are the engineering flavors of those words, more than the math flavors. But in both cases the divider, really, is a piece of hardware in a clock; the specified fraction configures that divider. IMO "Set the divider" makes it sound like the value you're passing is itself the divider, and will be used to divide in a math sense. Maybe "Configure the divider" is better?

I just think it should be as clear as possible that PWM and DMA use opposite schemes for configuring clock dividers with fractions.

myklemykle commented 3 months ago

OTOH, perhaps it makes sense for dma_timer_set_fraction to assert that numerator <= denominator ? 🤷

Seems sensible. And the opposite for pwm_config_set_clkdiv_int_frac .

lurch commented 3 months ago

And the opposite for pwm_config_set_clkdiv_int_frac .

That already checks that integer >= 1 :slightly_smiling_face: