From 13fe55df4c64754699369b84e0e7876f7e803242 Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Date: Wed, 15 Nov 2023 17:56:57 +0100
Subject: [PATCH] cpu/stm32/periph_spi: improve clock divider calculation

With only 8 possible clock dividers, we can just loop over the values
and shift the clock. In addition to being much easier to read, using
shifts over divisions can be a lot faster on CPUs without hardware
division.

In addition an `assert()` is added that checks if the API contract
regarding the SPI frequency is honored. If the requested clock is too
low to be generated, we should rather have a blown assertion than
hard to trace communication errors.
---
 cpu/stm32/periph/spi.c | 48 +++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/cpu/stm32/periph/spi.c b/cpu/stm32/periph/spi.c
index 59102e3f7728..0f0111719f17 100644
--- a/cpu/stm32/periph/spi.c
+++ b/cpu/stm32/periph/spi.c
@@ -28,7 +28,6 @@
 
 #include <assert.h>
 
-#include "bitarithm.h"
 #include "mutex.h"
 #include "periph/gpio.h"
 #include "periph/spi.h"
@@ -80,33 +79,24 @@ static inline bool _use_dma(const spi_conf_t *conf)
 }
 #endif
 
-/**
- * @brief Multiplier for clock divider calculations
- *
- * Makes the divider calculation fixed point
- */
-#define SPI_APB_CLOCK_SHIFT          (4U)
-#define SPI_APB_CLOCK_MULT           (1U << SPI_APB_CLOCK_SHIFT)
-
 static uint8_t _get_clkdiv(const spi_conf_t *conf, uint32_t clock)
 {
     uint32_t bus_clock = periph_apb_clk(conf->apbbus);
-    /* Shift bus_clock with SPI_APB_CLOCK_SHIFT to create a fixed point int */
-    uint32_t div = (bus_clock << SPI_APB_CLOCK_SHIFT) / (2 * clock);
-    DEBUG("[spi] clock: divider: %"PRIu32"\n", div);
-    /* Test if the divider is 2 or smaller, keeping the fixed point in mind */
-    if (div <= SPI_APB_CLOCK_MULT) {
-        return 0;
-    }
-    /* determine MSB and compensate back for the fixed point int shift */
-    uint8_t rounded_div = bitarithm_msb(div) - SPI_APB_CLOCK_SHIFT;
-    /* Determine if rounded_div is not a power of 2 */
-    if ((div & (div - 1)) != 0) {
-        /* increment by 1 to ensure that the clock speed at most the
-         * requested clock speed */
-        rounded_div++;
-    }
-    return rounded_div > BR_MAX ? BR_MAX : rounded_div;
+
+    uint8_t div = 0;
+    uint32_t divided_clock = bus_clock >> 1;
+    const uint8_t div_max =  SPI_CR1_BR_Msk >> SPI_CR1_BR_Pos;
+    for (; (divided_clock > clock) && (div < div_max); div++) {
+        divided_clock >>= 1;
+    }
+
+    /* If the callers asks for an SPI frequency of at most x, bad things will
+     * happen if this cannot be met. So let's have a blown assertion
+     * rather than runtime failures that require a logic analyzer to
+     * debug. */
+    assert(divided_clock <= clock);
+
+    return div;
 }
 
 void spi_init(spi_t bus)
@@ -239,11 +229,11 @@ void spi_acquire(spi_t bus, spi_cs_t cs, spi_mode_t mode, spi_clk_t clk)
     }
     uint8_t br = dividers[bus];
 
-    DEBUG("[spi] acquire: requested clock: %"PRIu32", resulting clock: %"PRIu32
-          " BR divider: %u\n",
+    DEBUG("[spi] acquire: requested clock: %" PRIu32
+         " Hz, resulting clock: %" PRIu32 " Hz, BR divider: %u\n",
           clk,
-          periph_apb_clk(spi_config[bus].apbbus)/(1 << (br + 1)),
-          br);
+          periph_apb_clk(spi_config[bus].apbbus) >> (br + 1),
+          (unsigned)br);
 
     uint16_t cr1_settings = ((br << BR_SHIFT) | mode | SPI_CR1_MSTR | SPI_CR1_SPE);
     /* Settings to add to CR2 in addition to SPI_CR2_SETTINGS */