Attention is currently required from: Bora Guvendik, DZ, Nikolai Artemiev, Stefan Reinauer, Subrata Banik.
Anastasia Klimchuk has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/flashrom/+/82626?usp=email )
Change subject: flashchips: add support for MX77U51250F chip ......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82626/comment/53b95ded_c4afbc6e?usp... : PS3, Line 10: Please add test information in commit message. Specifically, which programmer you are using (if that is internal, then which board was it on). Also which operations have you tested. From the other comment, if you are testing write-protect, also mention which exactly operations you were testing for wp. Thank you!
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/82626/comment/bd199659_1790d1b1?usp... : PS3, Line 11618: 65536
we might need to fake this part to show the max capacity is 32MB
I think this kind of custom adjustments should be added to ChromeOS fork (which is I assume what you meant to say, but I just want to be clear). I fully understand the idea to speed up flashing if you only happen to use half of available capacity. But if this flash has 64MB size, it should be defined like this in the flashchip definition. We have users around the world, and upstream definition should correctly describe the features of the chip model.
https://review.coreboot.org/c/flashrom/+/82626/comment/170295c4_aa379a58?usp... : PS3, Line 11620: .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA,
In the datasheet, this Flash doesn't support FEATURE_WRSR2, FEATURE_WRSR3.
How many bytes status register has? Perhaps it should be defined with `FEATURE_WRSR_EXT2` and/or `FEATURE_WRSR_EXT3` ? It depends on whether you can only write the whole status register (all bytes at the same time), or you can write each byte independently.
https://review.coreboot.org/c/flashrom/+/82626/comment/98f8f88d_09500662?usp... : PS3, Line 11648: },
OK, thank you for providing these backgroud information.
If you plan to add write-protect support (i.e. define reg_bits), you also need to test it , and mark test status accordingly. Current test status TEST_OK_PREW indicates that write-protect is not tested. If you test it successfully, then test status should be updated to TEST_OK_PREWB
-
Anastasia Klimchuk (Code Review)