pyocd / pyOCD

Open source Python library for programming and debugging Arm Cortex-M microcontrollers
https://pyocd.io
Apache License 2.0
1.12k stars 481 forks source link

reset subcommand ignores manual reset_type option value #1042

Closed nerdralph closed 2 years ago

nerdralph commented 3 years ago

The docs say the default reset_type is sw, but 'pyocd reset -v' says hw:

0000660:INFO:board:Target type is cortex_m
0000670:INFO:__main__:Performing 'hw' reset...
0000870:INFO:__main__:Done.

Even 'pyocd reset -O reset_type=sw -v' still does a hw reset.

This is with pyocd 0.28.3 installed using pip.

flit commented 3 years ago

Thanks for the bug report.

Manually setting the reset_type option doesn't work due to the implementation of the reset subcommand. It sets the reset_type option based on its --method argument, so it always overrides reset_type set using other ways of setting session options.

The default for reset_type is actually sw, just not for pyocd reset. You can see this default with the other pyocd subcommands (and -Oreset_type=x will change it as appropriate).

The reason the default is different is due to the intended usage of pyocd reset, where a default of hardware reset means that the subcommand can skip the usual device interrogation. It just connects, toggles nRESET, and disconnects. Whereas for software reset, it needs to perform discovery to identify the CPU(s) that have the software reset register (M-profile AIRCR).

So, some of this behaviour is as intended, but isn't clearly documented. Aside from improved documentation, the main thing that could be changed or fixed is to check for and honour (or at least print a warning) for a manually-set reset_type session option on the pyocd reset command line. Does that seem reasonable?

nerdralph commented 3 years ago

I haven't used pyOCD (or even SWD for that matter) enough to make a cogent argument on how it should work, so I'll leave it to you.

pfalcon commented 3 years ago

I'm currently experimenting with reset_type option too, but with the flash subcommand. And with the 0.29.0 version, latest as of now. My initial impressions are just as @nerdralph's here - it seems to be ignored. And that feeling is only aggravated by the fact that I can pass completely invalid values to -O reset_type=, and completely invalid option names to -O at all: #1103.

Some other comments:

The default for reset_type is actually sw

That's what pyocd --help-options | grep reset_type tells me:

reset_type (str) Which type of reset to use by default ('default', 'hw', 'sw', 'sw_sysresetreq', 'sw_vectreset', 'sw_emulated'). The default is 'sw'.

But not what I see in practice with frdm_k64f board:

# pyocd flash -v -v -t k64f -f 3000000 zephyr.bin
0003632:DEBUG:cortex_m:reset, core 0, type=SW_SYSRESETREQ

You can see this default with the other pyocd subcommands (and -Oreset_type=x will change it as appropriate).

With pyocd flash, -Oreset_type=x doesn't seem to have any effect.

Aside from improved documentation, the main thing that could be changed or fixed is to check for and honour (or at least print a warning) for a manually-set reset_type session option on the pyocd reset command line. Does that seem reasonable?

IMHO yes, there's should be reaction from the app when user sets an option, but the app ignores it (and not ignoring, but do as user asks for is even better).

Thanks.

flit commented 3 years ago

@pfalcon Thanks for the feedback, and thanks for creating #1103!

flit commented 2 years ago

Closing; this should be fixed now.