The .clock_div directive in pioasm computes the fractional part of the divider incorrectly for most inputs:
|
clock_div_frac = (uint8_t)((clock_div - (float)clock_div_frac) * (1u << 8u)); |
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:
.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_divdirective in pioasm computes the fractional part of the divider incorrectly for most inputs:pico-sdk/tools/pioasm/pio_assembler.cpp
Line 72 in 079c6f3
At this point, the value of
clock_div_fracis zero. Instead, it should subtractclock_div_int. The result is the whole divider multiplied by 256. This can be demonstrated by buildingtools/pioasm/test/amethyst.pio, which includes this.clock_divstatement:pico-sdk/tools/pioasm/test/amethyst.pio
Line 11 in 079c6f3
.clock_div 3.6has fractional part 0.6, which is 153/256. Therefore, we expectsm_config_set_clkdiv_int_frac(&c, 3, 153);, but instead we getsm_config_set_clkdiv_int_frac(&c, 3, 921);and a warning from GCC: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:-Werrortreats this as a hard build error..clock_div 1yields (1, 256), which the compiler folds to (1, 0). This is correct, but it needlessly emits aclkdivcall for the default divider that a correct pioasm would omit.-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.