Attention is currently required from: Felix Singer, Paul Menzel, Michael Büchler. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56838 )
Change subject: mb/acer/g43t-am3: Add documentation ......................................................................
Patch Set 2: Code-Review+1
(4 comments)
Patchset:
PS2: Sorry, this fell off my radar. A nice way to ping reviewers of a change is to rebase it atop current master from time to time.
File Documentation/mainboard/acer/g43t-am3.md:
https://review.coreboot.org/c/coreboot/+/56838/comment/717be60c_c7e31e11 PS2, Line 105: `-c MX25L1605D/MX25L1608D/MX25L1673E` and `-c MX25L1605` should work. Hmmm, the block erasers defined in flashrom for these chips are incompatible: opcode 0x20 erases 64 KiB with `MX25L1605`, and 4 KiB with `MX25L1605D/MX25L1608D/MX25L1673E`. I'm pretty sure the latter is the correct definition, since Intel southbridges require flash chips to support 4 KiB erase granularity to use descriptor mode.
https://review.coreboot.org/c/coreboot/+/56838/comment/6f0f5045_5ff87467 PS2, Line 108: flashrom -p internal -r backup.rom Huh, I would've expected the ME region to be unreadable internally. Have you modified the IFD?
https://review.coreboot.org/c/coreboot/+/56838/comment/f464ac08_9d21c5ba PS2, Line 120: There : seems to be a diode that prevents you from powering the whole board with : your external programmer. I typically avoid using 2nd person (you, your) when writing documentation. Here I would rephrase this as follows:
There seems to be a diode that prevents the external programmer from powering the whole board.
Feel free to change this sentence or leave it as-is.
-
Angel Pons (Code Review)