Attention is currently required from: Name of user not set #1005084.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/76853?usp=email )
Change subject: flashchips: Add support for MXIC MX2525643G It is similar to the MX2525635F and shares its RDID. ......................................................................
Patch Set 1:
(10 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/76853/comment/5b7e47b2_fa06df8f : PS1, Line 7: MX2525643G Let's use exact name MX25U25643G , can you please update?
https://review.coreboot.org/c/flashrom/+/76853/comment/c63ecb56_34d773ce : PS1, Line 8: MX2525635F Same here, probably MX25U25635F
https://review.coreboot.org/c/flashrom/+/76853/comment/978d2742_ecc2afdf : PS1, Line 10: Tested: read, write and erase. Could you please add info which programmer you ran?
https://review.coreboot.org/c/flashrom/+/76853/comment/82c36f2b_9b710bbd : PS1, Line 12: Datasheet is available at the following URL: : https://www.mxic.com.tw/Lists/Datasheet/Attachments/8766/MX2525643G,%201.8V.... You need to update the link (the new one you gave me in the comment works)
Patchset:
PS1:
Dear Anastasia Klimchuk […]
Thank you, new link works, I opened datasheet successfully. I added some comments, you will need to make changes to your patch and then upload new version and respond to comments. If you have questions, feel free to ask!
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/76853/comment/1788f041_bcb07613 : PS1, Line 10094: MACRONIX_MX25U25643G After you fix my other comment about id, you will change this to MACRONIX_MX25U25635F
https://review.coreboot.org/c/flashrom/+/76853/comment/56f7cf37_7ba56c55 : PS1, Line 10104: { : .eraseblocks = { {4 * 1024, 8192} }, : .block_erase = SPI_BLOCK_ERASE_21, : }, Maybe I am missing something, I don't see this in the datasheet. I can see 20h for sector erase but not 21?
https://review.coreboot.org/c/flashrom/+/76853/comment/ee93c4dd_8d63d827 : PS1, Line 10113: { : .eraseblocks = { {32 * 1024, 1024} }, : .block_erase = SPI_BLOCK_ERASE_52, : }, same
https://review.coreboot.org/c/flashrom/+/76853/comment/aff5d7a2_e211f672 : PS1, Line 10117: .eraseblocks = { {64 * 1024, 512} }, : .block_erase = SPI_BLOCK_ERASE_DC, : }, { same
File include/flashchips.h:
https://review.coreboot.org/c/flashrom/+/76853/comment/15ae0d87_479973b2 : PS1, Line 530: #define MACRONIX_MX25U25643G 0x2539 Don't add the same id, add a comment on existing id (MACRONIX_MX25U25635F), like that:
/* Same as MACRONIX_MX25U25635F */
You can look at examples of the same situation with same ids just few lines above in this file.