drivers: flash: sam: fix flash erase last page #80800
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bjarki-andreasen
added
bug
zephyrbot
added
area: Flash
platform: Microchip SAM
Contributor
Author
|
|
zephyrbot
requested review from
attie-argentum,
de-nordic,
mnkp,
nandojve,
pdgendt,
rghaddab and
stephanosio
de-nordic
previously approved these changes
Dec 16, 2024
Member
|
Hardware tests on sam_v71_xult/samv71q21. |
Member
|
The current tests with the samv71 partition cover the last page? If yes, I'm ready to approve. |
Contributor
Author
|
To erase the last page for that board, change this line toDT_NODELABEL(scratch_partition) and run the suite again :) we are looking for:
the board has a scratch partition at the end of the flash area :) |
Member
I can do that and provide feedback, yes. But in this case, it will be nice if we extend the test to run over these two partitions. I want to avoid regressions here. I'm fine if you want to do on another PR too to not switch your tasks at moment. |
Member
|
Running test using the scratch_partition |
Contributor
Author
|
well that sucks :) |
Member
|
Hi @bjarki-andreasen , I was wondering if you had time to look on it again. |
Member
|
friendly ping : ) |
Contributor
Author
|
I'm getting a dev kit in the coming days, then I will fix it properly |
Contributor
Author
|
I extended the test suite to include board overlays so we can adjust the slot1 partition to extend to the very end of the flash area. With this, and running the test suite both with and without MCU boot on an sam4e_xpro, it runs without error, erasing the last page successfully: I did not need to change the driver, from what I can tell the bug was indeed fixed :) |
bjarki-andreasen
removed
the
block: HW Test
Member
|
Hi @bjarki-andreasen , sam4e_xpro, as you mentioned passes sam_v71_xult, as I mentioned, do not passes if you don't mind to include the below patch, it helps to test samv71 +tests/drivers/flash/erase_blocks/boards/sam_v71_xult_samv71q21.overlay
+/*
+ * Copyright (c) 2025 Gerson Fernando Budke <[email protected]>
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+&flash0 {
+ partitions {
+ /delete-node/ storage_partition;
+
+ slot1_partition: partition@100000 {
+ label = "image-1";
+ reg = <0x00100000 0x100000>;
+ };
+ };
+}; |
Contributor
Author
I will add the overlay, i can't test it however given I don't have that hardware, could it be a dead flash page or something on your board, or is the flash layout maybe different from the sam4? Don't get why its failing... |
Member
|
I'm checking is there is something different on the flash controller that could fail on the test_erase_single_pages_in_part. If I can not find anything that explains we can assume it could be a page problem as you mention : ) |
Member
|
Hi @bjarki-andreasen , I was testing and enabled debug on the driver. I noted this: 1- Given the number is DECon the tests it is harder to detected that something goes wrong. If I understood correctly, the below should erase all the 1MiB but it is not. I saw that this is OK for the test_write_across_page_boundaries_in_part but still not evaluate the last sector! 0x1fa000 + 0x2000 => 1fc000 2- Look the debug here. The device do not even call the Erasing function on the driver. 2072576 is 1fa000. So, I assume there are something wrong in some place and that could be the issue when it tries to write, is this correct? 3- SAM4E erase 4k and SAMV71 erase 8k. 4- There are some restrictions on the erase page size. |
nandojve
requested changes
Jan 28, 2025
tests/drivers/flash/erase_blocks/boards/sam4e_xpro_sam4e16e.overlay
Outdated
Show resolved
Hide resolved
Member
|
Hi @bjarki-andreasen , As you mentioned in the pvt chat I think I found it, erase-blocks = <&eefc 8 2048>, <&eefc 252 8192>; is incorrect, should be erase-blocks = <&eefc 8 2048>, <&eefc 254 8192>; see samx7xx21.dtsi . I tested and now it passes on SAMV7 too. Please, send the final update : ) |
The implementation for erasing pages in the flash_sam.c driver indicated a successful erase after exceeding the last page to be erased successfully. Since the last page has no proceeding page, the succeeded status was not set. This fix switches around the status to be set as succeeded before erase begins, to have the succeeded status cleared if page unlock or erase fails. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
The flash erase-blocks property was a bit malformed, it has 254 pages of size 8192 after the first 8 pages of size 2048 to reach the total size of 2048K. This caused issues when trying to erase the last pages of the flash, which caused the flash erase_blocks test suite to fail. Adjust the erase-blocks to match the flash layout. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
bjarki-andreasen
force-pushed
the
flash-sam-fix-last-page-erase
branch
from
7c75749 to
ea1b6b9
Compare
nandojve
approved these changes
Mar 5, 2025
pdgendt
approved these changes
Mar 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The implementation for erasing pages in the flash_sam.c driver indicated a successful erase after exceeding the last page to be erased successfully. Since the last page has no proceeding page, the succeeded status was not set.
This fix switches around the status to be set as succeeded before erase begins, to have the succeeded status cleared if page unlock or erase fails.
fixes: #70344