From 7f829f9e448a4336be92fce19a4b1193c2c441d3 Mon Sep 17 00:00:00 2001 From: Michal Sieron Date: Mon, 23 Jan 2023 18:34:37 +0100 Subject: [PATCH] libbase/i2c: use busy_wait_us instead of cdelay `cdelay` function is not a proper thing to count time. It wouldn't count SoC clocks, but CPU clocks. But even then, there are multiple instructions in `cdelay`: - NOP - decrement - branch with compare Assuming each instruction takes exactly 1 CPU cycle it is still wrong, as we wait 3 time longer than requested. But they don't take exactly 1 CPU cycle. CPUs have caches, branch predictors, are out-of-order and so on. So a much better way to count this time would be `busy_wait_us`. I performed some test using vexriscv and Saleae Logic Analyzer: vexriscv variant | requested I2C speed | actual (cdelay) | actual (busy_wait_us) -----------------+---------------------+-----------------+---------------------- minimal | 50 kHz | 4 kHz | 38 kHz minimal | 200 kHz | 15 kHz | 96 kHz minimal | 400 kHz | 28 kHz | 137 kHz -----------------+---------------------+-----------------+---------------------- lite | 50 kHz | 12 kHz | 40 kHz lite | 200 kHz | 43 kHz | 115 kHz lite | 400 kHz | 74 kHz | 180 kHz -----------------+---------------------+-----------------+---------------------- standard | 50 kHz | 12 kHz | 45 kHz standard | 200 kHz | 48 kHz | 159 kHz standard | 400 kHz | 84 kHz | 311 kHz Signed-off-by: Michal Sieron --- litex/soc/software/libbase/i2c.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/litex/soc/software/libbase/i2c.c b/litex/soc/software/libbase/i2c.c index dd21b2b6d..a61eb68f2 100644 --- a/litex/soc/software/libbase/i2c.c +++ b/litex/soc/software/libbase/i2c.c @@ -7,11 +7,14 @@ #include #include +#include + #ifdef CONFIG_HAS_I2C #include -#define I2C_PERIOD_CYCLES (CONFIG_CLOCK_FREQUENCY / I2C_FREQ_HZ) -#define I2C_DELAY(n) cdelay((n)*I2C_PERIOD_CYCLES/4) +#define U_SECOND (1000000) +#define I2C_PERIOD (U_SECOND / I2C_FREQ_HZ) +#define I2C_DELAY(n) busy_wait_us(n * I2C_PERIOD / 4) int current_i2c_dev = DEFAULT_I2C_DEV; @@ -20,14 +23,6 @@ int get_i2c_devs_count(void) { return I2C_DEVS_COUNT; } void set_i2c_active_dev(int dev) { current_i2c_dev = dev; } int get_i2c_active_dev(void) { return current_i2c_dev; } -static inline void cdelay(int i) -{ - while(i > 0) { - __asm__ volatile(CONFIG_CPU_NOP); - i--; - } -} - int i2c_send_init_cmds(void) { #ifdef I2C_INIT