search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.17k stars 6.23k forks source link

drivers: rtc: nucleo_wb55rg: unable to setup RTC alarm callback after 1st exit from low-power mode #68673

Closed krisnaresi closed 4 months ago

krisnaresi commented 5 months ago

Describe the bug

I tried to implement Internal RTC using EVK ST Nucleo WB55RG, but always fail when set the RTC time with return error code -EIO.

This is my main code:

#include <zephyr/drivers/rtc.h>
#include <zephyr/sys/timeutil.h>
#include <zephyr/device.h>

static const struct device *rtcDev = DEVICE_DT_GET(DT_ALIAS(rtc));

int main(void)
{
    struct rtc_time datetime_set;
    time_t timer_set = (1707208172UL);
    gmtime_r(&timer_set, (struct tm *)(&datetime_set));
    printk("Time: %02d:%02d:%02d\n", datetime_set.tm_hour, datetime_set.tm_min, datetime_set.tm_sec);
    printk("Date: %02d/%02d/%04d\n", datetime_set.tm_mday, datetime_set.tm_mon, datetime_set.tm_year);
    int err = rtc_set_time(rtcDev, &datetime_set);
    if(err == 0) {
        printk("Success to set time\n");
    } else {
        printk("Fail to set time: %i\n", err);
    }

    struct rtc_time datetime_get;
    err = rtc_get_time(rtcDev, &datetime_get);
    if(err == 0) {
        printk("Success to get time\n");
        printk("%02d:%02d:%02d\n", datetime_get.tm_hour, datetime_get.tm_min, datetime_get.tm_sec);
        printk("%02d/%02d/%04d\n", datetime_get.tm_mday, datetime_get.tm_mon, datetime_get.tm_year);
    } else {
        printk("Fail to get time: %i\n", err);
    }

    while (1) {
        k_sleep(K_SECONDS(5));
    }

    return 0;
}

I create my own board with name stm32wb_sensor. Here is the dts file:

/ {
    model = "STM32WB sensor board";
    compatible = "st,stm32wb55rg-nucleo";

    chosen {
        zephyr,console = &usart1;
        zephyr,shell-uart = &usart1;
        zephyr,bt-mon-uart = &usart1;
        zephyr,bt-c2h-uart = &usart1;
        zephyr,sram = &sram0;
        zephyr,flash = &flash0;
        zephyr,code-partition = &slot0_partition;
    };

    aliases {
        rtc = &rtc;
    };
};

&rtc {
    clocks = <&rcc STM32_CLOCK_BUS_APB1 0x00000400>,
             <&rcc STM32_SRC_LSI RTC_SEL(2)>;
    status = "okay";

    backup_regs {
        status = "okay";
    };
};

Here is the final RTC node on zephyr.dts file on build folder:

        rtc: rtc@40002800 {
            compatible = "st,stm32-rtc";
            reg = < 0x40002800 0x400 >;
            interrupts = < 0x29 0x0 >;
            clocks = < &rcc 0x58 0x400 >, < &rcc 0x4 0x26890 >;
            prescaler = < 0x8000 >;
            status = "okay";
            bbram: backup_regs {
                compatible = "st,stm32-bbram";
                st,backup-regs = < 0x14 >;
                status = "okay";
            };
        };

Here is my prj.conf that related to RTC:

# RTC
CONFIG_RTC=y

My Observation

After digging down into the kernel driver, found out the -EIO is come from zephyr/drivers/rtc/rtc_ll_stm32.c on rtc_stm32_enter_initialization_mode function at line 87. I think LL_RTC_IsActiveFlag_INIT(RTC) never return true.

Aside from that, I found that rtc_stm32_init on the same file is never being called, even though it's registered using macro DEVICE_DT_INST_DEFINE(0, &rtc_stm32_init, NULL, &rtc_data, &rtc_config, PRE_KERNEL_1, CONFIG_RTC_INIT_PRIORITY, &rtc_stm32_driver_api); I suppose rtc_stm32_init should be called at least once.

To Reproduce

Steps to reproduce the behavior:

  1. Create docker_shell.sh 
    
DOCKER_ARGS="--privileged \
--rm=true \
--user="$(id --user):$(id --group)" \
-e ZEPHYR_SDK_INSTALL_DIR=/opt/toolchains/zephyr-sdk-0.16.5 \
-e CCACHE_DIR=$(pwd)/.ccache \
-v $(pwd):$(pwd) \
-w $(pwd)"
DOCKER_IMAGE=zephyrprojectrtos/ci:v0.26.7

docker run $DOCKER_ARGS $DOCKER_IMAGE "$@"

2. `./docker_shell.sh west build -b stm32wb_sensor --sysbuild <path_to_app>`
3. See error (see below on logs)

**Expected behavior**
When calling `rtc_set_time`, I expect the return value to be 0, not -5 (EIO).

**Impact**
It's blocking my task.

**Logs and console output**

Time: 08:29:32 Date: 06/01/0124 Fail to set time: -5 Success to get time 00:00:01 01/00/0100



**Environment (please complete the following information):**
 - OS: Host: Linux Ubuntu 18.04, Docker: zephyrprojectrtos/ci:v0.26.7
 - Toolchain: Zephyr SDK v0.16.5
 - Zephyr RTOS v3.5.0

I hope this information is complete enough to spark further idea. Any help or pointer to resolve the problem is pretty much appreciated. Thank you for your time and suggestion.

Krisna
github-actions[bot] commented 5 months ago

Hi @krisnaresi! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

ajarmouni-st commented 5 months ago

Hi @krisnaresi, thank you for the detailed report. I am not able to reproduce the problem on nucleo_wb55rg, I used your code as it is with Zephyr v3.5.0 & I got the following on the console:

*** Booting Zephyr OS build zephyr-v3.5.0-1-g7cc91e397911 ***
Time: 08:29:32
Date: 06/01/0124
Success to set time
Success to get time
08:29:32
06/01/0124

Although, when I ran tests/drivers/rtc/rtc_api on the board, test_y2k didn't pass, on both v3.5.0 & v3.6.0-rc2:

Running TESTSUITE rtc_api
===================================================================
START - test_set_get_time
I: Setting clock
 PASS - test_set_get_time in 0.003 seconds
===================================================================
START - test_time_counting
I: Setting clock
 PASS - test_time_counting in 10.507 seconds
===================================================================
START - test_y2k

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/rtc/rtc_api/src/test_y2k.c:43: rtc_api_testo

 FAIL - test_y2k in 0.014 seconds
===================================================================
TESTSUITE rtc_api failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL -  66.67% [rtc_api]: pass = 2, fail = 1, skip = 0, total = 3 duration = 10.524 seconds
 - PASS - [rtc_api.test_set_get_time] duration = 0.003 seconds
 - PASS - [rtc_api.test_time_counting] duration = 10.507 seconds
 - FAIL - [rtc_api.test_y2k] duration = 0.014 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION FAILED
krisnaresi commented 5 months ago

Hi @ajarmouni-st, thank you for prompt answer. After further investigation, the reason why it cannot work on my side is because on prj.conf, I set CONFIG_PM=y. If I remove this config variable, then it works too on my side. Now, I need this to be set for low power mode scenario later on. Do you have any idea how to make the RTC work with CONFIG_PM set on? I see several examples here but didn't find any RTC example. Sorry if on my last post I'm not post full prj.conf, this project is not an open source for now.

ajarmouni-st commented 5 months ago

Do you have any idea how to make the RTC work with CONFIG_PM set on?

I will investigate & get back to you. Just one question, are you planning on using the RTC as a wake-up source?

krisnaresi commented 5 months ago

Do you have any idea how to make the RTC work with CONFIG_PM set on?

I will investigate & get back to you. Just one question, are you planning on using the RTC as a wake-up source?

Yes, we have plan to use RTC as a wake-up source. After digging down, I start to realize that RTC_ALARM is not yet implemented on rtc_ll_stm32.c. It would be great if ST team can implement that for doing wake up routine. Thank you for your support.

ajarmouni-st commented 5 months ago

After digging down, I start to realize that RTC_ALARM is not yet implemented on rtc_ll_stm32.c.

RTC_ALARM is implemented in https://github.com/zephyrproject-rtos/zephyr/blob/ded0d48779e2dbf2f9b926b75ddcbb7c8a2b6268/drivers/counter/counter_ll_stm32_rtc.c

and it's not yet available in rtc_ll_stm32.c as it is a recent driver.

ajarmouni-st commented 5 months ago

After further investigation, the reason why it cannot work on my side is because on prj.conf, I set CONFIG_PM=y

FYI It works with CONFIG_PM=y on v3.6.0-rc2.

krisnaresi commented 5 months ago

After digging down, I start to realize that RTC_ALARM is not yet implemented on rtc_ll_stm32.c.

RTC_ALARM is implemented in https://github.com/zephyrproject-rtos/zephyr/blob/ded0d48779e2dbf2f9b926b75ddcbb7c8a2b6268/drivers/counter/counter_ll_stm32_rtc.c

and it's not yet available in rtc_ll_stm32.c as it is a recent driver.

Hi @ajarmouni-st, thank you for pointing this source code. I already work on it, and I confirm that I can use it as an "epoch counter" to set and get time because those two functions are not present on counter API. I confirm that the alarm callback is working too, but unfortunately, the callback is not working when I use CONFIG_PM=y. I already update zephyr to v3.6.0-rc2, it still the same. There is no error message when calling counter_set_channel_alarm. I use this example as reference. My snippet of code is as below:

#include <stddef.h>
#include <zephyr/kernel.h>
#include <zephyr/sys/util.h>
#include <zephyr/types.h>

#include <zephyr/drivers/counter.h>
#include <zephyr/sys/timeutil.h>
#include <zephyr/device.h>
#include <time.h>

static const struct device *mDevice = DEVICE_DT_GET(DT_INST(0, st_stm32_rtc));
static struct counter_alarm_cfg mAlarm;

void callback(const struct device* dev, uint8_t chan, uint32_t ticks, void* data)
{
    printk("Hi from callback\n");
}

int main(void)
{
    printk("Start counter\n");

    if (!device_is_ready(mDevice)) {
        printk("Device not ready\n");
        return 0;
    }
    counter_start(mDevice);

    mAlarm.flags = 0;
    mAlarm.ticks = 3; // Callback should come at 3 seconds
    mAlarm.callback = callback;
    mAlarm.user_data = &mAlarm;

    int err = counter_set_channel_alarm(mDevice, 0, &mAlarm);
    if(err == 0) {
        printk("Set alarm OK\n");
    } else {
        printk("Failed to set alarm\n");
    }

    while (1) {
        k_sleep(K_SECONDS(5));
    }

    return 0;
}

Log:

*** Booting Zephyr OS build v3.6.0-rc2 ***
Start counter
Set alarm OK

Any comment or pointer is pretty much appreciated. Thank you.

ajarmouni-st commented 4 months ago

Hi @krisnaresi, I am unable to reproduce the issue on my side. In the following sample rtc_issue_68673 the callback is working with CONFIG_PM=y.

*** Booting Zephyr OS build v3.6.0-53-gb38b3b19cc34 ***
Start counter
Set alarm OK
Hi from callback
krisnaresi commented 4 months ago

Hi @ajarmouni-st, I already copy prj.conf and CMakeLists.txt as it is from the link above, and already update Zephyr to newest 3.6.0, unfortunately the problem is still persisted. Can I see the final .dts on <build_folder>/app/zephyr/zephyr.dts from your side? I'm kinda lost where to troubleshot it. Thank you for your support.

ajarmouni-st commented 4 months ago

Can I see the final .dts on <build_folder>/app/zephyr/zephyr.dts from your side?

@krisnaresi zephyr.dts

krisnaresi commented 4 months ago

Hi @ajarmouni-st, after further troubleshot, I found the reason why it cannot work on my board. On my <board>_defconfig, I put CONFIG_BT=y, that's why it overlooked on my prj.conf. After I set board defconfig to default and set the prj.conf to match yours, I can see the callback is working properly. But after I put CONFIG_BT=y, the callback is not working anymore. Can you try this on your side? Thank you for your support.

CONFIG_COUNTER=y
CONFIG_PRINTK=y
CONFIG_DEBUG=y
CONFIG_LOG=y
CONFIG_PM=y
CONFIG_BT=y
krisnaresi commented 4 months ago

Hi @ajarmouni-st, I look into this problem for several days, and found that CONFIG_BT=y is irrelevant. Using your prj.conf, I can only trigger callback one time. If you assign alarm after callback happen, it won't work anymore.

After do a lot of trial and error, I can resolve the problem by adding LL_PWR_EnableBkUpAccess to rtc_stm32_set_alarm function. I see that this function is to handle DBP bit on PWR control register 1:

image

From the sentence above, I assume that every time CPU is going idle and power management goes suspend, RTC is goes to reset state, thus write protection is activated. By set the DBP to disable write protection, we can once again register alarm callback. Here is the modified code for rtc_stm32_set_alarm function:

static int rtc_stm32_set_alarm(const struct device *dev, uint8_t chan_id,
                const struct counter_alarm_cfg *alarm_cfg)
{
#if !defined(COUNTER_NO_DATE)
    struct tm alarm_tm;
    time_t alarm_val_s;
#ifdef CONFIG_COUNTER_RTC_STM32_SUBSECONDS
    uint32_t alarm_val_ss;
#endif /* CONFIG_COUNTER_RTC_STM32_SUBSECONDS */
#else
    uint32_t remain;
#endif
    LL_RTC_AlarmTypeDef rtc_alarm;
    struct rtc_stm32_data *data = dev->data;

    /* Enable Backup access */
#if defined(PWR_CR_DBP) || defined(PWR_CR1_DBP) || \
    defined(PWR_DBPCR_DBP) || defined(PWR_DBPR_DBP)
    LL_PWR_EnableBkUpAccess();
#endif /* PWR_CR_DBP || PWR_CR1_DBP || PWR_DBPR_DBP */

    tick_t now = rtc_stm32_read(dev);
    tick_t ticks = alarm_cfg->ticks;

    if (data->callback != NULL) {
        LOG_DBG("Alarm busy\n");
        return -EBUSY;
    }

    data->callback = alarm_cfg->callback;
    data->user_data = alarm_cfg->user_data;

#if !defined(COUNTER_NO_DATE)
    if ((alarm_cfg->flags & COUNTER_ALARM_CFG_ABSOLUTE) == 0) {
        /* Add +1 in order to compensate the partially started tick.
         * Alarm will expire between requested ticks and ticks+1.
         * In case only 1 tick is requested, it will avoid
         * that tick+1 event occurs before alarm setting is finished.
         */
        ticks += now + 1;
        alarm_val_s = (time_t)(ticks / counter_get_frequency(dev)) + T_TIME_OFFSET;
    } else {
        alarm_val_s = (time_t)(ticks / counter_get_frequency(dev));
    }
#ifdef CONFIG_COUNTER_RTC_STM32_SUBSECONDS
    alarm_val_ss = ticks % counter_get_frequency(dev);
#endif /* CONFIG_COUNTER_RTC_STM32_SUBSECONDS */

#else
    if ((alarm_cfg->flags & COUNTER_ALARM_CFG_ABSOLUTE) == 0) {
        remain = ticks + now + 1;
    } else {
        remain = ticks;
    }

    /* In F1X, an interrupt occurs when the counter expires,
     * not when the counter matches, so set -1
     */
    remain--;
#endif

#if !defined(COUNTER_NO_DATE)
#ifndef CONFIG_COUNTER_RTC_STM32_SUBSECONDS
    LOG_DBG("Set Alarm: %d\n", ticks);
#else /* !CONFIG_COUNTER_RTC_STM32_SUBSECONDS */
    LOG_DBG("Set Alarm: %llu\n", ticks);
#endif /* CONFIG_COUNTER_RTC_STM32_SUBSECONDS */

    gmtime_r(&alarm_val_s, &alarm_tm);

    /* Apply ALARM_A */
    rtc_alarm.AlarmTime.TimeFormat = LL_RTC_TIME_FORMAT_AM_OR_24;
    rtc_alarm.AlarmTime.Hours = alarm_tm.tm_hour;
    rtc_alarm.AlarmTime.Minutes = alarm_tm.tm_min;
    rtc_alarm.AlarmTime.Seconds = alarm_tm.tm_sec;

    rtc_alarm.AlarmMask = LL_RTC_ALMA_MASK_NONE;
    rtc_alarm.AlarmDateWeekDaySel = LL_RTC_ALMA_DATEWEEKDAYSEL_DATE;
    rtc_alarm.AlarmDateWeekDay = alarm_tm.tm_mday;
#else
    rtc_alarm.AlarmTime.Hours = remain / 3600;
    remain -= rtc_alarm.AlarmTime.Hours * 3600;
    rtc_alarm.AlarmTime.Minutes = remain / 60;
    remain -= rtc_alarm.AlarmTime.Minutes * 60;
    rtc_alarm.AlarmTime.Seconds = remain;
#endif

    LL_RTC_DisableWriteProtection(RTC);
    ll_func_disable_alarm(RTC);
    LL_RTC_EnableWriteProtection(RTC);

    if (ll_func_init_alarm(RTC, LL_RTC_FORMAT_BIN, &rtc_alarm) != SUCCESS) {
        return -EIO;
    }

    LL_RTC_DisableWriteProtection(RTC);
#ifdef CONFIG_COUNTER_RTC_STM32_SUBSECONDS
    /* Care about all bits of the subsecond register */
    LL_RTC_ALMA_SetSubSecondMask(RTC, 0xF);
    LL_RTC_ALMA_SetSubSecond(RTC, RTC_SYNCPRE - alarm_val_ss);
#endif /* CONFIG_COUNTER_RTC_STM32_SUBSECONDS */
    ll_func_enable_alarm(RTC);
    ll_func_clear_alarm_flag(RTC);
    ll_func_enable_interrupt_alarm(RTC);
    LL_RTC_EnableWriteProtection(RTC);

#ifdef CONFIG_COUNTER_RTC_STM32_SUBSECONDS
    /* The reference manual says:
     * "Each change of the RTC_CR register is taken into account after
     * 1 to 2 RTCCLK clock cycles due to clock synchronization."
     * It means we need at least two cycles after programming the CR
     * register. It is confirmed experimentally.
     *
     * It should happen only if one tick alarm is requested and a tick
     * occurs while processing the function. Trigger the irq manually in
     * this case.
     */
    now = rtc_stm32_read(dev);
    if ((ticks - now < 2) || (now > ticks)) {
        data->irq_on_late = 1;
        rtc_stm32_set_int_pending();
    }
#endif /* CONFIG_COUNTER_RTC_STM32_SUBSECONDS */

    return 0;
}

Already test by enabling and disabling CONFIG_PM, it works properly. I'm not STM32 expert, so I'm open for any comment or discussion. Thanks.

ajarmouni-st commented 4 months ago

Hi @ajarmouni-st, I look into this problem for several days, and found that CONFIG_BT=y is irrelevant. Using your prj.conf, I can only trigger callback one time. If you assign alarm after callback happen, it won't work anymore.

After do a lot of trial and error, I can resolve the problem by adding LL_PWR_EnableBkUpAccess to rtc_stm32_set_alarm function. I see that this function is to handle DBP bit on PWR control register 1:

Hi @krisnaresi, thank you for investigating this & sharing the fix. Backup domain write access management is a well-known issue internally, & we already have an open issue to address this problem with a generic solution that applies not only to RTC but also to LSI, LSE, BBRAM...

ajarmouni-st commented 4 months ago

@krisnaresi In the meantime, does adding LL_PWR_EnableBkUpAccess produce the desired behavior in your use-case? &, if yes, are you willing to contribute a PR to patch this?

krisnaresi commented 4 months ago

Hi @krisnaresi, thank you for investigating this & sharing the fix. Backup domain write access management is a well-known issue internally, & we already have an open issue to address this problem with a generic solution that applies not only to RTC but also to LSI, LSE, BBRAM...

I see, hopefully ST team can find generic solution in near future.

@krisnaresi In the meantime, does adding LL_PWR_EnableBkUpAccess produce the desired behavior in your use-case? &, if yes, are you willing to contribute a PR to patch this?

In my case, yes, the RTC callback can work accordingly with or without CONFIG_PM for Nucleo WB55RG. Sure, let me try to create PR for this one, will let you know once I create one.

krisnaresi commented 4 months ago

Hi @ajarmouni-st, please find my PR on #70078. Any comment is welcome. Thanks.