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

Added swd_uninit_debug() function. Used in nRF5x reset routine and flash uninit #175

Merged
merged 1 commit into from
Nov 18, 2016

Conversation

gamnes
Copy link
Contributor

@gamnes gamnes commented Oct 28, 2016

No description provided.

Copy link
Contributor

@c1728p9 c1728p9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. This will be good to have in the codebase.

__attribute__((weak)) void swd_set_target_reset(uint8_t asserted)
{
(asserted) ? PIN_nRESET_OUT(0) : PIN_nRESET_OUT(1);
}

// This function is currently never called, see swd_set_target_state_sw() instead
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in use by some targets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will remove comment

}
else {
else {
swd_init_debug();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were there problems without swd_init_debug()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but since we do uninit_debug() in the asserted statement above, we have to do init debug again here? Or can we always assume swd_set_target_reset() will be called with asserted == False after it has been called with asserted == True? Then i wouldnt have to call uninit_debug() in the asserted statement above, and then i wouldnt have to call init_debug in the NOTasserted statement here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that sequence can be guaranteed. What you have here is good.

@c1728p9
Copy link
Contributor

c1728p9 commented Oct 29, 2016

@mbed-bot: TEST

@gamnes
Copy link
Contributor Author

gamnes commented Oct 31, 2016

Do you have the log of the failing mbed-bot testing?

@c1728p9
Copy link
Contributor

c1728p9 commented Oct 31, 2016

It doesn't look like this was a real failure. I'll re-trigger.

@c1728p9
Copy link
Contributor

c1728p9 commented Nov 1, 2016

@mbed-bot: TEST

1 similar comment
@c1728p9
Copy link
Contributor

c1728p9 commented Nov 11, 2016

@mbed-bot: TEST

// Enable debug again because TARGET RESET might have disabled it
if (!swd_write_word(DBG_HCSR, DBGKEY | C_DEBUGEN)) {
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is causing tests to fail. The sequence here is to turn on the reset vector catch and then perform a hardware reset so the device is halted right at boot. By writing to the DHCSR here the C_HALT bit gets written to 0 causing the processor to start running. Is this change required for the NRF51 or NRF52 to work correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, since its never used by the nRF51 nor the nRF52, this line is not necessary. It was added because the set_target_reset(0) might have disabled the debug connection. I can remove it.

… for nRF5x, as well as target_flash.c uninit.
@c1728p9
Copy link
Contributor

c1728p9 commented Nov 18, 2016

@mbed-bot: TEST

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants