Make all float->int/frac conversions round to nearest by default (#2065)

* making pio_calculate_clkdiv8_from_float round to the neareset 1/256 (not lower 1/256)

* chage rounding of all float clkdivs to round to neareset; default to nearest (which is a backwards incompatible change, but I think OK), and add ability to turn it off

* fix copy/paste errors in PICO_CONFIG

* Calculate size of FRAC field using its own MSB and LSB, rather than hoping that INT_LSB is in the right place

* Add a new REG_FIELD_WIDTH macro, and make the calculation of the clock-dividers more consistent

---------

Co-authored-by: Andrew Scheller <[email protected]>

src/rp2040/hardware_regs/include/hardware/platform_defs.h

@@ -116,4 +116,6 @@
 #define FIRST_USER_IRQ (NUM_IRQS - NUM_USER_IRQS)
 #define VTABLE_FIRST_IRQ 16
 
+#define REG_FIELD_WIDTH(f) (f ## _MSB + 1 - f ## _LSB)
+
 #endif

src/rp2040/pico_platform/include/pico/platform.h

@@ -70,6 +70,11 @@
 #define PICO_RAM_VECTOR_TABLE_SIZE (VTABLE_FIRST_IRQ + NUM_IRQS)
 #endif
 
+// PICO_CONFIG: PICO_CLKDIV_ROUND_NEAREST, True if floating point clock divisors should be rounded to the nearest possible clock divisor by default rather than rounding down, type=bool, default=1, group=pico_platform
+#ifndef PICO_CLKDIV_ROUND_NEAREST
+#define PICO_CLKDIV_ROUND_NEAREST 1
+#endif
+
 #ifndef __ASSEMBLER__
 
 /*! \brief No-op function for the body of tight loops

src/rp2350/hardware_regs/include/hardware/platform_defs.h

@@ -160,4 +160,6 @@
 #endif
 #define FIRST_USER_IRQ (NUM_IRQS - NUM_USER_IRQS)
 
+#define REG_FIELD_WIDTH(f) (f ## _MSB + 1 - f ## _LSB)
+
 #endif

src/rp2350/pico_platform/include/pico/platform.h

@@ -63,6 +63,11 @@
 #define PICO_USE_STACK_GUARDS 0
 #endif
 
+// PICO_CONFIG: PICO_CLKDIV_ROUND_NEAREST, True if floating point clock divisors should be rounded to the nearest possible clock divisor by default rather than rounding down, type=bool, default=1, group=pico_platform
+#ifndef PICO_CLKDIV_ROUND_NEAREST
+#define PICO_CLKDIV_ROUND_NEAREST 1
+#endif
+
 #ifndef __ASSEMBLER__
 
 /*! \brief No-op function for the body of tight loops

src/rp2_common/hardware_adc/include/hardware/adc.h

@@ -70,6 +70,11 @@
 #define ADC_TEMPERATURE_CHANNEL_NUM (NUM_ADC_CHANNELS - 1)
 #endif
 
+// PICO_CONFIG: PICO_ADC_CLKDIV_ROUND_NEAREST, True if floating point ADC clock divisors should be rounded to the nearest possible clock divisor rather than rounding down, type=bool, default=PICO_CLKDIV_ROUND_NEAREST, group=hardware_adc
+#ifndef PICO_ADC_CLKDIV_ROUND_NEAREST
+#define PICO_ADC_CLKDIV_ROUND_NEAREST PICO_CLKDIV_ROUND_NEAREST
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -197,8 +202,12 @@ static inline void adc_run(bool run) {
  * \param clkdiv If non-zero, conversion will be started at intervals rather than back to back.
  */
 static inline void adc_set_clkdiv(float clkdiv) {
-    invalid_params_if(HARDWARE_ADC, clkdiv >= 1 << (ADC_DIV_INT_MSB - ADC_DIV_INT_LSB + 1));
-    adc_hw->div = (uint32_t)(clkdiv * (float) (1 << ADC_DIV_INT_LSB));
+    invalid_params_if(HARDWARE_ADC, clkdiv >= 1 << REG_FIELD_WIDTH(ADC_DIV_INT));
+    const int frac_bit_count = REG_FIELD_WIDTH(ADC_DIV_FRAC);
+#if PICO_ADC_CLKDIV_ROUND_NEAREST
+    clkdiv += 0.5f / (1 << frac_bit_count); // round to the nearest fraction
+#endif
+    adc_hw->div = (uint32_t)(clkdiv * (float) (1 << frac_bit_count));
 }
 
 /*! \brief Setup the ADC FIFO

src/rp2_common/hardware_clocks/clocks.c

@@ -245,13 +245,13 @@ void clock_gpio_init_int_frac16(uint gpio, uint src, uint32_t div_int, uint16_t
         invalid_params_if(HARDWARE_CLOCKS, true);
     }
 
-    invalid_params_if(HARDWARE_CLOCKS, div_int >> (CLOCKS_CLK_GPOUT0_DIV_INT_MSB - CLOCKS_CLK_GPOUT0_DIV_INT_LSB + 1));
+    invalid_params_if(HARDWARE_CLOCKS, div_int >> REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_INT));
     // Set up the gpclk generator
     clocks_hw->clk[gpclk].ctrl = (src << CLOCKS_CLK_GPOUT0_CTRL_AUXSRC_LSB) |
                                  CLOCKS_CLK_GPOUT0_CTRL_ENABLE_BITS;
-#if CLOCKS_CLK_GPOUT0_DIV_FRAC_MSB - CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB == 15
+#if REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_FRAC) == 16
     clocks_hw->clk[gpclk].div = (div_int << CLOCKS_CLK_GPOUT0_DIV_INT_LSB) | (div_frac16 << CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB);
-#elif CLOCKS_CLK_GPOUT0_DIV_FRAC_MSB - CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB == 7
+#elif REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_FRAC) == 8
     clocks_hw->clk[gpclk].div = (div_int << CLOCKS_CLK_GPOUT0_DIV_INT_LSB) | ((div_frac16>>8u) << CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB);
 #else
 #error unsupported number of fractional bits
@@ -439,4 +439,4 @@ bool check_sys_clock_khz(uint32_t freq_khz, uint *vco_out, uint *postdiv1_out, u
         }
     }
     return false;
-}
+}

src/rp2_common/hardware_clocks/include/hardware/clocks.h

@@ -253,6 +253,11 @@ extern "C" {
 #else
 #define PARAM_ASSERTIONS_ENABLED_HARDWARE_CLOCKS 0
 #endif
+#endif
+
+ // PICO_CONFIG: PICO_CLOCK_GPIO_CLKDIV_ROUND_NEAREST, True if floating point GPIO clock divisors should be rounded to the nearest possible clock divisor rather than rounding down, type=bool, default=PICO_CLKDIV_ROUND_NEAREST, group=hardware_clocks
+#ifndef PICO_CLOCK_GPIO_CLKDIV_ROUND_NEAREST
+#define PICO_CLOCK_GPIO_CLKDIV_ROUND_NEAREST PICO_CLKDIV_ROUND_NEAREST
 #endif
 
 typedef clock_num_t clock_handle_t;
@@ -387,11 +392,15 @@ static inline void clock_gpio_init_int_frac(uint gpio, uint src, uint32_t div_in
 static inline void clock_gpio_init(uint gpio, uint src, float div)
 {
     uint div_int = (uint)div;
-#if CLOCKS_CLK_GPOUT0_DIV_FRAC_MSB - CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB == 15
-    uint16_t frac = (uint16_t)((div - (float)div_int) * (1u << 16));
+    const int frac_bit_count = REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_FRAC);
+#if PICO_CLOCK_GPIO_CLKDIV_ROUND_NEAREST
+    div += 0.5f / (1 << frac_bit_count); // round to the nearest fraction
+#endif
+#if REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_FRAC) == 16
+    uint16_t frac = (uint16_t)((div - (float)div_int) * (1u << frac_bit_count));
     clock_gpio_init_int_frac16(gpio, src, div_int, frac);
-#elif CLOCKS_CLK_GPOUT0_DIV_FRAC_MSB - CLOCKS_CLK_GPOUT0_DIV_FRAC_LSB == 7
-    uint8_t frac = (uint8_t)((div - (float)div_int) * (1u << 8));
+#elif REG_FIELD_WIDTH(CLOCKS_CLK_GPOUT0_DIV_FRAC) == 8
+    uint8_t frac = (uint8_t)((div - (float)div_int) * (1u << frac_bit_count));
     clock_gpio_init_int_frac8(gpio, src, div_int, frac);
 #else
 #error unsupported number of fractional bits

src/rp2_common/hardware_pio/include/hardware/pio.h

@@ -31,6 +31,12 @@
 #define PICO_PIO_VERSION 0
 #endif
 #endif
+
+// PICO_CONFIG: PICO_PIO_CLKDIV_ROUND_NEAREST, True if floating point PIO clock divisors should be rounded to the nearest possible clock divisor rather than rounding down, type=bool, default=PICO_CLKDIV_ROUND_NEAREST, group=hardware_pio
+#ifndef PICO_PIO_CLKDIV_ROUND_NEAREST
+#define PICO_PIO_CLKDIV_ROUND_NEAREST PICO_CLKDIV_ROUND_NEAREST
+#endif
+
 /** \file hardware/pio.h
  *  \defgroup hardware_pio hardware_pio
  *
@@ -472,10 +478,10 @@ static inline void sm_config_set_sideset(pio_sm_config *c, uint bit_count, bool
  * \sa sm_config_set_clkdiv()
  */
 static inline void sm_config_set_clkdiv_int_frac8(pio_sm_config *c, uint32_t div_int, uint8_t div_frac8) {
-    static_assert(PIO_SM0_CLKDIV_INT_MSB - PIO_SM0_CLKDIV_INT_LSB == 15, "");
+    static_assert(REG_FIELD_WIDTH(PIO_SM0_CLKDIV_INT) == 16, "");
     invalid_params_if(HARDWARE_PIO, div_int >> 16);
     invalid_params_if(HARDWARE_PIO, div_int == 0 && div_frac8 != 0);
-    static_assert(PIO_SM0_CLKDIV_FRAC_MSB - PIO_SM0_CLKDIV_FRAC_LSB == 7, "");
+    static_assert(REG_FIELD_WIDTH(PIO_SM0_CLKDIV_FRAC) == 8, "");
     c->clkdiv =
             (((uint)div_frac8) << PIO_SM0_CLKDIV_FRAC_LSB) |
             (((uint)div_int) << PIO_SM0_CLKDIV_INT_LSB);
@@ -488,14 +494,18 @@ static inline void sm_config_set_clkdiv_int_frac(pio_sm_config *c, uint16_t div_
 
 static inline void pio_calculate_clkdiv8_from_float(float div, uint32_t *div_int, uint8_t *div_frac8) {
     valid_params_if(HARDWARE_PIO, div >= 1 && div <= 65536);
+    const int frac_bit_count = REG_FIELD_WIDTH(PIO_SM0_CLKDIV_FRAC);
+#if PICO_PIO_CLKDIV_ROUND_NEAREST
+    div += 0.5f / (1 << frac_bit_count); // round to the nearest 1/256
+#endif
     *div_int = (uint16_t)div;
     // not a strictly necessary check, but if this changes, then this method should
     // probably no longer be used in favor of one with a larger fraction
-    static_assert(PIO_SM0_CLKDIV_FRAC_MSB - PIO_SM0_CLKDIV_FRAC_LSB == 7, "");
+    static_assert(REG_FIELD_WIDTH(PIO_SM0_CLKDIV_FRAC) == 8, "");
     if (*div_int == 0) {
         *div_frac8 = 0;
     } else {
-        *div_frac8 = (uint8_t)((div - (float)*div_int) * (1u << 8u));
+        *div_frac8 = (uint8_t)((div - (float)*div_int) * (1u << frac_bit_count));
     }
 }
 
@@ -1675,10 +1685,10 @@ void pio_sm_drain_tx_fifo(PIO pio, uint sm);
 static inline void pio_sm_set_clkdiv_int_frac8(PIO pio, uint sm, uint32_t div_int, uint8_t div_frac8) {
     check_pio_param(pio);
     check_sm_param(sm);
-    static_assert(PIO_SM0_CLKDIV_INT_MSB - PIO_SM0_CLKDIV_INT_LSB == 15, "");
+    static_assert(REG_FIELD_WIDTH(PIO_SM0_CLKDIV_INT) == 16, "");
     invalid_params_if(HARDWARE_PIO, div_int >> 16);
     invalid_params_if(HARDWARE_PIO, div_int == 0 && div_frac8 != 0);
-    static_assert(PIO_SM0_CLKDIV_FRAC_MSB - PIO_SM0_CLKDIV_FRAC_LSB == 7, "");
+    static_assert(REG_FIELD_WIDTH(PIO_SM0_CLKDIV_FRAC) == 8, "");
     pio->sm[sm].clkdiv =
             (((uint)div_frac8) << PIO_SM0_CLKDIV_FRAC_LSB) |
             (((uint)div_int) << PIO_SM0_CLKDIV_INT_LSB);

src/rp2_common/hardware_pwm/include/hardware/pwm.h

@@ -103,6 +103,11 @@ static_assert(DREQ_PWM_WRAP7 == DREQ_PWM_WRAP0 + 7, "");
 })
 #endif
 
+// PICO_CONFIG: PICO_PWM_CLKDIV_ROUND_NEAREST, True if floating point PWM clock divisors should be rounded to the nearest possible clock divisor rather than rounding down, type=bool, default=PICO_CLKDIV_ROUND_NEAREST, group=hardware_pwm
+#ifndef PICO_PWM_CLKDIV_ROUND_NEAREST
+#define PICO_PWM_CLKDIV_ROUND_NEAREST PICO_CLKDIV_ROUND_NEAREST
+#endif
+
 static inline void check_slice_num_param(__unused uint slice_num) {
     valid_params_if(HARDWARE_PWM, slice_num < NUM_PWM_SLICES);
 }
@@ -155,7 +160,11 @@ static inline void pwm_config_set_phase_correct(pwm_config *c, bool phase_correc
  */
 static inline void pwm_config_set_clkdiv(pwm_config *c, float div) {
     valid_params_if(HARDWARE_PWM, div >= 1.f && div < 256.f);
-    c->div = (uint32_t)(div * (float)(1u << PWM_CH0_DIV_INT_LSB));
+    const int frac_bit_count = REG_FIELD_WIDTH(PWM_CH0_DIV_FRAC);
+#if PICO_PWM_CLKDIV_ROUND_NEAREST
+    div += 0.5f / (1 << frac_bit_count); // round to the nearest fraction
+#endif
+    c->div = (uint32_t)(div * (float)(1u << frac_bit_count));
 }
 
 /** \brief Set PWM clock divider in a PWM configuration using an 8:4 fractional value
@@ -170,9 +179,9 @@ static inline void pwm_config_set_clkdiv(pwm_config *c, float div) {
  * before passing them on to the PWM counter.
  */
 static inline void pwm_config_set_clkdiv_int_frac4(pwm_config *c, uint32_t div_int, uint8_t div_frac4) {
-    static_assert(PWM_CH0_DIV_INT_MSB - PWM_CH0_DIV_INT_LSB == 7, "");
+    static_assert(REG_FIELD_WIDTH(PWM_CH0_DIV_INT) == 8, "");
     valid_params_if(HARDWARE_PWM, div_int >= 1 && div_int < 256);
-    static_assert(PWM_CH0_DIV_FRAC_MSB - PWM_CH0_DIV_FRAC_LSB == 3, "");
+    static_assert(REG_FIELD_WIDTH(PWM_CH0_DIV_FRAC) == 4, "");
     valid_params_if(HARDWARE_PWM, div_frac4 < 16);
     c->div = (((uint)div_int) << PWM_CH0_DIV_INT_LSB) | (((uint)div_frac4) << PWM_CH0_DIV_FRAC_LSB);
 }
@@ -439,7 +448,7 @@ static inline void pwm_retard_count(uint slice_num) {
 static inline void pwm_set_clkdiv_int_frac4(uint slice_num, uint8_t div_int, uint8_t div_frac4) {
     check_slice_num_param(slice_num);
     valid_params_if(HARDWARE_PWM, div_int >= 1);
-    static_assert(PWM_CH0_DIV_FRAC_MSB - PWM_CH0_DIV_FRAC_LSB == 3, "");
+    static_assert(REG_FIELD_WIDTH(PWM_CH0_DIV_FRAC) == 4, "");
     valid_params_if(HARDWARE_PWM, div_frac4 < 16);
     pwm_hw->slice[slice_num].div = (((uint)div_int) << PWM_CH0_DIV_INT_LSB) | (((uint)div_frac4) << PWM_CH0_DIV_FRAC_LSB);
 }