Nordic RF52 does not correctly support simultaneous use of I2C and SPI

[NeilMacMullen]

Mbed OS 5
Target UBLOX_EVK_NINA_B1
Toolchain GCC_ARM

The Nordic RF52 SoC uses digital 'TWI' control blocks to control both I2C and SPI. When declaring an I2C object, the Nordic platform code in targets/TARGET_NORDIC/TARGET_NRF5/i2c_api.c uses the method "i2c_init" to allocate the first available TWI by examining previously-allocated I2C instances. Similar code is used in the SPI drivers (spi_api.c) where previously-allocated SPI instances are checked.

This approach fails when allocating a mixture of I2C and SPI instances since both _init methods then consider TWI0 to be available and allocate it to both the I2C and SPI interfaces. This leads to unpredictable results when the interfaces are used.

If you only have a single I2C and SPI instance, it's possible to work around this by forcing one of the allocators to allocate TWI1 (example code below) but the correct fix would be to implement a central allocator which would be used for both types of interface.

diff --git a/targets/TARGET_NORDIC/TARGET_NRF5/i2c_api.c b/targets/TARGET_NORDIC/TARGET_NRF5/i2c_api.c
index 66063b4..ca551d4 100644
--- a/targets/TARGET_NORDIC/TARGET_NRF5/i2c_api.c
+++ b/targets/TARGET_NORDIC/TARGET_NRF5/i2c_api.c
@@ -283,6 +283,7 @@ static void twi_clear_bus(twi_info_t *twi_info)
     }
}
+#define I2C_TWI 1
void i2c_init(i2c_t *obj, PinName sda, PinName scl)
{
     int i;
@@ -290,7 +291,7 @@ void i2c_init(i2c_t *obj, PinName sda, PinName scl)
         if (m_twi_info[i].initialized &&
             m_twi_info[i].pselsda == (uint32_t)sda &&
             m_twi_info[i].pselscl == (uint32_t)scl) {
-            TWI_IDX(obj) = i;
+            TWI_IDX(obj) = I2C_TWI;
             TWI_INFO(obj)->frequency = NRF_TWI_FREQ_100K;
             i2c_reset(obj);
             return;
@@ -299,7 +300,7 @@ void i2c_init(i2c_t *obj, PinName sda, PinName scl)
 
     for (i = 0; i < TWI_COUNT; ++i) {
         if (!m_twi_info[i].initialized) {
-            TWI_IDX(obj) = i;
+            TWI_IDX(obj) = I2C_TWI;
 
             twi_info_t *twi_info = TWI_INFO(obj);
             twi_info->initialized = true;
 

Copying @MarceloSalazar by request

[0xc0170]

cc @nvlsianpu @pan-

[nvlsianpu]

Thank you for describing this bug.

[shayo]

@nvlsianpu, we are experiencing the same bug here as well.
Our board has both I2C and SPI.
When we define both SPI and I2C, none of them work.
When I remove the declaration of SPI, I2C works.

Is there a patch to solve this issue? I tried taking the latest code but the problem is still there.
This is a major blocker for us right now.

[shayo]

@NeilMacMullen , I also tried your solution (modifying the i2c_init), but it didn't work for me. Is there something else I can try? Which version did you base your patch on?

[nvlsianpu]

did you test the path #4491?

[shayo]

I've tried c416fdf, after you added the central resource manager. It still doesn't work (both I2C and SPI).

[shayo]

It seems like the system hangs with your branch when I call "wait".
This happens also when I use the main branch.
Are there also changes to us_ticker_read that I should take from somewhere?
This might also explain why the SPI code initialization hang.

[NeilMacMullen]

@nvlsianpu - apologies, I didn't get any notification of #4491 so have only just seen this change. @shayo - I will try this here and let you know whether it works for me.

[NeilMacMullen]

@nvlsianpu @shayo I agree that this seems at first sight to be broken. I did... "git pull origin pull/4491/head" in a new project then verified that the i2c_api file does include the latest changes then copied my application source to the new project and built it. In my case, the symptoms are that "i2c->write(address, NULL, 0);" returns 0 for any address, indicating that the entire bus is populated with devices. The same code without these changes (and with my original workaround) correctly identifies the few I2C devices I have on the bus.