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