Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35427 )
Change subject: mb/supermicro/x11: add x11ssm-f board ......................................................................
Patch Set 25:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... PS25, Line 3: select BOARD_SUPERMICRO_BASEBOARD_X11
Might not be compulsory, but I would prefer if the last part matched the directory name here.
Done
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... PS25, Line 4: config BOARD_SUPERMICRO_X11SSM_F
Typically there is an empty line before keyword config. […]
Done
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/mainboard.c:
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... PS25, Line 27: if (CONFIG(BOARD_SUPERMICRO_X11SSM_F)) {
If you ask me, these should be forbidden in the base directory, once you apply variants-directory sc […]
How should this be done, then? (I wanted to move as many stuff as possible to the variants dir but many people complained about it....)
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/ramstage.c:
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... PS25, Line 25: // probably can be removed here
So this code is also variant-dependent?
Seems so, yes; x11ssh crashes, x11ssm does not
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/variants/x11ssm-f/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... PS25, Line 22: #ifndef __ACPI__
Why? This file does not provide anything usable for . […]
This was copied from x11ssh. I have no clue. See src/mainboard/intel/kblrvp/variants/rvp11/include/variant/gpio.h
https://review.coreboot.org/c/coreboot/+/35427/25/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11-lga1151-series/variants/x11ssm-f/working_config:
PS25:
We have configs/ directory, maybe better choice would be to commit the result of 'make savedefconfig […]
as the board isn't complete yet, I wanted to have a complete, fully working config somewhere (to make testing for others easier). As soon as all problems are resolved, I aggree that this should be removed/moved.