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