Bug in how bootloader treats GPREGRET

I suspect this is related to https://devzone.nordicsemi.com/f/nordic-q-a/36601/gpregret-and-nrf_bootloader-c-evolution-proposal (Case ID: 211047)

In nrf_bootloader_info.h you find this:

#define BOOTLOADER_DFU_GPREGRET_MASK            (0xB0)      /**< Magic pattern written to GPREGRET register to signal between main app and DFU. The 3 lower bits are assumed to be used for signalling purposes.*/
#define BOOTLOADER_DFU_START_BIT_MASK (0x01) /**< Bit mask to signal from main application to enter DFU mode using a buttonless service. */

As I understand the comment, B0 is used as a kind of signature to say "yes, the register content is related to bootloader", and reading the 3 lower bits are used to "signal", bit 0 (lowest bit) is used to signal DFU should start.

I wanted to create my own signal, for which I use bit 1. So at some point, I set it, which gives the register a value of B2.

Now let's have a look at code in dfu_enter_flags_clears:

if (NRF_BL_DFU_ENTER_METHOD_GPREGRET &&
(nrf_power_gpregret_get() & BOOTLOADER_DFU_START))
{
// Clear DFU mark in GPREGRET register.
nrf_power_gpregret_set(nrf_power_gpregret_get() & ~BOOTLOADER_DFU_START);
}

This code will be executed as B2 & B1 != 0... Shouldn't it be replaced with:

if (NRF_BL_DFU_ENTER_METHOD_GPREGRET &&
((nrf_power_gpregret_get() & 0xF8 == BOOTLOADER_DFU_GPREGRET_MASK) && (nrf_power_gpregret_get() & BOOTLOADER_DFU_START_BIT_MASK)))
{
// Clear DFU mark in GPREGRET register.
nrf_power_gpregret_set(nrf_power_gpregret_get() & ~BOOTLOADER_DFU_START);
}

Now let's have a look at this code from dfu_enter_check:

if (NRF_BL_DFU_ENTER_METHOD_GPREGRET &&
(nrf_power_gpregret_get() & BOOTLOADER_DFU_START))
{
NRF_LOG_DEBUG("DFU mode requested via GPREGRET.");
return true;
}

When GPRGRET == 0xB2, this code will get executed, which, as far as I understand, contradicts the documentation and the way it was intended to work (as BOOTLOADER_DFU_START_BIT_MASK is here unimportant, replacing BOOTLOADER_DFU_START with BOOTLOADER_DFU_GPREGRET_MASK would do the same job, while clearly ignoring the "signal flags", and then what's the point of having signal flags?)

So I guess I'm expecting 2 things:
- Confirmation that I understood properly the way "signal bits" were intended to be used.
- That case ID 211047 would not be regarded as a possible evolution, but rather as a bugfix, as the bootloader does not behave according to the way "described" by the documentation.

2.5.0.0
Parents Reply Children
No Data