Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38978 )
Change subject: [WIP] mainboard: Add Acer ES1-572 ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... File src/mainboard/acer/es1-572/Kconfig:
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... PS4, Line 1: ## IMHO config is not copyrightable
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... PS4, Line 16: if BOARD_ACER_ES1_572 || BOARD_ACER_EXTENSA_2540 if you're sure that there is no other device out there with that board, I'd be ok with that. otherwise why not use a "baseboard scheme" here?
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... PS4, Line 60: config ADD_FSP_BINARIES : default y I read somewhere we normally don't set this to default "yes" because coreboot should be free by default (even when the board does not work) - not sure if that's still true
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... File src/mainboard/acer/es1-572/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38978/4/src/mainboard/acer/es1-572/... PS4, Line 261: device pci 1f.0 on end # LPC Interface no superio?