Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Documentation: Minor fixes for X210 page ......................................................................
Documentation: Minor fixes for X210 page
Fix code blocks and some minor stuff.
Change-Id: Iecf04b00ed12323c124517f2557cc8b60640b618 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- M Documentation/mainboard/51nb/x210.md 1 file changed, 7 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/39670/1
diff --git a/Documentation/mainboard/51nb/x210.md b/Documentation/mainboard/51nb/x210.md index 645c3ba..2c41fd8 100644 --- a/Documentation/mainboard/51nb/x210.md +++ b/Documentation/mainboard/51nb/x210.md @@ -4,20 +4,21 @@
EC firmware is included in the SPI image. To extract it, run:
-`` +``` dd bs=64K skip=32 count=1 if=bios.rom of=ec.bin -`` +```
-and ensure that you have a file that includes the string "Insyde Software Corp" +and ensure that you have a file that includes the string "Insyde Software Corp".
## Flashing instructions
This can be performed using the internal SPI controller, even when flashing -from stock firmware. Use flashrom -p internal and follow the appropriate +from stock firmware. Use `flashrom -p internal` and follow the appropriate flashrom instructions to force it. Alternatively, external flashing has been tested with Dediprog SF100 and SF600 and using a Beaglebone Black. The flash is located on the upper side of the motherboard, below the keyboard connector. It is circled in red here: + 
## Flashing a subset of the ROM @@ -32,14 +33,14 @@ 00210000:007fffff main ```
-and run flashrom with the "--layout rom.layout --image main" arguments. This +and run flashrom with the `--layout rom.layout --image main` arguments. This will flash the main firmware without overwriting the existing EC or ME firmware.
## Working
All hardware features are believed to be working, although the SD reader is -untested. Note that certain hotkeys don't work (including the Thinkvantage +untested. Note that certain hotkeys don't work (including the ThinkVantage button) - this is a limitation of the EC firmware, and these keys also generate no events under the stock vendor firmware.
Peter Lemenkov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Documentation: Minor fixes for X210 page ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Documentation: Minor fixes for X210 page ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG@7 PS1, Line 7: Documentation: Minor fixes for X210 page How about:
Doc/mb/51nb/x210: Do minor fixes
Fix code blocks, add a newline and inline blocks for commands.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Documentation: Minor fixes for X210 page ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG@7 PS1, Line 7: Documentation: Minor fixes for X210 page Or:
Doc: Improve X210 page
mb/51nb/x210: Improve documentation
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... File Documentation/mainboard/51nb/x210.md:
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... PS1, Line 9: ``` At least for one line blocks, I prefer indenting with four spaces.
Hello build bot (Jenkins), Peter Lemenkov, Paul Menzel, Angel Pons, Matthew Garrett,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39670
to look at the new patch set (#2).
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Doc/mb/51nb/x210: Do minor fixes
Fix code blocks, add a newline, use inline code blocks for commands.
Change-Id: Iecf04b00ed12323c124517f2557cc8b60640b618 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- M Documentation/mainboard/51nb/x210.md 1 file changed, 7 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/39670/2
Evgeny Zinoviev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39670/1//COMMIT_MSG@7 PS1, Line 7: Documentation: Minor fixes for X210 page
Or: […]
Is that really so important for commit with minor fixes? :)
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... File Documentation/mainboard/51nb/x210.md:
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... PS1, Line 9: ```
At least for one line blocks, I prefer indenting with four spaces.
There are other pages with one line code blocks with ```. If we want to change it, would be better to do it in separate commit.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... File Documentation/mainboard/51nb/x210.md:
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... PS1, Line 9: ```
There are other pages with one line code blocks with ```. […]
I don't think this is worrying enough to block this change, or is it?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... File Documentation/mainboard/51nb/x210.md:
https://review.coreboot.org/c/coreboot/+/39670/1/Documentation/mainboard/51n... PS1, Line 9: ```
I don't think this is worrying enough to block this change, or is it?
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Doc/mb/51nb/x210: Do minor fixes
Fix code blocks, add a newline, use inline code blocks for commands.
Change-Id: Iecf04b00ed12323c124517f2557cc8b60640b618 Signed-off-by: Evgeny Zinoviev me@ch1p.io Reviewed-on: https://review.coreboot.org/c/coreboot/+/39670 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Peter Lemenkov lemenkov@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M Documentation/mainboard/51nb/x210.md 1 file changed, 7 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Peter Lemenkov: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/Documentation/mainboard/51nb/x210.md b/Documentation/mainboard/51nb/x210.md index 645c3ba..2c41fd8 100644 --- a/Documentation/mainboard/51nb/x210.md +++ b/Documentation/mainboard/51nb/x210.md @@ -4,20 +4,21 @@
EC firmware is included in the SPI image. To extract it, run:
-`` +``` dd bs=64K skip=32 count=1 if=bios.rom of=ec.bin -`` +```
-and ensure that you have a file that includes the string "Insyde Software Corp" +and ensure that you have a file that includes the string "Insyde Software Corp".
## Flashing instructions
This can be performed using the internal SPI controller, even when flashing -from stock firmware. Use flashrom -p internal and follow the appropriate +from stock firmware. Use `flashrom -p internal` and follow the appropriate flashrom instructions to force it. Alternatively, external flashing has been tested with Dediprog SF100 and SF600 and using a Beaglebone Black. The flash is located on the upper side of the motherboard, below the keyboard connector. It is circled in red here: + 
## Flashing a subset of the ROM @@ -32,14 +33,14 @@ 00210000:007fffff main ```
-and run flashrom with the "--layout rom.layout --image main" arguments. This +and run flashrom with the `--layout rom.layout --image main` arguments. This will flash the main firmware without overwriting the existing EC or ME firmware.
## Working
All hardware features are believed to be working, although the SD reader is -untested. Note that certain hotkeys don't work (including the Thinkvantage +untested. Note that certain hotkeys don't work (including the ThinkVantage button) - this is a limitation of the EC firmware, and these keys also generate no events under the stock vendor firmware.
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39670 )
Change subject: Doc/mb/51nb/x210: Do minor fixes ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3006 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3005 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3004 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3003
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Evgeny Zinoviev (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Peter Lemenkov (Code Review)