Piotr Kleinschmidt has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32907 )
Change subject: Documentation: How to run coreboot on PC Engines APU1 ......................................................................
Patch Set 3:
(21 comments)
This change is ready for review.
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG@7 PS1, Line 7: apu1
APU1
Done
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG@7 PS1, Line 7: [WIP]
You can tag this WIP in Gerrit, so you do not need to amend the commit message.
Done
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG@8 PS1, Line 8: : There were no documentation about running coreboot on apu1 platform, so now it describes how to do this.
Please wrap the line after 75/72 characters.
Done
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG@9 PS1, Line 9: were
is
Done
https://review.coreboot.org/#/c/32907/1//COMMIT_MSG@11 PS1, Line 11: Signed-off-by: Piotr Kleinschmidt piotr.kleinschmidt@3mdeb.com
Please move it below the Change-Id line.
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... File Documentation/mainboard/pcengines/apu1.md:
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 1: apu1
APU1
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 13: | Boot | From SD card, USB, m-SATA |
mSATA, SATA
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 25: | Write protection | no |
available with jumper on #WP pin?
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 31: [flashrom]
Does this work? Shouldn’t it be `[flashrom][]`?
According to other documents it should work fine.
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 34: `flashrom -p internal -c MX25L1606E -w coreboot.rom `
should also be "MX25L1606E", flashrom expects the chip to be passed in quotes ""
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 39: ACPI : Specification documentation
Add information that S5 can be forced by shorting power button pin on J2 header.
I deleted reference to that documentation and only gave information how to force that state
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 43: which is
Can be removed.
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 48: to programmer device
to *the* …
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 49: a linux_spi
a linux_spi *device*?
It's not actually linux_spi device, so now I described it more precisely.
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 52: `flashrom -w coreboot.rom -p linux_spi:dev=/dev/spidev1.0,spispeed=16000 -c : "MX25L1606E"`
Indent this please instead of using backticks.
Done
https://review.coreboot.org/#/c/32907/1/Documentation/mainboard/pcengines/ap... PS1, Line 63: [here]: https://www.coreboot.org/Binary_situation
Where is this used?
I deleted it, it was unnecessary.
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... File Documentation/mainboard/pcengines/apu1.md:
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... PS2, Line 1: # PC Engines APU1
reference file from index. […]
Added reference to index.md
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... PS2, Line 7: | | |
use supported tables: […]
Done
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... PS2, Line 27: r
EC or Super IO?
Added information about Super IO
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... PS2, Line 43: SOP-8 header next to the flash chip on the board. Notice that not all boards
pin header […]
Added pin layout as picture at the end of the document
https://review.coreboot.org/#/c/32907/2/Documentation/mainboard/pcengines/ap... PS2, Line 48: There is no restrictions as to the programmer device. However, [flashrom] is
no need for that, already covered by https://doc.coreboot.org/flash_tutorial/index.html […]
Done