Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reset subcommand ignores manual reset_type option value #1042

Closed
nerdralph opened this issue Jan 9, 2021 · 5 comments
Closed

reset subcommand ignores manual reset_type option value #1042

nerdralph opened this issue Jan 9, 2021 · 5 comments
Labels

Comments

@nerdralph
Copy link

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
Copy link
Member

flit commented Jan 12, 2021

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?

@flit flit changed the title reset_type is always hw reset subcommand ignores manual reset_type option value Jan 12, 2021
@flit flit added the bug label Jan 12, 2021
@nerdralph
Copy link
Author

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
Copy link

pfalcon commented Mar 1, 2021

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
Copy link
Member

flit commented Mar 1, 2021

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

@flit
Copy link
Member

flit commented Jul 5, 2022

Closing; this should be fixed now.

@flit flit closed this as completed Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants