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

Add 'erase_sectors' function to flash-algorithm #2132

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

9names
Copy link
Contributor

@9names 9names commented Feb 2, 2024

With SPI flash, erasing sector-by-sector is much slowing than erasing by block.
The CMSIS flash algo standard doesn't provide an interface for doing a block erase.
By augmenting the flash_sector function with a "number of sectors to erase" parameter, we allow the flash algo to use larger erase blocks.

This change results in slightly more than a 2x improvement in erase times for large binaries.

Open questions:

  • Should this take a start address and an end address instead, to help deal with regions with different sector sizes?
  • Should the function be called something other than flash_sectors to make it less easy to confuse with flash_sector

This draft impl assumes that all sectors sent to the erase command are of the same size and are contiguous - before merging I need to write something to collect all contiguous areas into batches to pass on to erase.
This draft also includes an update to the rp2040 flash algo to exercise this functionality

@bugadani
Copy link
Contributor

bugadani commented Feb 2, 2024

In the esp loaders we pretty much just ended up setting sector size to block size, and doing block erases in the flash algos, because I didn't want to add exactly this. However, if we're at it, I think I would prefer an erase(from, to) that would allow the flash algos to choose whichever is the best way to erase the required memory region.

@9names
Copy link
Contributor Author

9names commented Feb 9, 2024

would you expect the to address to be inclusive or exclusive?

@bugadani
Copy link
Contributor

bugadani commented Feb 9, 2024

I think exclusive is more logical, and easier to work with.

first.base_address.try_into().unwrap(),
qty.try_into().unwrap(),
)?;
active.erase_range(first.base_address, last.base_address + last.size - 1)?;
Copy link
Contributor

@bugadani bugadani Feb 10, 2024

Choose a reason for hiding this comment

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

How come you need that -1? If I'm not mixing up my intervals, the end of an exclusive range is just address + size 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My brain malfunctioning, basically 🤦.
I probably should have not done any programming today.

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.

None yet

2 participants