Skip to content

Commit

Permalink
scripts: runners: nrf: Default to soft reset for the nRF52 series
Browse files Browse the repository at this point in the history
The Nordic nRF52 series have a peculiarity that is not shared with any
other Nordic families of SoCs: the reset pin can be reconfigured as a
regular GPIO. This has an unintended consequence: if the user has
reconfigured the pin in Devicetree to be a GPIO, `west flash` will
override that and configure the IC to use it as a reset pin, and the firmware
at boot won't be able to switch it back to GPIO, because that requires a UICR
erase. This behavior is very confusing to users, because the GPIO does
not work at all, since it is now just a reset line.

With this patch, `west flash` defaults to using soft reset instead of
pin reset for the nRF52 family of devices, to avoid overwriting the
reset pin configuration that the user includes in the image.

In order to be able to continue to use pin reset for users that so
desire it, a new option `--pinreset` is added that forces the use of pin
reset. The existing `--softreset` option is left exactly as it was, but
it is now applicable only to families other than the nRF52.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
  • Loading branch information
carlescufi committed Feb 1, 2025
1 parent 5e90d4f commit 35d20fa
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 21 deletions.
4 changes: 4 additions & 0 deletions doc/releases/migration-guide-4.1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ Boards
the required configuration by using ``mikroe_weather_click_i2c`` or ``mikroe_weather_click_spi``
instead of ``mikroe_weather_click``.

* All nRF52-based boards will now default to soft (system) reset
instead of pin reset when flashing with ``west flash``. If you want to keep
using pin reset on the nRF52 family of ICs you can use ``west flash --pinreset``.

Devicetree
**********

Expand Down
29 changes: 22 additions & 7 deletions scripts/west_commands/runners/nrf_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ def _get_suit_starter():
class NrfBinaryRunner(ZephyrBinaryRunner):
'''Runner front-end base class for nrf tools.'''

def __init__(self, cfg, family, softreset, dev_id, erase=False,
def __init__(self, cfg, family, softreset, pinreset, dev_id, erase=False,
reset=True, tool_opt=None, force=False, recover=False):
super().__init__(cfg)
self.hex_ = cfg.hex_file
# The old --nrf-family options takes upper-case family names
self.family = family.lower() if family else None
self.softreset = softreset
self.pinreset = pinreset
self.dev_id = dev_id
self.erase = bool(erase)
self.reset = bool(reset)
Expand Down Expand Up @@ -117,9 +118,14 @@ def do_add_parser(cls, parser):
'NRF54H', 'NRF91', 'NRF92'],
help='''MCU family; still accepted for
compatibility only''')
# Not using a muutal exclusive group for softreset and pinreset due to
# the way dump_runner_option_help() works in run_common.py
parser.add_argument('--softreset', required=False,
action='store_true',
help='use reset instead of pinreset')
help='use softreset instead of pinreset')
parser.add_argument('--pinreset', required=False,
action='store_true',
help='use pinreset instead of softreset')
parser.add_argument('--snr', required=False, dest='dev_id',
help='obsolete synonym for -i/--dev-id')
parser.add_argument('--force', required=False,
Expand Down Expand Up @@ -412,13 +418,18 @@ def program_hex(self):


def reset_target(self):
if self.family == 'nrf52' and not self.softreset:
# Default to soft reset on nRF52 only, because ICs in these series can
# reconfigure the reset pin as a regular GPIO
default = "RESET_SYSTEM" if self.family == 'nrf52' else "RESET_PIN"
kind = ("RESET_SYSTEM" if self.softreset else "RESET_PIN" if
self.pinreset else default)

if self.family == 'nrf52' and kind == "RESET_PIN":
# Write to the UICR enabling nRESET in the corresponding pin
self.exec_op('pinreset-enable')

if self.softreset:
self.exec_op('reset', kind="RESET_SYSTEM")
else:
self.exec_op('reset', kind="RESET_PIN")
self.logger.debug(f'Reset kind: {kind}')
self.exec_op('reset', kind=kind)

@abc.abstractmethod
def do_require(self):
Expand Down Expand Up @@ -488,6 +499,10 @@ def flush_ops(self, force=True):
def do_run(self, command, **kwargs):
self.do_require()

if self.softreset and self.pinreset:
raise RuntimeError('Options --softreset and --pinreset are mutually '
'exclusive.')

self.ensure_output('hex')
if IntelHex is None:
raise RuntimeError('Python dependency intelhex was missing; '
Expand Down
6 changes: 3 additions & 3 deletions scripts/west_commands/runners/nrfjprog.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class NrfJprogBinaryRunner(NrfBinaryRunner):
'''Runner front-end for nrfjprog.'''

def __init__(self, cfg, family, softreset, dev_id, erase=False,
def __init__(self, cfg, family, softreset, pinreset, dev_id, erase=False,
reset=True, tool_opt=None, force=False, recover=False,
qspi_ini=None):

super().__init__(cfg, family, softreset, dev_id, erase, reset,
super().__init__(cfg, family, softreset, pinreset, dev_id, erase, reset,
tool_opt, force, recover)

self.qspi_ini = qspi_ini
Expand All @@ -37,7 +37,7 @@ def tool_opt_help(cls) -> str:
@classmethod
def do_create(cls, cfg, args):
return NrfJprogBinaryRunner(cfg, args.nrf_family, args.softreset,
args.dev_id, erase=args.erase,
args.pinreset, args.dev_id, erase=args.erase,
reset=args.reset,
tool_opt=args.tool_opt, force=args.force,
recover=args.recover, qspi_ini=args.qspi_ini)
Expand Down
6 changes: 3 additions & 3 deletions scripts/west_commands/runners/nrfutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
class NrfUtilBinaryRunner(NrfBinaryRunner):
'''Runner front-end for nrfutil.'''

def __init__(self, cfg, family, softreset, dev_id, erase=False,
def __init__(self, cfg, family, softreset, pinreset, dev_id, erase=False,
reset=True, tool_opt=None, force=False, recover=False,
suit_starter=False):

super().__init__(cfg, family, softreset, dev_id, erase, reset,
super().__init__(cfg, family, softreset, pinreset, dev_id, erase, reset,
tool_opt, force, recover)

self.suit_starter = suit_starter
Expand All @@ -39,7 +39,7 @@ def tool_opt_help(cls) -> str:
@classmethod
def do_create(cls, cfg, args):
return NrfUtilBinaryRunner(cfg, args.nrf_family, args.softreset,
args.dev_id, erase=args.erase,
args.pinreset, args.dev_id, erase=args.erase,
reset=args.reset,
tool_opt=args.tool_opt, force=args.force,
recover=args.recover,
Expand Down
13 changes: 5 additions & 8 deletions scripts/west_commands/tests/test_nrf.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,22 +125,19 @@ class TC(typing.NamedTuple): # 'TestCase'
TC('nrf52', None, False, False, False, False, False):
((['nrfjprog', '--program', RC_KERNEL_HEX, '--sectoranduicrerase',
'--verify', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--pinresetenable', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--pinreset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
['nrfjprog', '--reset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
(TEST_DEF_SNR, None)),

TC('nrf52', None, False, False, False, True, False):
((['nrfjprog', '--program', RC_KERNEL_HEX, '--chiperase', '--verify', '-f', 'NRF52',
'--snr', TEST_DEF_SNR],
['nrfjprog', '--pinresetenable', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--pinreset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
['nrfjprog', '--reset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
(TEST_DEF_SNR, None)),

TC('nrf52', None, False, False, True, False, False):
((['nrfjprog', '--program', RC_KERNEL_HEX, '--sectoranduicrerase',
'--verify', '-f', 'NRF52', '--snr', TEST_OVR_SNR],
['nrfjprog', '--pinresetenable', '-f', 'NRF52', '--snr', TEST_OVR_SNR],
['nrfjprog', '--pinreset', '-f', 'NRF52', '--snr', TEST_OVR_SNR]),
['nrfjprog', '--reset', '-f', 'NRF52', '--snr', TEST_OVR_SNR]),
(TEST_OVR_SNR, None)),

TC('nrf52', None, False, True, False, False, False):
Expand All @@ -153,8 +150,7 @@ class TC(typing.NamedTuple): # 'TestCase'
((['nrfjprog', '--recover', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--program', RC_KERNEL_HEX, '--sectoranduicrerase',
'--verify', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--pinresetenable', '-f', 'NRF52', '--snr', TEST_DEF_SNR],
['nrfjprog', '--pinreset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
['nrfjprog', '--reset', '-f', 'NRF52', '--snr', TEST_DEF_SNR]),
(TEST_DEF_SNR, None)),

TC('nrf52', None, True, True, True, True, False):
Expand Down Expand Up @@ -421,6 +417,7 @@ def test_init(check_call, popen, get_snr, require, tool, test_case,
runner = cls(runner_config,
test_case.family,
test_case.softreset,
False,
snr,
erase=test_case.erase,
tool_opt=tool_opt,
Expand Down

0 comments on commit 35d20fa

Please sign in to comment.