Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
This is WIP and partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
It also list what should be documented for new mainboard ports.
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 3 files changed, 95 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/1
diff --git a/Documentation/getting_started/index.md b/Documentation/getting_started/index.md index 52d873e..7821ee1 100644 --- a/Documentation/getting_started/index.md +++ b/Documentation/getting_started/index.md @@ -7,3 +7,4 @@ * [Gerrit Guidelines](gerrit_guidelines.md) * [Documentation License](license.md) * [Writing Documentation](writing_documentation.md) +* [Adding new mainboards](new_mainboard_ports.md) diff --git a/Documentation/getting_started/new_mainboard_ports.md b/Documentation/getting_started/new_mainboard_ports.md new file mode 100644 index 0000000..cb7cd9b --- /dev/null +++ b/Documentation/getting_started/new_mainboard_ports.md @@ -0,0 +1,91 @@ +# Adding new mainboards + +Mainboard code is placed under `src/mainboards`. It glues the HW components +together by selecting appropiate northbridge, southbridge, on-board HW and +drivers. + +It provides the devicetree.cb, configures GPIOs, audio codecs, a static ACPI +board desciption and allows the romstage code to retrieve the SPD for +DRAM training. + +## Starting a new port from scratch + +1. Identify your platform + Try to find out as much as possible about the current hardware. + Dump information with existing tools: + * lspci + * lsusb + * superiotool + * inteltool + * ectool + * dmidecode + * acpidump + + Have a look at the mainboard schematics to see how those components are + connected. +2. Find a similar existing board + It a good idea to find a similar board and use it as reference. You should + then provide a proper GPIO configuration and devicetree.cb. +3. Find a similar SuperIo or BMC + To debug coreboot you should get the serial console working first. If your + board has a SuperIO or BMC try to find a compatible one. + Some boards have UART hardware on the SoC, makeing external components + obsolete. +4. Start with a minimal bootable configuration. That is: + * serial is working + * SPD reading/DRAM is working + * A payload that is able to boot an operating system +5. Be able to recover from a bad flash. You should use external flashing as + it's likely that you will brick your platform on the first attempts. + Make sure to backup the original firmware (if any). + +## Code submission + +Here's a checklist for new mainboard ports: + +1. Try to not duplicate code, use variants scheme instead. +2. Split changes logical into top level directories: + `src/mainboard` , `src/northbridge`, `src/ec`, ... +3. Provide good documentation, see point 4 +4. In the commit message describe as much as possible or point to the + documentation, satisfying the following questions: + * Where's the flash IC located? + * Can you flash incircuit? + * Are there pinheaders for flashing? + * Is the flash hardware write-protected? + * Which flash IC is usually equipped? + * If the board features an BMC, can it be used for developing and + debugging? + * What was tested and is working? + * What was tested and isn't working? + * What wasn't tested (due to lack of testing equipment)? + * Are blobs necessary? + * How can the board be debugged? + * Are there serial, EHCI debug, xHCI debug or BMC connections? + * Does it have a SuperIO? + + In addition please descibe: + * How to retrieve blobs, like dumping them from vendor firmware + + See [Writing Documentation] for more details. + +5. What you should **not** document: + * Steps or instructions how to flash the board + * Steps or instructions how to use a specific application for flashing + * Please do not provide pictures of the whole board or it's backplate + connectors +6. Recommand a flashing method as descibed in [Flashing tutorial] +7. If you are working at/for a hardware vendor, please provide free board + schematics as well. +8. Provide a picture of the flash IC or flash connector. + * The picture should be less than 800pc in width and compressed with + 70% compression to reduce size. + * You must own the Copyright + * Try to cut of uninteresting parts of the image, like tables, cables, ... + +Once done submit your board to Gerrit. Please follow the [Gerrit Guidelines] as +well. + +[Writing Documentation]: writing_documentation.md +[Flashing tutorial]: ../flash_tutorial/index.md +[Gerrit Guidelines]: gerrit_guidelines.md diff --git a/Documentation/getting_started/writing_documentation.md b/Documentation/getting_started/writing_documentation.md index fb942a4..0b820fc 100644 --- a/Documentation/getting_started/writing_documentation.md +++ b/Documentation/getting_started/writing_documentation.md @@ -49,6 +49,8 @@ the current theme doesn't allow bigger images. 12. Shouldn't cover implementation details; for details, the code is the reference. +13. New mainboard ports should also add documentation. See + [Adding new mainboards] for more details.
## Referencing markdown documents
@@ -122,3 +124,4 @@ [Markdown Guide]: https://www.markdownguide.org/ [Gerrit Guidelines]: gerrit_guidelines.md [review.coreboot.org]: https://review.coreboot.org +[Adding new mainboards]: new_mainboard_ports.md
Patrick Rudolph has uploaded a new patch set (#2) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
Add a checklist howt to port new boards, how to submit new board ports, and provide a markdown template for new boards.
This is partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/mainboard_template.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 4 files changed, 207 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/2
Patrick Rudolph has uploaded a new patch set (#3) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
Add a checklist how to port new boards, how to submit new board ports, and provide a markdown template for new boards.
This is partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/mainboard_template.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 4 files changed, 208 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/3
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 124: Remove the blank lines.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 9: DRAM Put on line above?
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 61: descibed described
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 61: Recommand Recommend
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 65: pc px
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 68: of off
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 40: * How can the blobs be retrieved? these
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 13: 1. Identify your platform 0. Backup existing Vendor Firmware? :P
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 22: * acpidump Maybe we should write a little bit more here for people who are not familiar with the tools - what commands should they use? e.g. acpidump -b etc.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 28: then provide a proper GPIO configuration and devicetree.cb. Where do I get those from?
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 29: 3. Find a similar SuperIo or BMC SuperIO
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 32: Some boards have UART hardware on the SoC, makeing external components making
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 24: * 'EVT' : * 'DVT' I find using abbreviations without explanation is a bad style. There is a chance that different people have different understanding for a given abbreviation. Please write the meaning down here.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 28: 'EOL' same here
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 55: Can you flash in-circuit using a SOIC-8 clip? Please add here something to cover the board power supply. Something like:
'Need the ISP programmer provide the flash voltage or need the board be powered for flashing? If the ISP programmer needs to provide the voltage, which value should it have?'
could be helpful. Though part of it is mentioned in the lower section.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 23: Add a hint here to take care about the ME/TXE on Intel plattforms? Things like 'Has the mainboard one or two SPI flashes for ME and BIOS?' and 'How are the access rights set up in the descriptor?' may influence the final image creation.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 63: schematics as well. if these are available.
Hello Werner Zeh, ron minnich, Felix Held, Patrick Rudolph, Matt DeVillier, Christian Walter, David Hendricks, Stefan Reinauer, Philipp Deppenwiese, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34632
to look at the new patch set (#4).
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
Add a checklist how to port new boards, how to submit new board ports, and provide a markdown template for new boards.
This is partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/mainboard_template.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 4 files changed, 217 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4:
(17 comments)
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 24: * 'EVT' : * 'DVT'
I find using abbreviations without explanation is a bad style. […]
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 28: 'EOL'
same here
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 40: * How can the blobs be retrieved?
these
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 55: Can you flash in-circuit using a SOIC-8 clip?
Please add here something to cover the board power supply. Something like: […]
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 124:
Remove the blank lines.
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 9: DRAM
Put on line above?
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 13: 1. Identify your platform
- […]
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 22: * acpidump
Maybe we should write a little bit more here for people who are not familiar with the tools - what c […]
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 23:
Add a hint here to take care about the ME/TXE on Intel plattforms? Things like 'Has the mainboard on […]
Mentioned SPI flash count here.
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 28: then provide a proper GPIO configuration and devicetree.cb.
Where do I get those from?
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 29: 3. Find a similar SuperIo or BMC
SuperIO
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 32: Some boards have UART hardware on the SoC, makeing external components
making
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 61: Recommand
Recommend
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 61: descibed
described
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 63: schematics as well.
if these are available.
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 65: pc
px
Done
https://review.coreboot.org/c/coreboot/+/34632/3/Documentation/getting_start... PS3, Line 68: of
off
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4: Code-Review+2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 24: * 'EVT early prototype' (Unit is for 'Engineering validation test') : * 'DVT prototype' (Unit is for 'Design verification test') This is generally confidential information, and is generally only a few months apart.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 30: * What's the expected release date? This is also confidential information
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 7: It provides "It generally provides"? For example the AMD Picasso code won't be getting SPDs since memory is already up.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 16: Try to find out as much as possible about the current hardware. : Dump information with existing tools: : * lspci : * lsusb : * superiotool : * inteltool : * ectool : * dmidecode : * acpidump This is only valid for a port from an IBV BIOS.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 53: scheme instead. When it makes sense.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 56: Provide good documentation, see point 4 This was discussed in the coreboot leadership meeting, and while desirable, documentation is NOT a requirement to submit a board, and should not be presented as such.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 63: What you should **not** document: Why?
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 68: Recommended a flashing method as described in [Flashing tutorial] Doesn't this directly contradict point 5?
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/writing_documentation.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 52: New mainboard ports should also add documentation "It's requested, although not required that new mainboards add documentation."
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 24: * 'EVT early prototype' (Unit is for 'Engineering validation test') : * 'DVT prototype' (Unit is for 'Design verification test')
This is generally confidential information, and is generally only a few months apart.
I don't see why we should let the documentation rot for "a few" month. It should be as accurate as possible, as there's no other source of information.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 30: * What's the expected release date?
This is also confidential information
I don't see why releasing hardware specs (by pushing source code) is NOT confidential, but the release date should be. You can still answer this question with: "in the future"
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 56: Provide good documentation, see point 4
This was discussed in the coreboot leadership meeting, and while desirable, documentation is NOT a r […]
Nothing to comment here.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 63: What you should **not** document:
Why?
Violates https://doc.coreboot.org/getting_started/writing_documentation.html point 9.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 68: Recommended a flashing method as described in [Flashing tutorial]
Doesn't this directly contradict point 5?
No, instead of duplication how to use "flashrom -p internal" on every mainboard, you'd just reference existing documentation.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 24: * 'EVT early prototype' (Unit is for 'Engineering validation test') : * 'DVT prototype' (Unit is for 'Design verification test')
I don't see why we should let the documentation rot for "a few" month. […]
I'd still rather we didn't request this information in the template. I worry that it's going to get people to release information that they shouldn't be releasing. "Unreleased" should be sufficient in my opinion.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 30: * What's the expected release date?
I don't see why releasing hardware specs (by pushing source code) is NOT confidential, but the relea […]
I can understand your confusion, nevertheless, releasing the hardware specs by pushing source code is OK, but the release date is confidential.
From some internal documentation: """ It is ok to include Google project 'code' names (ie 'board' names) in public bugs / CLs. That is what the code names are for. It is ok to associate most established technology / features with these board names (aka, CPU vendor, type & specs, touch / no-touch, panel size, etc). """
Maybe "* What's the expected release date (if public)?"
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 16: Try to find out as much as possible about the current hardware. : Dump information with existing tools: : * lspci : * lsusb : * superiotool : * inteltool : * ectool : * dmidecode : * acpidump
This is only valid for a port from an IBV BIOS.
Maybe: If you are porting a platform from an existing BIOS, try to find out...
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 56: Provide good documentation, see point 4
Nothing to comment here.
Maybe "Provide good documentation when possible. See point 4"
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 63: What you should **not** document:
Violates https://doc.coreboot.org/getting_started/writing_documentation.html point 9.
You might want to make that more clear here then:
5. What you should **not** have in the mainboard documentation: * Generic steps or instructions how to flash the board. I.e. instead of duplication how to use "flashrom -p internal" on every mainboard, just reference existing documentation. * Similar to the above comment, don't document steps or instructions how to use a specific application for flashing, instead document that in the flashing tutorial if it's different from existing flash methods, then reference that documentation.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 68: Recommended a flashing method as described in [Flashing tutorial]
No, instead of duplication how to use "flashrom -p internal" on every mainboard, you'd just referenc […]
I see now. I think this should be "Recommend" instead of "Recommended" or maybe "A recommended flashing method" then.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 74: You must own the Copyright Thank you. Maybe:
* You must own the copyright so only submit your own photos, not downloaded photos
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 4:
(29 comments)
I'm not sure if using a template would be better than suggesting the same procedure used for mainboards: just copy the documentation from another similar mainboard and edit it until it's true for your board.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 15: Consumer What's the criteria one should use to choose this?
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 24: * 'EVT early prototype' (Unit is for 'Engineering validation test') : * 'DVT prototype' (Unit is for 'Design verification test')
I'd still rather we didn't request this information in the template. […]
Would "Prototype" be good enough?
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 30: * What's the expected release date?
I can understand your confusion, nevertheless, releasing the hardware specs by pushing source code i […]
I agree with Martin, "What's the expected release date (if public)?" should be good enough.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 42: * Are the blobs part of the coreboot build process? Please do not count the IFD, GbE, ME, et. al. as part of the coreboot build process. It's just more things that could go wrong.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 44: ## Programming In-circuit programming is worth documenting thoroughly. For instance, presence of a protection diode on the flash chip (or flash chips! there can be several) power rail and resistors between the flash chip(s) and the SoC/chipset means in-circuit programming is intended to work.
I have also seen "programming" headers, which are actually intended to be used with a second flash chip, bypassing the on-board flash chip using the HOLD# pin or a logic gate. That header does not mean the board is safe for in-circuit programming.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 47: Where's the programming header? : * Can you provide a picture? Ideally the picture shows the header, and where it is on the board (by giving some context around it)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 56: BMC BMC or SuperIO or embedded controller
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 59: * Are there 'backup' features, like DualBIOS, enabled? I'd specify that DualBIOS is a Gigabyte-specific feature (it's even patented). I wouldn't say "enabled", though - "present" sounds better.
And no, software-based "backup" features won't work with coreboot.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 77: Does it support USB serial? Is there any board that can do this?
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 78: LVTTL Even I don't know where I'd find that
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 83: schematics schematics and/or boardviews
Most of these are classified as confidential information, though.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 89: W Please add something stating something along the lines of:
"thing X does not work" is as useful as saying nothing at all. Instead, please consider explaining how the actual behavior differs from the desired behavior, e.g.: "plugging an external graphics card causes the fans to spin at full speed". Additionally, it is advisable to document what was attempted to fix the issue, so that others don't suggest/try it again in vain.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 3: mainboards singular 'mainboard'
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 3: Mainboard code is placed under `src/mainboards`. It glues the HW components : together by selecting appropiate northbridge, southbridge, on-board HW and : drivers. : : It provides the devicetree.cb, configures GPIOs, audio codecs, a static ACPI : board desciption and allows the romstage code to retrieve the SPD for DRAM : training. I would suggest rewriting this a bit:
Mainboard code, located under `src/mainboard`, specifies how hardware components are connected together on a specific mainboard. Examples include: the devicetree, GPIO settings, audio codec parameters, the ACPI DSDT, memory-related parameters...
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 13: B I almost missed this. Please make it thicker, so that people will see it.
For example, explain what to do a backup for (so that there's a possibility to recover, and also machine-specific data like MAC addresses), how to do a backup, why one backup is not enough (at least one copy should be off-site, just in case), why using me_cleaned firmwares for coreboot porting is bad (remember p8h61-m pro and LPC not working fine without a working ME? imagine that, but while porting)...
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 16: Try to find out as much as possible about the current hardware. : Dump information with existing tools: : * lspci : * lsusb : * superiotool : * inteltool : * ectool : * dmidecode : * acpidump
Maybe: If you are porting a platform from an existing BIOS, try to find out...
Maybe "guard" this with an "if you are retrofitting coreboot to a mainboard which already has a working firmware"
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 26: Have a look at the mainboard schematics to see how those components are : connected. Sometimes you don't have schematics/boardviews (you really should, if you're not retrofitting coreboot).
I would suggest focusing on GPIOs, as this is usually the most dangerous thing coreboot configures (get the GPIO configs wrong, and you can short-circuit things)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 33: It a good idea to find a similar board and use it as reference. Add the following remark:
Prefer a board from a different vendor which uses the same platform (chipset) over a board from the same vendor, but which uses a different platform.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 36: 3. Find a similar SuperIO or BMC : To debug coreboot you should get the serial console working first. If your : board has a SuperIO or BMC try to find a compatible one. : Some boards have UART hardware on the SoC, making external components : obsolete. Maybe rewrite as "Find a usable debug console". I would say it is more important than a SuperIO/BMC
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 41: 4. Start with a minimal bootable configuration. That is: Since some people tend to just change all the things in menuconfig, I'd add a recommendation to not do so. The less that deviates from defaults, the easier to track down issues is.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 45: 5. Be able to recover from a bad flash. You should use external flashing as : it's likely that you will brick your platform on the first attempts. : Make sure to backup the original firmware (if any). IMHO, this should be the first thing. If somebody is unable to get a recovery method, then they should reconsider things, because porting things will be impossible for them.
Some people just don't know how to recover from a bad flash (just need knowledge and the tools to do it), whereas others have imposed restrictions which makes it impossible for them to have a reliable recovery method...
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 53: scheme instead.
When it makes sense.
I would prefer to tell somebody to use the variants mechanism, than to have them undo the variants because the result is just spaghetti.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 54: logical should be an adverb: logically
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 63: What you should **not** document:
You might want to make that more clear here then: […]
I would suggest mentioning what to do: reference existing documentation as much as possible. Especially w.r.t. flashing.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 68: Recommended a flashing method as described in [Flashing tutorial]
I see now. […]
Recommended flashing methods could also be generalized into a sorted list, and then mainboard capabilities would just decide which would be the best method. Some examples (assuming one SPI flash chip):
if the flash chip is socketed, just remove it and connect it to your programmer (no in-circuit programming)
if the flash chip isn't socketed, but the mainboard has a diode on the flash chip's VCC rail (so that the rest of the board doesn't get powered on) and also has resistors between the flash chip and the SoC, then it's recommended to do in-circuit programming (board is designed for it).
if the flash chip isn't socketed, has no protection diode nor resistors, and also has a BMC/EC connected to the flash chip, then you're screwed. In-circuit programming is pretty much impossible. If you can attach a second flash chip and disable the on-board flash chip, you could use that (no in-circuit programming). Otherwise, you would have to desolder the flash chip (and if it's WSON... you're gonna have a good time)
Another weird case I have: Toshiba A300-1ME laptop (mainboard name: Inventec Potomac 10G). The board has a single SPI flash chip on the EC (board boots from LPC, using the EC as a translator). But it also has pads for another SPI flash chip, directly connected to the southbridge (no other masters around). In that case, I could have done several things. However, the best solution I found was to wire up the strap that changes the default boot location between LPC and SPI to a switch (the wifi rfkill switch), install a 2nd flash chip to put coreboot on, leave the vendor firmware on the EC-attached flash chip, and flash internally. While it might seem rather complicated, this setup means I can do all the coreboot tests I want with a closed chassis, and without *no* additional hardware to recover from a bad flash, just flipping the rfkill switch does the trick. I'd call this an absolute win over any other recovery method :-)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 70: schematics schematics/boardviews
You don't sound like you ever had to hunt down R347 on a mainboard without a boardview... ಠ_ಠ
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 72: The picture should be less than 800px in width and compressed with : 70% compression to reduce size. How about specifying a size in kiB as a reference? It's easier to check the size while reviewing (it literally appears just there).
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 77: Once done submit your board to Gerrit. How? We have lesson2 for that, so maybe link it here?
Also, encourage people to use gerrit from the very beginning: people can point out mistakes, improvements, suggestions... And if you want your new board merged, you will have to use gerrit anyway.
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/writing_documentation.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 52: New mainboard ports should also add documentation
"It's requested, although not required that new mainboards add documentation. […]
"it is highly appreciated" ?
Patrick Georgi has uploaded a new patch set (#5) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
Add a checklist how to port new boards, how to submit new board ports, and provide a markdown template for new boards.
This is partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/mainboard_template.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 4 files changed, 217 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/5
Patrick Georgi has uploaded a new patch set (#6) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Documentation: Add guidelines for new mainboard ports
Add a checklist how to port new boards, how to submit new board ports, and provide a markdown template for new boards.
This is partly copied from https://www.coreboot.org/Motherboard_Porting_Guide
Change-Id: I134dbf2341696ba0dd33c3d52bf787b8eaabbdab Signed-off-by: Patrick Rudolph siro@das-labor.org --- M Documentation/getting_started/index.md A Documentation/getting_started/mainboard_template.md A Documentation/getting_started/new_mainboard_ports.md M Documentation/getting_started/writing_documentation.md 4 files changed, 219 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/34632/6
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 6:
(8 comments)
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 3: mainboards
singular 'mainboard'
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 7: It provides
"It generally provides"? For example the AMD Picasso code won't be getting SPDs since memory is alr […]
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 16: Try to find out as much as possible about the current hardware. : Dump information with existing tools: : * lspci : * lsusb : * superiotool : * inteltool : * ectool : * dmidecode : * acpidump
Maybe "guard" this with an "if you are retrofitting coreboot to a mainboard which already has a work […]
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 45: 5. Be able to recover from a bad flash. You should use external flashing as : it's likely that you will brick your platform on the first attempts. : Make sure to backup the original firmware (if any).
IMHO, this should be the first thing. […]
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 53: scheme instead.
I would prefer to tell somebody to use the variants mechanism, than to have them undo the variants b […]
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 54: logical
should be an adverb: logically
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 56: Provide good documentation, see point 4
Maybe "Provide good documentation when possible. […]
Done
https://review.coreboot.org/c/coreboot/+/34632/4/Documentation/getting_start... PS4, Line 74: You must own the Copyright
Thank you. Maybe: […]
Done
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 6: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... File Documentation/getting_started/mainboard_template.md:
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 72: * Does it support RS232 serial? Maybe add something to describe the voltage level if a TTL UART is available?
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 30: Maybe add "if available" here avoid confusion?
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 35: Some boards have two SPI flashes for ME and BIOS. Maybe more clear: "Some boards have two SPI flashes, one for ME and a second for BIOS."
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 37: It It is
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 35: Some boards have two SPI flashes for ME and BIOS.
Maybe more clear: […]
in the case of two flashes, it doesn't have to be that the ifd/me/gbe is in one and the bios section is in the other; for example the x230 has parts of the bios section in both flash chips
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34632 )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... File Documentation/getting_started/new_mainboard_ports.md:
https://review.coreboot.org/c/coreboot/+/34632/6/Documentation/getting_start... PS6, Line 35: Some boards have two SPI flashes for ME and BIOS.
in the case of two flashes, it doesn't have to be that the ifd/me/gbe is in one and the bios section […]
OK, fair enough.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34632?usp=email )
Change subject: Documentation: Add guidelines for new mainboard ports ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Angel Pons (Code Review) -
Christian Walter (Code Review)
-
Felix Held (Code Review)
-
Martin L Roth (Code Review)
-
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Werner Zeh (Code Review)