Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I2C object causes error in SPI communication #14742

Closed
idea--list opened this issue Jun 7, 2021 · 5 comments
Closed

I2C object causes error in SPI communication #14742

idea--list opened this issue Jun 7, 2021 · 5 comments

Comments

@idea--list
Copy link
Contributor

Description of defect

Discovered a totally unexpected behaviour which took me long to track back. My code is over 1600 lines long and i really can not share it this time. However that is what i am facing:

  • My main peripheral (1 sensor) is connected over SPI. It samples data in it's own FIFO and and sends an interrupt to the MCU when half of the FIFO is filled up.

  • The MCU fetches all the values of the FIFO via SPI and saves those to an SD card (via the same SPI bus)

  • I have another sensor that connects via I2C to the MCU

  • All this worked for almost 2 years now.

  • Recently however i activated a second interrupt of the SPI sensor, which is triggered when the sensor acquires an outlier value.

  • Both interrupts are signalized to the MCU over the same pin.

  • Problem is that after the newly added interrupt gets triggered ca. 5-6 times the SPI sensor stops any further FIFO activity thus the whole program hangs up.

  • While tracking down the issue i commented out all the I2C specific code except for the instantiation of my I2C object without any effect. Also tried to place i2c.stop() at the beginning of main() but doing so the hang-up occurred even earlier.

  • As a last thing to try i also commented out the instantiation of the I2C object ... and that way both interrupts work, the SPI sensor never locks up ... i just can not use my I2C sensor at all.

All this makes me wonder why can i use those 2 buses with 1 interrupt or 1 bus with 2 interrupts, but why things just break on 2 completely different targets if i try to use 2 buses with 2 interrupts?

Not sure if #14735 or #14004 might be related to this in any way

Target(s) affected by this defect ?

Artemis Thing Plus, MAX32630FTHR

Toolchain(s) (name and version) displaying this defect ?

GNU Arm Embedded Toolchain 8-2018-q4-major

What version of Mbed-os are you using (tag or sha) ?

Mbed OS V6.11

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Mbed Studio V1.4.1

How is this defect reproduced ?

Try communicating with 2 sensors (1 SPI +1 I2C) and activate 2 interrupt sources on the SPI sensor.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 8, 2021

It would be ideal to create an example where 2 sensors are connected and the issue can be reproduced.
It might be worth testing on another target (different MCU family) - is this a generic a problem in the driver layer or down to HAL/MCU?

@jeromecoutant
Copy link
Collaborator

@idea--list
Copy link
Contributor Author

idea--list commented Jun 9, 2021

@0xc0170 I can not test it on a different processor family as i only have those 2 boards that are compatible with Mbed. Will consider sharing a super simplified example, however i only have the MAX30003 sensor that communicates over SPI and you might not have that. If this would turn out to be specific to that sensor, then sharing any simplified code will not help much.

@jeromecoutant thanks for the suggestion but i could not make that work with Mbed Studio and CLI freaks me out, furthermore in the readme.md of ci-test-shield i have found this:
use the mbed HDK to modify the design to match the headers on your board.
But that link shows a 404 page and thus i have no clue how should i customize it to work with my targets. Have never done anything like that before. (Once i tried greentea tests but it did not work as documented and gave up)

Some more info that will be helpful to identify the issue:

  • I am sampling data with the MAX30003 chip at 512Hz.
  • That chip has an internal FIFO that has a capacity of storing 32 values (24-bit each of which 16-bit is the actual value and there are also some other bits that contain metadata like the measurement and timestamp is valid, has the FIFO overflown, was there a premature SPI connection, etc.)
  • The chip has many settings one of which is the FIFO interrupt threshold value

In case i comment out my I2C object as reported yesterday i can set the FIFO interrupt threshold level to 32. Meaning i can wait until the FIFO is completely full before the MCU will grab those data. With both the FIFO full interrupt and R-peak detection interrupt my code runs without any glitches for long periods of time. For sure processing all that puts some load on the MCU, but i never got any values out of the MAX30003 chip that would have been tagged as invalid or would have contained FIFO overflow flag (but by adding ThisThread::sleep_for() to the loop for testing purposes i can confirm that this feature of the sensor works).

I enabled the R-peak detection interrupt on the sensor some time ago as i noticed that not all the R-peaks will get detected if i only rely on the STATUS register of MAX30003 (i guess it is because the workload might be quite high on the MCU in my use case), but then this second interrupt introduced the reported problem. It was hard to narrow down what might be causing that as the hang-ups occurred only on the 8th or 9th R-peak interrupt but until that time the sampling worked even at 512Hz even in the presence of an I2C object...

Today i made further tests and found that if i set the FIFO threshold value down to just 4 (out of 32) slots on the MAX30003 chip, then i can sample at 512Hz for extended periods of time with both interrupts active while also an I2C object can be instantiated.

All this means:

  • Without an I2C object the MCU can handle 2 interrupts, one with constant and another one with irregular intervals within just 2 milliseconds (otherwise i would get FIFO overflow warnings while sampling at 512Hz and allowing to fill the FIFO completely before the FIFO interrupt)
  • If also an I2C object is instantiated, i have to trigger the FIFO interrupt as soon as only 4 slots of the FIFO are occupied, which allows up to 56 milliseconds for the MCU to grab&process the content of the FIFO. So the root cause seems to be an unexpectedly high impact on performance of I2C as reported in Unable to work with two physical I2C buses #14735 even if no I2C methods are used at all. The mere existence of an I2C object is enough to introduce the issue, which is quite unexpected i guess and can render some use cases impossible.

@idea--list
Copy link
Contributor Author

idea--list commented Jun 15, 2021

Spent one more week with this issue.
Refactored a part of my code (was using printf() inside a for loop, which was not optimal especially as the existence of an I2C seems to overload the MCU). My refactored code just aggregates a string inside the for loop and printf() only once after the loop. For sure this reduces load on the MCU but even after that i was still facing the problem.

After trying a great number of combinations and observing results, i realized i will need to take a compromise and just destroy the I2C object while recording ECG data via SPI... for my surprise i found that mbed-os/drivers/include/drivers/I2C.h defines a destructor that does nothing:

virtual ~I2C()
    {
        // Do nothing
    }

While the board specific mbed-os/targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/i2c_api.c offers this function:

void i2c_free(i2c_t *obj)
{
    MBED_ASSERT(obj);
    iom_deinit(&obj->i2c.iom_obj);
}

So i ended up modifying mbed-os/drivers/include/drivers/I2C.h like that:

virtual ~I2C()
    {
        i2c_free(&_i2c);
    }

This way the MCU does not struggle anymore and can keep up collecting data via SPI at 512Hz easily, meaning it can also detect all the R-peaks just by reading the STATUS register of MAX30003. So i do not need to activate interrupt generation to detect all the R-peaks, which has somehow interfered with the I2C object and lead to hang ups...

Though destructing the I2C object renders any I2C sensors unusable while the SPI sensor is delivering the stream. Once the SPI finishes to work, i can reinstantiate the I2C object and the I2C sensor delivers data again. This is still not optimal, but allows for normal SPI functionality if one can make such a compromise.

Also took a superficial look at the i2c_api.c file of other targets: a few of them also offer the i2c_free(), while others do not define that function... which might explain why the core I2C.h prefers to do nothing out of the box.

IMHO it would be great to make use of i2c_free() inside the destructor at least for those targets, that define that function in the target specific files. Do not know how one could check for the existence of that function and do something like this in I2C.h:

virtual ~I2C()
    {
        if (function 'i2c_free' exists ) {
           i2c_free(&_i2c);
        }
    }

Hope all this info is helpful to further evaluate the issue and the team can come up with a more general solution or document this issue at least.

@idea--list
Copy link
Contributor Author

#14805 fixes this issue.

Issue Workflow automation moved this from Needs Triage to Done Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants