pioasm: .clock_div emits wrong fractional divider #3189

New issue

Open

opened 2026-09-09 00:59:11 -05:00 by jacobmarble · 0 comments

jacobmarble commented

2026-09-09 00:59:11 -05:00

(Migrated from github.com)

The .clock_div directive in pioasm computes the fractional part of the divider incorrectly for most inputs: github.com/raspberrypi/pico-sdk@079c6f3902/tools/pioasm/pio_assembler.cpp (L72)

At this point, the value of clock_div_frac is zero. Instead, it should subtract clock_div_int. The result is the whole divider multiplied by 256. This can be demonstrated by building tools/pioasm/test/amethyst.pio, which includes this .clock_div statement: github.com/raspberrypi/pico-sdk@079c6f3902/tools/pioasm/test/amethyst.pio (L11)

.clock_div 3.6 has fractional part 0.6, which is 153/256. Therefore, we expect sm_config_set_clkdiv_int_frac(&c, 3, 153);, but instead we get sm_config_set_clkdiv_int_frac(&c, 3, 921); and a warning from GCC:

amethyst.h:90:42: warning: unsigned conversion from 'int' to 'uint8_t' {aka 'unsigned char'} changes value from '921' to '153' [-Woverflow]
   90 |     sm_config_set_clkdiv_int_frac(&c, 3, 921);

Because function sm_config_set_clkdiv_int_frac() takes as its third argument an 8-bit unsigned integer, the overflow logic happens to work out fine in the end. However, the result is needless tedium:

Anyone reading the generated header, or copying the call into their own code, sees (3, 921) instead of (3, 153).
-Werror treats this as a hard build error.
.clock_div 1 yields (1, 256), which the compiler folds to (1, 0). This is correct, but it needlessly emits a clkdiv call for the default divider that a correct pioasm would omit.
With -o json, pioasm emits "clockDiv": {"int": 3, "frac": 921}. There is no 8-bit conversion downstream, so any non-C consumer of that format gets a wrong divider. This is the one case where the bug is not merely cosmetic.

The `.clock_div` directive in pioasm computes the fractional part of the divider incorrectly for most inputs: https://github.com/raspberrypi/pico-sdk/blob/079c6f39023649b154152db30f1d781e884879bc/tools/pioasm/pio_assembler.cpp#L72 At this point, the value of `clock_div_frac` is zero. Instead, it should subtract `clock_div_int`. The result is the whole divider multiplied by 256. This can be demonstrated by building `tools/pioasm/test/amethyst.pio`, which includes this `.clock_div` statement: https://github.com/raspberrypi/pico-sdk/blob/079c6f39023649b154152db30f1d781e884879bc/tools/pioasm/test/amethyst.pio#L11 `.clock_div 3.6` has fractional part 0.6, which is 153/256. Therefore, we expect `sm_config_set_clkdiv_int_frac(&c, 3, 153);`, but instead we get `sm_config_set_clkdiv_int_frac(&c, 3, 921);` and a warning from GCC: ```text amethyst.h:90:42: warning: unsigned conversion from 'int' to 'uint8_t' {aka 'unsigned char'} changes value from '921' to '153' [-Woverflow] 90 | sm_config_set_clkdiv_int_frac(&c, 3, 921); ``` Because function `sm_config_set_clkdiv_int_frac()` takes as its third argument an 8-bit unsigned integer, the overflow logic happens to work out fine in the end. However, the result is needless tedium: - Anyone reading the generated header, or copying the call into their own code, sees (3, 921) instead of (3, 153). - `-Werror` treats this as a hard build error. - `.clock_div 1` yields (1, 256), which the compiler folds to (1, 0). This is correct, but it needlessly emits a `clkdiv` call for the default divider that a correct pioasm would omit. - With `-o json`, pioasm emits `"clockDiv": {"int": 3, "frac": 921}`. There is no 8-bit conversion downstream, so any non-C consumer of that format gets a wrong divider. **This is the one case where the bug is not merely cosmetic.**

Rows
Columns