arduino / ArduinoCore-mbed

330 stars 195 forks source link

I2C Clock on the Giga Only Supports 100khz, 400khz and 1mhz. No intermediate frequencies Supported #798

Open mjs513 opened 9 months ago

mjs513 commented 9 months ago

See https://forum.arduino.cc/t/bno086-does-not-work-at-i2c-clock-100mhz/1200365. But quoting from that:

While testing the BNO086 with the Arduino Giga I noticed that setting the I2C clock at 400khz would cause the BNO086 to not sent any of reports. Tested with a couple of examples.

Did find that Sparkfun recommended running at a I2c Clock of 350khz instead of 400khz for reliable operation: MCU keeps resetting the sensor · Issue #2 · sparkfun/SparkFun_BNO08x_Arduino_Library · GitHub as a work around.

However if I try any other clock > 100khz or <400khz (specifically I tried 200khz and 350Khz) I get the "red blinking LED of doom". So my guess is that nothing but 100 and 400khz is supported by the giga.

So the question is how can I setClock for clocks other than 100 and 400khz?

Just for reference I was able to setClock to 350khz on a Teensy. Had to make sure it wasn't me.

mjs513 commented 9 months ago

what was posted is deleted as incorrect - see later comment

mjs513 commented 9 months ago

This is an interesting article: https://hackaday.com/2022/05/11/bare-metal-stm32-using-the-i2c-bus-in-master-transceiver-mode/

which shows that"

At this point in time, the Nodate framework I2C implementation for STM32 uses both, with the predefined values as above for STM32F0, and dynamically calculated values for other families.

basically shows how to set intermediate values

mjs513 commented 9 months ago

@facchinm - think you had a hand it fixing i2c in the referenced issue.

Doing a trace found the following: In mbed_config.h calculating the I2C timing from the requested frequency is enabled: #define MBED_CONF_TARGET_I2C_TIMING_VALUE_ALGO 1

Tracing that it appears in ArduinoCore-mbed/cores/arduino/mbed/targets/TARGET_STM/TARGET_STM32H7/i2c_device.h

uint32_t i2c_get_pclk(I2CName i2c);
uint32_t i2c_get_timing(I2CName i2c, uint32_t current_timing, int current_hz, int hz);

#if MBED_CONF_TARGET_I2C_TIMING_VALUE_ALGO
uint32_t i2c_compute_timing(uint32_t clock_src_freq, uint32_t i2c_freq);
void i2c_compute_presc_scldel_sdadel(uint32_t clock_src_freq, uint32_t I2C_speed);
uint32_t i2c_compute_scll_sclh(uint32_t clock_src_freq, uint32_t I2C_speed);
#endif // MBED_CONF_TARGET_I2C_TIMING_VALUE_ALGO

in i2c_device.c for the H7 (targets/TARGET_STM/TARGET_STM32H7/i2c_device.c in the mbedos repo you can see the calculation https://github.com/arduino/mbed-os/blob/dc2d8607a3f29231d962c42729b191cf0af1baa9/targets/TARGET_STM/TARGET_STM32H7/i2c_device.c

so technically it should be calculating the setting for frequency at 350khz.

i2c_device.h is referenced in ArduinoCore-mbed/cores/arduino/mbed/targets/TARGET_STM/stm_i2c_api.h

Not sure if this is associated with this issue on the portenta: https://github.com/arduino/ArduinoCore-mbed/issues/89

Additional info I did attach a jlink for debugging and all that can confirm is that the red lights of doom happen when setClock(350000) is called which happens in Wire.cpp: master->frequency(freq);

don;t think I can do much more.

mjs513 commented 9 months ago

Did a bit more digging. Found that for the GIGA:

/*  Define IP version */
#define I2C_IP_VERSION_V2

which is used in the i2c_api.c ( mbed-os/targets/TARGET_STM/i2c_api.c at 3636262ae786ae623451985dd40c7a54c40b9ab1 · arduino/mbed-os (github.com) ) shows that only 100Khz, 400khz and 1Mhz is supported:

    /*  Only predefined timing for below frequencies are supported */
    MBED_ASSERT((hz == 100000) || (hz == 400000) || (hz == 1000000));

    /* Derives I2C timing value with respect to I2C input clock source speed
    and I2C bus frequency requested. "Init.Timing" is passed to this function to
    reduce multiple computation of timing value which there by reduces CPU load.
    */
    handle->Init.Timing = i2c_get_timing(obj_s->i2c, handle->Init.Timing, \
                                         obj_s->current_hz, hz);
    /* Only non-zero timing value is supported */
    MBED_ASSERT(handle->Init.Timing != 0);

    /* hz value is stored for computing timing value next time */
    obj_s->current_hz = hz;
#endif // I2C_IP_VERSION_V2

which makes no sense since you are calling i2c_get_timing anyway.