Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, garyodernichts@gmail.com.
Anastasia Klimchuk has posted comments on this change by garyodernichts@gmail.com. ( https://review.coreboot.org/c/flashrom/+/85869?usp=email )
Change subject: flashchips/micron: Not all N25QL256 support 4BA_WRITE ......................................................................
Patch Set 1:
(3 comments)
Patchset:
PS1:
Looking at this again, it might be better to add a separate flashchip entry for the N25Q256A, instea […]
Thank you for your work! I agree with your idea, it is better to split. Those models which do support 4ba can take advantage of it, and the one who doesn't will have a separate chip definition.
It seems to me, the new definition will go to `micron_numonyx_st.c` right?
The thing to take care about is, eraseblocks ordering. If the new definition does not support 4ba, then first eraseblock for a given size should be 3ba (for example, 20h should be first and then 21h). I know this is not ideal and I want to fix, so that flashrom knows which erase opcodes correspond to 4ba vs 3ba - but at the moment it is what it is.
Commit Message:
https://review.coreboot.org/c/flashrom/+/85869/comment/9d2a30d6_64800319?usp... : PS1, Line 13: Since there are other chips with the N25Q256 rdid (0xBA19), it is not safe to assume that all chips support 4BA_WRITE. : When testing this on a 25Q256, the 4BA_WRITE command behaved more like an erase by writing all 0xFFs. : With the patch, which is now using the default page program operation, it works fine. We wrap commit message by 72 chars width, could you please wrap the lines?
https://flashrom.org/dev_guide/development_guide.html#commit-message-1
https://review.coreboot.org/c/flashrom/+/85869/comment/aec62b7e_2e4b1715?usp... : PS1, Line 16: You can mention in commit message that the patch splits an entry into two (that was in the other comment thread),
and also if you could add link to datasheet into a commit message, thanks!
-
Anastasia Klimchuk (Code Review)