Skip to content

Cortex-M target reset unify#371

Closed
cederom wants to merge 1 commit intoARMmbed:masterfrom
cederom:cederom_reset_unify
Closed

Cortex-M target reset unify#371
cederom wants to merge 1 commit intoARMmbed:masterfrom
cederom:cederom_reset_unify

Conversation

@cederom
Copy link
Contributor

@cederom cederom commented Dec 14, 2017

This PR brings the unification to target reset routines trying the software reset first and then hardware reset if the first fails.

Both swd_set_target_state_hw() and swd_set_target_state_sw() are replaced with target_set_state() which also removes set state from target implementations unless reset really requires some unique reset quirk. New functions will be placed in dedicated target.{c,h} file also separate from swd_host.{c,h}.

Still testing, please merge only after label is removed and code is confirmed after in depth testing..

@cederom
Copy link
Contributor Author

cederom commented Dec 14, 2017

/morph test

@cederom
Copy link
Contributor Author

cederom commented Dec 14, 2017

Boards test status:

Vendor HIC Target Status Previous Reset Implementation and Remarks
Freescale K20DX FRDM-K64F 💚 OK HARDWARE + BEFORE_INIT + QUIRK + TRICKY (UN)LOCK + SECURITY_BITS
Onsemi SAM3U2C NCS36510-RFv1.0 💚 OK SOFTWARE RESET
Nordic SAM3U2C nRF51, nRF52 💚 OK SOFTWARE + SWD_SET_TARGET_RESET() QUIRK MIXING HW GPIO AND SW RESET
Microbit KL26 nRF51822 💚 OK NO HARDWARE LINES?
LPC LPC4322 LPC54114 💚 OK
NXP 11U35 * 💚 OK HARDWARE RESET
NXP 11U35 LPC4088 💚 OK QUIRK
Renesas 11U35 R7S721001 💚 OK HARDWARE, GR-PEACH, CORTEX-A, SW RST DOES NOT WORK!
SiliconLabs * * 💛 TODO SOFTWARE
STM * * 🔴 DAPLink on STM HARDWARE
STM NZ32SC151 * 🔴 DAPLink on STM HARDWARE
STM xDOT-L151 * 🔴 DAPLink on STM HARDWARE
Toshiba * * 🔴 HARDWARE UNAVAILABLE SOFTWATE + HARDWARE_PRE_INIT
Wiznet * * 🔴 HARDWARE UNAVAILABLE QUIRK

@mbed-bot
Copy link
Collaborator

Build finished.

@cederom cederom force-pushed the cederom_reset_unify branch 2 times, most recently from 95cf63a to 2fd637d Compare December 14, 2017 23:39
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 14, 2017
Details and progress at ARMmbed#371

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom cederom force-pushed the cederom_reset_unify branch from 2fd637d to d493edd Compare December 14, 2017 23:49
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 14, 2017
Details and progress at ARMmbed#371

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>

/**
* Unlock Sequence clears out the Security Bits and Flash Memory.
* Hardware Reset line MUST be asserted due implementation issues.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here are the details to add in the place of my todo:

During the unlock sequence the hardware reset must be asserted to ensure device code cannot execute and interfere with the unlocking procedure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, thanks! :-)


case RESET_PROGRAM:
// Initialize SWD DAP Port so we have a known state of the Target.
// DAP is essential for both softwaRESET_HOLDre/hardware halt-on-reset.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a typo - softwaRESET_HOLDre

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks! :-)

@cederom cederom force-pushed the cederom_reset_unify branch from d493edd to ea96943 Compare December 16, 2017 00:41
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 16, 2017
Details and progress at ARMmbed#371

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom
Copy link
Contributor Author

cederom commented Dec 16, 2017

Need to rewrite / separate nRF51822 code that de-asserts the nRST line because it also disables DAP which breaks RESET_PROGRAM (it has de-assert in the middle).. except that existing (de)asserting can be used in RESET_HOLD and RESET_RUN.. introduced swd_debug_deinit() for that :-)

@cederom cederom force-pushed the cederom_reset_unify branch from ea96943 to 48b75e2 Compare December 18, 2017 23:58
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 18, 2017
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom
Copy link
Contributor Author

cederom commented Dec 19, 2017

Redesigned target_set_state():

  • RESET_HOLD and RESET_PROGRAM now works the same way and is the same code (init debug, reset, halt on reset using vector catch).
  • RESET_RUN first reset and halt just as above then reset again to run the code.

Remarks for NORDIC implementation and changes:

  • Unified target_set_state() now seems to work without specific swd_set_target_reset() implementation that was quirky and was causing problems.
  • NORDIC target probably still needs a quirk to disable debug logic.

swd_set_target_reset() is not __attribute__((weak)) anymore. This way only hardware reset is handled by those calls if implemented.

@cederom cederom force-pushed the cederom_reset_unify branch from 48b75e2 to 011dcb5 Compare December 19, 2017 00:10
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 19, 2017
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom
Copy link
Contributor Author

cederom commented Dec 19, 2017

New reset code seems to work on nRF51822, nRF52832 (flashing, cdc break). Even on nRF52840 except flash algo cannot yet erase the memory but the cdc break worked..
Introduced changes did not break K64F and NCS36510 :-)

@cederom
Copy link
Contributor Author

cederom commented Dec 19, 2017

/morph test

@mbed-bot
Copy link
Collaborator

Build finished.

@cederom
Copy link
Contributor Author

cederom commented Dec 19, 2017

Microbit KL26 / nRF51822 works fine :-)

@cederom
Copy link
Contributor Author

cederom commented Dec 19, 2017

LPCXpressso5411x HIC_LPC4322 / TGT_LPC54114 works fine :-)

@cederom
Copy link
Contributor Author

cederom commented Dec 20, 2017

Embedded Artists 11U35_HIC / LPC4088_TGT works fine :-)

TODO: Rewrite ISP GPIO toggling wrapped around reset calls..

@cederom cederom force-pushed the cederom_reset_unify branch from 011dcb5 to 66de826 Compare December 20, 2017 21:34
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 20, 2017
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom cederom force-pushed the cederom_reset_unify branch from 66de826 to c78bf50 Compare December 20, 2017 21:50
cederom added a commit to cederom/DAPLink that referenced this pull request Dec 20, 2017
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@Derrick45
Copy link

LPC11U35 plus nrf51822 can't reset automatically using msd,why?

@cederom
Copy link
Contributor Author

cederom commented Jan 15, 2018

Hey @wzt19910506 this thread is about DAPLink RESET REWORK ONLY not the LPC+nRF Drag-n-Drop as stated in #375, please talk about LPC+nRF over there.

However, you can try new RESET code from this Pull Request on your LPC HIC with external nRF Target and will should work even with no nRST line attached as we first try the software reset by default!

Note: without custom HIC+Target firmware you only get HID Debug and CDC UART working, both can reset your target, so you can verify the new code.

@cederom cederom force-pushed the cederom_reset_unify branch from c78bf50 to fe222a5 Compare January 18, 2018 12:25
cederom added a commit to cederom/DAPLink that referenced this pull request Jan 18, 2018
WARNING: BREAKS NCS FLASHING!

Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.
* Introducing pre/post reset flash/run quirks.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom
Copy link
Contributor Author

cederom commented Jan 18, 2018

4.0 adds pre/post reset run/flash quirks as required by LPC4088DM board.

Some problems with NCS flashing are observed but not caused by last change.. BIN files flashing works fine, HEX flashing does not work over UMS. Is that a known issue?

@cederom
Copy link
Contributor Author

cederom commented Jan 18, 2018

/morph test

@mbed-bot
Copy link
Collaborator

Build finished.

@cederom
Copy link
Contributor Author

cederom commented Jan 18, 2018

  • Current (copy-paste) updated reset code for Cortex-M does not work on R7S Renesas GR-PEACH as this is the Cortex-A Target.

  • Cortex-M has its SWD implementation in swd_host.c while Cortex-A has its implementation in swd_host_ca.c. They both share swd_host.h but they are complete separate implementations compiled into the firmware based on TARGET_MCU_CORTEX_A define and source files.

  • On the GR-PEACH board, the old Cortex-A swd_set_target_state_hw(state) works fine, while swd_set_target_state_sw(state) does not. Therefore Cortex-A version of target_set_state() simply wraps the swd_set_target_state_hw(state).

  • Reset unify is be done for Cortex-M only. If that works fine, the target_ code could be taken out to the separate source file and use underlying (alternate) Cortex-M or Cortex-A SWD Host routines. Cortex-A could be rewritten then in a similar manner.

@cederom cederom force-pushed the cederom_reset_unify branch from fe222a5 to 8be221d Compare January 18, 2018 20:55
cederom added a commit to cederom/DAPLink that referenced this pull request Jan 18, 2018
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.
* Introducing pre/post reset flash/run quirks.
* Leaves Cortex-A rework for Target code separation.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom cederom force-pushed the cederom_reset_unify branch from 8be221d to acb0b7c Compare January 18, 2018 20:59
cederom added a commit to cederom/DAPLink that referenced this pull request Jan 18, 2018
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state)
* Removed return swd_set_target_state_sw(state)
* Removed return swd_set_target_state_hw(state)
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.
* Introducing pre/post reset flash/run quirks.
* Leaves Cortex-A rework for Target code separation.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom cederom changed the title target reset unify - DO NOT YET MERGE - Cortex-M target reset unify - DO NOT YET MERGE - Jan 19, 2018
Details and progress at ARMmbed#371

Summary of changes:
* Introducing global unified target_set_state(TARGET_RESET_STATE state).
* Removed swd_set_target_state_sw(state).
* Removed swd_set_target_state_hw(state).
* Function swd_set_target_reset(uint8_t asserted) is not weak anymore.
* Updated some pre/post reset routine quirks.
* Introducing pre/post reset flash/run quirks.
* Leaves Cortex-A rework for Target code separation.

Signed-off-by: CeDeROM Tomasz CEDRO <tomek@cedro.info>
@cederom cederom force-pushed the cederom_reset_unify branch from acb0b7c to bf51af5 Compare January 22, 2018 00:08
@cederom
Copy link
Contributor Author

cederom commented Jan 22, 2018

@c1728p9 could you please take a look in a free moment if this is okay? :-)

  • Do we want to test the STM targets? If so how can I replace ST-LINK with DAPLink?
  • Cortex-A seems to have separate implementation of SWD Host so reset unify will have to be done similar way there too, preferably moving out the Target code from swd_host.c to target.c and using provided underlying Cortex-A/M swd_ functions..
  • I have also utilised the DEBUG and NO_DEBUG states using existing code + switch() syntax trick.

@cederom cederom changed the title Cortex-M target reset unify - DO NOT YET MERGE - Cortex-M target reset unify Feb 9, 2018
@40Grit
Copy link

40Grit commented May 22, 2019

What is status of this?

@0xc0170
Copy link
Contributor

0xc0170 commented May 22, 2019

@40Grit What's your interest in this PR? I believe it's stalled and might be just closed now. If there's anything that should be picked up again, let us know.

@0Grit
Copy link

0Grit commented May 22, 2019

@0xc0170
Wondering:

  • Has it been superseded?
  • Is it still relevant?
  • What was its benefit?

@cederom
Copy link
Contributor Author

cederom commented May 22, 2019

Hey @loverdeg-ep / @40Grit :-) Developers had different idea for reset mechanism and machine state design for DAPLink, and the whole architecture, if any, in general, so probably this is kinda dead end sorry. The master went ahead. You can make use of it as you like :-)

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.

7 participants