No need for the indent

We probably want a

   if (!IS_USED(MODULE_PERIPH_BACKUP_RAM)) {
        return;
    }

here so we don't increase standby current if backup ram is not used

Couldn't that function still be useful when backup ram is not used?
We could move the implementation to cpu_common.c instead of backup_ram.c

I skimmed over the cmsis-header-stm32 repository.
I guess a function for all stm32 would be more complicated. Some don´t have RCC_CSR_PORRSTF. Some don´t have RCC_CSR_BORRSTF. And stm32h7 doesn´t have reset flags at all.

I would rename it to backup_ram_is_retained() because the use case of this function is to check exactly that.

or backup_ram_was_retained() because it is called after reset

Maybe that function name was a bad idea since originally it should just return if we did a cold boot or woke from Deep Sleep, but you are right that for this use case we also should know if the backup battery was connected.
But shouldn't this then return false if the backup battery is not connected()?

Then again if no backup battery exists we can't use the backup RAM and there is little the software can do about it, so we can just ignore it. Or are there cases where backup RAM retention works without backup battery?

The battery is used to retain backup ram when there is no power supply, E.g. someone removed the USB cable. If there is no battery and the board entered standby mode while the power supply was not interrupted, the backup ram is retained even without battery.

Or maybe rewrite like this?:

    if (backup_ram_regulator_is_on()) {
        if (_backup_battery_connected()) {
            /* if regulator is on and a battery is connected after reset,
            backup ram should have been retained */
            return true;
        }
        uint32_t coldboot = (RCC->CSR & (RCC_CSR_PORRSTF | RCC_CSR_BORRSTF));
        RCC->CSR |= RCC_CSR_RMVF; /* clear reset flags */
        return !(coldboot == (RCC_CSR_PORRSTF | RCC_CSR_BORRSTF));
    }
    return false;

So backup_ram_regulator_is_on() is only true if we did not cold boot? Than that version looks more correct to me!
But then we never clear reset flags if the battery is connected? Why is this early return needed?

So backup_ram_regulator_is_on() is only true if we did not cold boot?

If we enter the first if and skip the second if, coldboot can still be true or false, so no.
What I did not mention so far is a note in the manual regarding the BRE and BRR bits (the regulator).

Note: This bit is not reset when the device wakes up from Standby mode, by a system reset,
or by a power reset. [RM0410 p. 146]

I try to formulate my thoughts more verbose.
This function answers the question: Does the backup ram contain valid data (true) or can it be overridden.
We are at the point where the function is called, the reset handler, so right after reset.
A necessary condition for the backup ram to be valid after reset is that the regulator must have been on.
So if that is not true, it returns false.
If the regulator was on and we have a battery, then backup ram survived even cold boots, so that is not tested further and it returns true.
Else backup ram survived only soft resets where the power supply was not interrupted.

By the time of writing I notice two things.

1. backup_ram_init() possibly (re)enables the regulator, thus checking whether the regulator was on is meaningless. 😯
2. No other function after that one will be able to check the reset flags because they are cleared immediately. A good place to clear reset flags would be at the and of reset_handler_default(), I guess.
   But this is architecture dependent. Could this be done in an stm32::post_startup(), or a new reset_done() at the very end?

void backup_ram_init(void)
{
    /* see reference manual 4.1.5 "Battery backup domain" */
    periph_clk_en(APB1, RCC_APB1ENR_PWREN);
    stmclk_dbp_unlock();
    periph_clk_en(AHB1, RCC_AHB1ENR_BKPSRAMEN);
#if defined(VBAT_ADC) && IS_USED(MODULE_PERIPH_ADC)
    adc_init(VBAT_ADC);
#endif
}

bool cpu_woke_from_backup(void) {
    bool regon = backup_ram_regulator_is_on();
    if (_backup_battery_connected()) {
        if (regon) {
            /* if regulator is on and a battery is connected after reset,
            backup ram should have been retained */
            return true;
        }
        /* in case the board has a backup battery the regulator must be on
        to mitigate (unexpected) outage of VDD, so RTC register and
        backup domain register contents are not lost */
        backup_ram_regulator_on();
        return false;
    }
    uint32_t coldboot = (RCC->CSR & (RCC_CSR_PORRSTF | RCC_CSR_BORRSTF));
    RCC->CSR |= RCC_CSR_RMVF; /* clear reset flags somewhere else */
    return regon && !(coldboot == (RCC_CSR_PORRSTF | RCC_CSR_BORRSTF));
}

I am sorry for the confusion. I think now the battery + regulator thing is clear. Of course I am open for change requests as long as it works out.

those code snippets above don´t work 😔.
But the current code works as before.

How about we put a magic string in backup RAM to check if the memory was retained? To me that appears like a simpler solution than pulling in the ADC peripheral for that. (measuring Vbat is still valuable, but I'd prefer to have that as a separate PR and independent of backup RAM)

lpc23xx had a similar issue.

works good

It was working too well. I wondered why the backup ram was retained when battery was on but the regulator was off. Then I figured out that backup_ram_regulator_off() was never called here .
So a better place to switch off the regulator would be in cpu_woke_from_backup().

You can rebase and add stm32u5% to the list, then apply this patch:

diff --git a/cpu/stm32/include/cpu_conf.h b/cpu/stm32/include/cpu_conf.h
index 8930b13cea..e89fd40a45 100644
--- a/cpu/stm32/include/cpu_conf.h
+++ b/cpu/stm32/include/cpu_conf.h
@@ -67,7 +67,7 @@
 #elif CPU_FAM_STM32U5
 #include "stm32u5xx.h"
 #include "irqs/u5/irqs.h"
-#define NUM_HEAPS   2
+#define NUM_HEAPS   3
 #elif CPU_FAM_STM32WB
 #include "stm32wbxx.h"
 #include "irqs/wb/irqs.h"
diff --git a/cpu/stm32/ldscripts/stm32.ld b/cpu/stm32/ldscripts/stm32.ld
index 1ed6fa43d5..f216ac3ff6 100644
--- a/cpu/stm32/ldscripts/stm32.ld
+++ b/cpu/stm32/ldscripts/stm32.ld
@@ -31,8 +31,8 @@ SECTIONS
 {
     .heap2 ALIGN(4) (NOLOAD) :
     {
-        _sheap1 = . ;
-        _eheap1 = ORIGIN(sram4) + LENGTH(sram4);
+        _sheap2 = . ;
+        _eheap2 = ORIGIN(sram4) + LENGTH(sram4);
     } > sram4
 }

Why do we need APB1 here?

Looks like this is on another bus on STM32U5

The st32u5 seems to be a little different then. I must take a closer look on that one. For the others APB1 is also required.

My source of information which MCUs support support backup RAM was cmsis-header-stm32. I just pulled and noted that stm32h745xg, stm32h747xg, stm32u575xx, and stm32u585xx have been added.

All right, I am not going to make it a periph_ - module in this PR.

Is it correct to use the "Non-secure boundary address" here?

@aabadie could give it a try

I think the stm32u5 lacks periph_rtc support yet. I got compiling errors when I tried to compile the backup RAM test for the board with that CPU. That was the reason why I added FEATURES_REQUIRED += periph_rtc to the test.
At least the hello world example with a variable in backup RAM did compile.

Is this supposed to be unavailable in riotboot.elf? However, BOARD=nucleo-f767zi make -C tests/riotboot flash test prints "TEST PASSED".

This is only about saving a bit of energy right?
Given that riotboot only runs for a very brief moment and we want to keep it as small as possible, I'd say it's the right thing to do to leave it on here.

Of course we should not end up in a situation where using riotboot causes us to lose the contents of the backup RAM.

Yes turning it off is just to save some power.
When I do FEATURES_REQUIRED+=riotboot BOARD=nucleo-f446re make -c tests/periph_backup_ram flash term, the counter in backup RAM is not reset, so I´d say it´s fine.