Replace wait_us() calls in ST I2C HAL driver with RTOS-less/pure-C waits

[RobMeades]

Description

Replace the calls to wait_us() in i2c_api.c for the STM platforms with a microsecond-ticker-based wait (hence becoming RTOS-less and all pure-C code). This is required because the UBLOX_C030 family of boards (which use an STM32F437VG MCU) need to configure a voltage threshold via I2C for reliable operation. Such Mbed target configuration can only be done in the HAL_MspInit() hook, which is called before either the RTOS or LIBC have been initialised.

Status

Tested on C030 platform, @ARMmbed/team-st-mcd to regression test on other ST platforms.

Related PRs

#5957, #5995, #6034

Steps to test or reproduce

Run mbed test and the test mbed_platform-critical_section will now pass on a UBLOX_C030 platform.

Test results attached (noting that the UDP test always fail on our network).

waitless_i2c_test_output.txt

CC @ARMmbed/team-ublox.

[0xc0170]

Thanks @RobMeades for the fix

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 1123
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6069/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

[LMESTM]

Few questions about this lines

• us_ticker_read returns uint32_t - why using start as an int ?
• what would be the resulting type of (us_ticker_read() - start) and how to make sure that wrap-around case is well covered ? rather than < 1,maybe != 0 would be safer
• In any case, with current proposal, the start will happen any time between T0 and T0+1, so when ((us_ticker_read() - start) < 1) is true, we have not reached a 1µs time, but rather any time between 0 and 1µs, in random way, so that the result will probably be around 0,5µs in average. The actual measured timeout will be half the expected one. Most probably OK as we take margins in the timeout computation, nevertheless not what we expect to code.

[RobMeades]

Happy to use uint32_t instead of int, was just being lazy, though actually if they were both signed then at least the wrap-around case would return immediately rather than get stuck. But rather than dwelling here I'll comment on your other remark below.

[LMESTM]

@RobMeades @0xc0170 @jeromecoutant I've added questions on the code - I'm not sure that it does what we actually expect.
@RobMeades Note that if you move your battery driver to use i2c_byte_read and i2c_byte_write, you would not need this change anymore ...

[RobMeades]

@LMESTM: good point about switching to i2c_byte_read() and i2c_byte_write() but presumably we would need the same construction just up in our I2C code rather than down in the I2C driver layer? That would be localised to our platform, though, which I think is desirable rather than mucking about with the generic I2C code. We will investigate...

[mbed-ci]

Test : SUCCESS

Build number : 930
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6069/930

[LMESTM]

@RobMeades
the i2c_byte_read and i2c_byte_write are constantly polling on I2C registers. You don't need to manage a timeout in your code, this is done in the i2c driver using a fixed number of loops rather than wait, which is easy to do because we're always reading or writing 1 single byte.
The more complete I2C API (the one you use today) is based on I2C interrupts instead of registers polling, and the wait loop is done on a variable not registers, which is why I wanted to release some CPU cycles to let application run by using the wait calls.

[RobMeades]

@LMESTM: understood, @bqam-ublox is looking at the implementation now. Assuming it works out we can close this and submit a PR which addresses #5957, #5995, #6034 and #6011 all at once. Boo-yaa :-).

[mbed-ci]

Exporter Build : FAILURE

Build number : 800
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6069/

[LMESTM]

Boo-yaa :-).

fingers crossed ;-)

[0xc0170]

@LMESTM: understood, @bqam-ublox is looking at the implementation now. Assuming it works out we can close this and submit a PR which addresses #5957, #5995, #6034 and #6011 all at once. Boo-yaa :-).

The bugfix for C030 I2C was integrated, is this PR still relevant @RobMeades ?

[RobMeades]

No longer required, #6117 is the right fix.