Pin reset handling in the sdk

[Guillaume76]

Hello,

I just wanted to share a problem I encountered and make some suggestions :

I lost some time looking for a TWI problem while it was actually related to the pin reset configuration. After flashing a test firmware which had the CONFIG_GPIO_AS_PINRESET flag defined, I flashed another code which hadn't defined the flag and was using the reset pin as SCL for TWI, so it didn't work.
Given the pin reset configuration is done by PSELRESET registers in the UICR section, it's mandatory to do an eraseuicr or an eraseall to deactivate the pin reset. That's not done by defaut in the sdk makefiles (sectoreraseonly).

Once explained it seems obvious but when you're debugging firmwares you don't need to add that trouble on top of the pile.

I suggest in the system_nrf52.c to check if the configuration is not correct with something like :

#if !defined (CONFIG_GPIO_AS_PINRESET)
    if (((NRF_UICR->PSELRESET[0] & UICR_PSELRESET_CONNECT_Msk) == (UICR_PSELRESET_CONNECT_Connected << UICR_PSELRESET_CONNECT_Pos)) ||
        ((NRF_UICR->PSELRESET[1] & UICR_PSELRESET_CONNECT_Msk) == (UICR_PSELRESET_CONNECT_Connected << UICR_PSELRESET_CONNECT_Pos))){
        NVIC_SystemReset();
    }
#endif

Meaning if we didn't enable the pin reset but it is actually enabled on the chip we do a soft reset. That's obvious enough so the user can understand it and erase the uicr manually to solve the situation. We could also directly erase the uicr using the NVMC before rebooting (but perhaps a bit intrusive).

Cheers
Guillaume

[Susheel Nuguru]

Hi Guillaume,

I understand your pain of debugging this. I also thank you for caring to come here to express your opinion.

You are suggesting that if the configuration does not match then we do a system reset. But this is very dangerous. This will make the device go into an endless reset loop and the users will have no clue of why this is happening until they spend time on debugging also. And as most of the compilers have this checkbox to run until main() when you start a debugger, in this case main() function is never called making the developer life more difficult to figure out the problem.

I thought about this for a while and I think the best is to make sure that you enable defines correctly. Does this makes sense? I am open to listen to your argument, if you think otherwise.

[Guillaume76]

Hi Aryan,

Thanks for your reply.

Yes the device will enter an endless loop but that was actually the point : if the device doesn't start at all look at your device configuration, including the pin reset, and try an erase all. That's kind of choosing the lesser evil : having a device that starts in a wrong configuration and will end up with not explicit errors or having a device that doesn't start.

You say to make sure to enable the defines correctly but as mentioned above that's not enough in the actual sdk : removing the CONFIG_GPIO_AS_PINRESET flag does not mean for sure that pin 21 won't act as a reset pin because it's handled in the PSELRESET registers in the UICR section that is not erased by default in the sdk before flashing. So if it pin reset was enabled before on the device, you need to remove the flag and do an erase UICR or erase all.

In my own code I check the pin reset config after the main so I can debug it properly but the idea was to try to include it in the sdk. We could also erase UICR registers by default in the sdk makefiles but that could lead to data loss for some users.

A last idea : in system_nrf52.c instead of rebooting if the config is wrong we could save the user uicr registers somewhere safe in ram, then do an uicr erase, rewrite user uicr data and finally do a soft reset to take into account the new configuration. This time reset will happen juste once because the configuration will be consistent at the next boot.

[Susheel Nuguru]

You last idea is like you say less evil but could be felt intrusive from other customers. 

Nevertheless, your points are valid so I have created a internal ticket for the developer to brainstorm this ideas and act accordingly. For your reference the internal ticket number is MDK-1622