Christoph Pomaska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35163 ) Change subject: mb/supermicro: Add X10SLM+-LN4F as X10SLM variant ...................................................................... Patch Set 3: (9 comments) https://review.coreboot.org/c/coreboot/+/35163/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35163/2//COMMIT_MSG@9 PS2, Line 9: X10SLM-F
X10SLM+-F Done
https://review.coreboot.org/c/coreboot/+/35163/2//COMMIT_MSG@10 PS2, Line 10: X10SLM-LN4F
X10SLM+-LN4F Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x10slm/Kconfig: https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 20: config BOARD_SPECIFIC_OPTIONS
This options here are sorted alphabetically, and it would be nice to […] I changed the order to make it more structed actually, such as putting the superio chips together. Honestly I don't know if there is a sorting convention for these listings.
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 35: select MAINBOARD_HAS_LPC_TPM : select MAINBOARD_HAS_TPM2
Since this TPM config affects the code flow, and it's untested at least […] Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x10slm/Kconfig.name: https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 3: config BOARD_SUPERMICRO_X10SLM_PLUS_LN4F
Small nit: it would be nice to have a blank line between the two configs. Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... File src/mainboard/supermicro/x10slm/gpio.c: PS2:
Please put solely formatting changes in a separate patch. Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 192: {
that open brace { should be on the previous line Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 201: {
that open brace { should be on the previous line Done
https://review.coreboot.org/c/coreboot/+/35163/2/src/mainboard/supermicro/x1... PS2, Line 208: {
that open brace { should be on the previous line Done
-- To view, visit https://review.coreboot.org/c/coreboot/+/35163 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I686d8d4e2ec5b4eb2db214b6e0827ac9c33829d1 Gerrit-Change-Number: 35163 Gerrit-PatchSet: 3 Gerrit-Owner: Christoph Pomaska <github@aufmachen.jetzt> Gerrit-Reviewer: Christoph Pomaska <github@aufmachen.jetzt> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Tristan Corrick <tristan@corrick.kiwi> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Felix Singer <felixsinger@posteo.net> Gerrit-Comment-Date: Tue, 17 Sep 2019 09:05:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tristan Corrick <tristan@corrick.kiwi> Comment-In-Reply-To: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: comment