ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

STM32WL I2C_1 and I2C_2: Unable to deep sleep #15191

Closed IoTopenTech closed 2 years ago

IoTopenTech commented 2 years ago

Description of defect

Using STM32WLE5C, I2C_3 is able to enter deep sleep, but I2C_1 and I2C_2 no.

Target(s) affected by this defect ?

STM32WL

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

ARM GCC

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

mbed-os-6.15.0

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

Mbed Studio: 1.4.3

How is this defect reproduced ?

The following code in https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/i2c_api.c for I2C_1 and I2C_2 seems to lock deepsleep

imagen

But there is no way to re-enable deepsleep (for example in a low power device that only needs to read a sensor once per hour).

This increases the power consumption in 3mA.

It would be useful to have a free or deinit method.

mbedmain commented 2 years ago

@IoTopenTech thank you for raising this issue.Please take a look at the following comments:

Could you add some more detail to the description? A good description should be at least 25 words.

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'. This indicates to us that at least all the fields have been considered. Please update the issue header with the missing information.

jeromecoutant commented 2 years ago

Yes, deepsleep is currently disabled during I2C init, and re-enabled in deinit... Maybe it should be implemented with more detailed conditions; Help would be required for that.

In the current implementation, I suppose that sensor should be "init" before each transmission..

jeromecoutant commented 2 years ago

Or deepsleep procedure could be re-implemented to save I2C registers before going in STOP mode, and then restored after?

https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/sleep.c#L166

IoTopenTech commented 2 years ago

Yes, deepsleep is currently disabled during I2C init, and re-enabled in deinit... Maybe it should be implemented with more detailed conditions; Help would be required for that.

In the current implementation, I suppose that sensor should be "init" before each transmission..

Thanks @jeromecoutant

The problem is that there is no way to deinit I2C in the I2C API (https://os.mbed.com/docs/mbed-os/v6.15/apis/i2c.html)

jeromecoutant commented 2 years ago

The problem is that there is no way to deinit I2C in the I2C API (https://os.mbed.com/docs/mbed-os/v6.15/apis/i2c.html)

Oh! I didn't know that! https://github.com/ARMmbed/mbed-os/blob/master/drivers/include/drivers/I2C.h#L189

You can have a look on https://github.com/ARMmbed/mbed-os/blob/master/hal/tests/TESTS/mbed_hal_fpga_ci_test_shield/i2c/main.cpp#L54

IoTopenTech commented 2 years ago

Sorry @jeromecoutant I'm not experienced enough with mbed as to know what to look on there (i2c_free of mbed_hal_fpga_ci_test_shield/i2c/main.cpp).

I think that there should be a free method in I2C.cpp; something like this void I2C::free(void) { lock(); i2c_free(&_i2c); unlock(); }

As there is already a i2c_free function in https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/i2c_api.c

imagen

That unlocks deep sleep in I2C_1 and I2C_2 imagen

Kind regards

LMESTM commented 2 years ago

Related issue : https://github.com/ARMmbed/mbed-os/issues/3106 I would suggest you make a local change to your project (similar to the one you propose) which will allow you to call to i2c_free / I2c_init as needed.

IoTopenTech commented 2 years ago

Thanks @LMESTM, I've made that local change and it seems to work properly (now the uC is deepsleeping).

0xc0170 commented 2 years ago

closing , being tracked in 3106. Thanks for the confirmation the local change fixed it.

JenertsA commented 2 years ago

Thanks @LMESTM, I've made that local change and it seems to work properly (now the uC is deepsleeping).

Hi,

got into the same situation. I am new to MbedOs so I tried to implement changes discussed here but seems like not 100% successfully.

I did add: void I2C::free(){ lock(); i2c_free(&_i2c); unlock(); } to I2C.cpp and use it before entering sleep and init in the next cycle.

Behaviour I get is unreliable. After some time (sometime after a single uplink, sometime after 20mins) MCU doesn't enter sleep mode and with MBED_SLEEP_TRACING_ENABLED is see that at some point deepsleep locked by: [i2c_api.c x 1] pops up and after even more time x1 changed to x188 and increasing.

jeromecoutant commented 2 years ago

Hi @JenertsA

As indicated in https://github.com/ARMmbed/mbed-os/issues/15191#issuecomment-990944395 In the current implementation + the patch, it is supposed that sensor should be "init" before each transmission and "reset" after.

When MCU doesn't enter into deepsleep any more, maybe a I2C "reset" is misisng ?

JenertsA commented 2 years ago

hmm, could you tell me more about where reset should be placed? I am currently initing and freeing inside each method that uses I2C.

saffetblt commented 2 years ago

Hi, did you find any solution to the problem? I'm having the same problem and I can't find a solution.

IoTopenTech commented 2 years ago

Hi @saffetblt

Yes I found a workaround. You can find more information here: https://www.thethingsnetwork.org/forum/t/rak3172-mbedos-help-randomly-not-going-to-deep-sleep/55979/12

JenertsA commented 2 years ago

Hi, did you find any solution to the problem? I'm having the same problem and I can't find a solution.

Hi! Yes, I did! Sorry for not posting here! My wrong doing was defining I2C instance globaly. You should define I2C in each function that uses the I2C an call free() in the function.

take a look here: https://www.thethingsnetwork.org/forum/t/rak3172-mbedos-help-randomly-not-going-to-deep-sleep/55979/11

saffetblt commented 2 years ago

Hi @JenertsA @IoTopenTech Thanks for the quick response. I tried this and it works fine.

hallard commented 1 year ago

Hi @JenertsA @IoTopenTech,

Thanks for the tips, I was able to solve my issues with the tips, but unfortunately as I'm using a secure element with mbed-cryptoauth this one prevented any deepsleep. I rewrited according wrapper to use same method, I mean:

But still no success, then I enabled MBED_SLEEP_TRACING_ENABLED to see what was going on. And found that just after a call to read ATECC Serial number wit atcab_read_serial_number() I got 21 deepsleep lock on i2c which mean that cryptoauthlib was doing some i2c stuff outside the wrapper function. And each time I was calling ATECC stuff the counter was incrementing (so somewhere library was not releasing lock)

Then following your recommandation I digged into mbed I2C stack and found 2 interesting stuff and explanation on why deep sleep is prevented on I2C1 and I2C2 on i2c_api.c

#if defined(TARGET_STM32WL)
        /* In Stop2 mode, I2C1 and I2C2 instances are powered down (only I2C3 register content is kept) */
        sleep_manager_lock_deep_sleep();
#endif

That means that by design, default I2C to use should be I2C3 to avoid all these issues (good to know for new design), but needs re routing our board that is not an option on the one we have.

So I decided to fix that other way, assuming I know what I'm doing so let me do what I need.

I commented out these 2 instance of sleep_manager_lock_deep_sleep();

#if defined(TARGET_STM32WL)
        /* In Stop2 mode, I2C1 and I2C2 instances are powered down (only I2C3 register content is kept) */
        //sleep_manager_lock_deep_sleep();
#endif

So this mean any sleep between I2C transfer will break everything because I2C register will be lost, but in majority of case I wake up, do I2C stuff then going to sleep, so It's my charge to prevent deep sleep when I'm working on.

and tada on each wake when I need to read sensors I call a function like that

void read_sensor(void) 
{
    // Init back I2C since was lost during deep sleep
    I2C i2c(I2C_SDA,I2C_SCL);

    // Prevent deep sleep when using I2C
    sleep_manager_lock_deep_sleep();

    // Sensor stuff
    Init_my_sensor(&i2c);

    // Do all your sensor stuff 
    // bla bla bla you can use sleep it will not go to deep sleep to prevent I2C lost

    // We done unlock deep sleep
    sleep_manager_unlock_deep_sleep();
}

quite easy, works with mbed-cryptoauth or any other. Just need 2 comments on mbed-os i2c_api.c, no need to call free or whatever, and you can have your I2C instance declared globally, you may just need to call back I2C init such as frequency each time you need to use I2C stuff of course.

Any hints are welcome

shiwzhi commented 7 months ago

I'm currently doing

class STM_I2C : public I2C {
    public:
        using I2C::I2C;
        ~STM_I2C() {
            i2c_free(&_i2c);
        }
    private:
};

And use it locally