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

Feature: mass erase target API #1619

Merged

Conversation

perigoso
Copy link
Contributor

@perigoso perigoso commented Sep 8, 2023

Detailed description

This PR is a refactor of the target mass erase API, it can be summed up in:

  • deeper integration with target flash API
    Target is placed in "flash mode" (enter/exit flash mode routine) before a mass erase, the flash API is used when a dedicated mass erase routine is not provided
  • all targets now support mass erase by default
    when a specialized mass erase routine is not provided the mass erase consists of erasing all the target flash' block by block via the target flash API
  • flash API routine added for mass erase of individual flash', used in place of block by block erase when provided and appropriate, useful for example on targets with multiple flash banks, though none were added as part of this PR

This is not yet thoroughly tested, and due to the lack of targets at my disposal, it likely won't be adequately tested by just me.

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

@perigoso perigoso force-pushed the feature/mass-erase-target-api branch from d0595a9 to 49b8138 Compare September 9, 2023 12:50
@perigoso perigoso force-pushed the feature/mass-erase-target-api branch from 49b8138 to a403c4b Compare October 3, 2023 15:29
@dragonmux dragonmux added this to the v2.0 release milestone Oct 3, 2023
@dragonmux dragonmux added Enhancement General project improvement User Testing Needed Looking for user testing reports labels Oct 3, 2023
@dragonmux
Copy link
Member

Apologies this PR got a bit forgotten about.. If you could please rebase it on main, we'll try to get it reviewed in the next week or two with the intention of getting it merged pronto.

@perigoso perigoso force-pushed the feature/mass-erase-target-api branch 3 times, most recently from 235fc7e to 971b340 Compare November 7, 2024 15:22
@perigoso
Copy link
Contributor Author

perigoso commented Nov 7, 2024

@dragonmux Managed to rebase this, sorry for the MIA...

There have been quite a lot of changes since this was first submitted, and though I got things to build and it appears most changes should not affect it I could not meaningfully test things, so here be dragons.

src/target/target_flash.c Outdated Show resolved Hide resolved
src/target/target_flash.c Outdated Show resolved Hide resolved
src/target/nrf51.c Outdated Show resolved Hide resolved
src/target/lpc546xx.c Outdated Show resolved Hide resolved
src/target/target_flash.c Outdated Show resolved Hide resolved
@dragonmux dragonmux mentioned this pull request Nov 11, 2024
6 tasks
target_flash_mass_erase abstracts the implementation details of the target mass erase
Move the scope of the print progress timeout creation to the common target API
target_enter_flash_mode_stub is a No-op stub for use in the
target->enter_flash_mode API where the default behaviour of
resetting the target is undesirable
…outine is available

Targets that don't provide a dedicated mass erase routine will
have it's flash erased block by block when the user requests
a mass erase
Mass erase now by default manually erases target flash block by block
when no specialized routine is available, the removed mass erase
routine duplicates said functionality
Mass erase now by default manually erases target flash block by block
when no specialized routine is available, this mass erase routine
duplicates said functionality
Mass erase now by default manually erases target flash block by block
when no specialized routine is available, this mass erase routine
duplicates said functionality
This routine, like the other flash routines is called in between a flash prepare and flash done,
with the FLASH_OPERATION_MASS_ERASE. It is meant to be used where the flash controller implements
functionality to totally erase the relevant flash, and nothing else, mainly targeted at targets
with multiple flash banks and funcionality to erase the banks wholly, individually
All targets that add a spi flash will have mass erase routines by default
without te need to explicitly provide them, this also is more correct for
scope and fixes the assumption that the first target flash registred is
a spi flash
Using the buffer allocation state as a means to determine the flash operation
is undefined and not guaranteed to work in the future, use proper API
When an erase is requested that would result in a complete erase of a flash
and the mass erase routine is provided, use it as a speed optimization
This is a very important restriction to ensure the method is safe to use during normal load operations
@perigoso perigoso force-pushed the feature/mass-erase-target-api branch from 9afb1f9 to 5627de9 Compare November 11, 2024 16:25
@perigoso perigoso force-pushed the feature/mass-erase-target-api branch from 7d1aadc to 3576cdf Compare November 11, 2024 17:02
Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

All LGTM, merging. Thank you for the contribution and this huge clean-up!

@dragonmux dragonmux removed the User Testing Needed Looking for user testing reports label Nov 11, 2024
@dragonmux dragonmux merged commit 3576cdf into blackmagic-debug:main Nov 11, 2024
36 checks passed
@perigoso perigoso deleted the feature/mass-erase-target-api branch November 11, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants