Daniel Thompson has posted comments on this change. ( https://review.coreboot.org/26949 )
Change subject: flashchips: Add Macronix MX25U51245G ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/26949/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/26949/2//COMMIT_MSG@14 PS2, Line 14: the later means I : have tested 4-byte addressing
Only if you verified the result with another programmer... […]
Agree, and its why I was fairly fastidious in documenting exactly what I did test.
You want me to add something like "but only enough to show the chip doesn't report errors when we try to use 4 byte addressing" to the commit message?
More importantly, do you think the test OK flags I used are acceptable with this level of testing?
https://review.coreboot.org/#/c/26949/2/flashchips.c File flashchips.c:
https://review.coreboot.org/#/c/26949/2/flashchips.c@8572 PS2, Line 8572: 512B
datasheet says 8kb (4kb factory, 4kb customer)
8kbit isn't it? Or are bit units implicit?
MX25U12835F has 4kbit OTP and is commented as 512B. Is that a mistake or does the B imply bytes?
Changing to this is OK?
/* OTP: 512B factory programmed and 512B customer programmed; enter 0xB1, exit 0xC1 */
Wow... this comment has a lot of question marks in it!
https://review.coreboot.org/#/c/26949/2/flashchips.c@8579 PS2, Line 8579: { : .eraseblocks = { {4 * 1024, 16384} }, : .block_erase = spi_block_erase_20, : }, { : .eraseblocks = { {32 * 1024, 2048} }, : .block_erase = spi_block_erase_52, : }, { : .eraseblocks = { {64 * 1024, 1024} }, : .block_erase = spi_block_erase_d8, : }
Please list the native 4BA versions too: 21 5c dc
Will fix.
-
Daniel Thompson (Code Review)