Bug in hal_device_reset()

[JonasJ]

Hi,

It seems to be a bug in hal_device_reset() when using S132.

void hal_device_reset(uint8_t gpregret_value)
{
#if defined(SOFTDEVICE_PRESENT)
    (void) sd_power_reset_reason_clr(RESET_REASON_MASK); /* avoid wrongful state-readout on reboot */
#if defined(S130) || defined(S110)
    (void) sd_power_gpregret_set(gpregret_value);
#elif defined(S132)
    (void) sd_power_gpregret_set(gpregret_value, RESET_REASON_MASK);
#endif
    (void) sd_nvic_SystemReset();
#else
    NRF_POWER->RESETREAS = RESET_REASON_MASK; /* avoid wrongful state-readout on reboot */
    NRF_POWER->GPREGRET = gpregret_value;
    NVIC_SystemReset();
#endif
    NRF_MESH_ASSERT(false);
}

On line 8 above the function sd_power_gpregret_set() is called with the wrong arguments. The first arguments should be either 0 or 1 to select between GPREGRET and GPREGRET2, the second argument should be the bits to set.

So instead of setting GPREGRET to gpregret_value, either one of the two registers or none at all is set to RESET_REASON_MASK (0xFFFFFFFF).

This bug seem to exist in v3.0.0 back to v1.0.0.

Changing line 8 to (void) sd_power_gpregret_set(0, gpregret_value); solves the problem and the correct value is written to the register.

Thanks.

[JonasJ]

Hmm, not sure that I follow you on this one.

If I look at the function above it should do the following.

    NRF_POWER->GPREGRET = gpregret_value;

This is inline with the documentation in the header file (gpregret_value Value to set the retention register to).

However, when using S132 the following code is executed instead

    (void) sd_power_gpregret_set(gpregret_value, RESET_REASON_MASK);

Note that the first parameter to this function specifies which register, i.e GPREGRET or GPREGRET2 and the second specifies what bits to set. Simply the fact that RESET_REASON_MASK is used as an argument hints that something is wrong.

If I run without a softdevice GPREGRET is set to gpregret_value.

if I run with S130/S110 GPREGRET will have the bits of gpregret_value set, which is different from above, i.e assignment vs OR.

If I run with S132 three things can happen.

• GPREGRET is set to 0xFFFFFFFF (or rather 0xFF since it's 8-bit) if gpregret_value is 0,
• GPREGRET2 is set to 0xFFFFFFFF (or rather 0xFF since it's 8-bit) if gpregret_value is 1
• sd_power_gpregret_set() probably fails and returns an error if gpregret_value > 1

The header file suggest that the direct set of GPREGRET is the correct one. To get the same behavior when using a softdevice sd_power_gpregret_clr() should probably be called as well.

/**
 * Clear the reset reason register, set the retention register, and reset the
 * device. All volatile memory will be lost. This function will never return.
 *
 * @param[in] gpregret_value Value to set the retention register to.
 */

Thanks.

[AndreasF]

Hi again.

I misunderstood a bit when I talked to the developer, he had made the changes you proposed to the master branch, and it will be available in the next release.

Sorry for the confusion, have a nice Christmas.

Best regards,

Andreas

[JonasJ]

Thanks.

I guess the differences between NRF_POWER->GPREGRET = gpregret_value; and sd_power_gpregret_set(0, gpregret_value); is not something you would like to fix?