Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30767 )
Change subject: mb/asus/h61m-cs: Add ASUS H61M-CS ......................................................................
Patch Set 8: Code-Review+1
(7 comments)
Looking very good. I'm happy to give this a +2 once the following comments are addressed.
Also, are you planning on adding a data.vbt file?
https://review.coreboot.org/#/c/30767/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30767/7//COMMIT_MSG@9 PS7, Line 9: Working Have you rechecked this with the latest patch set? It's very unlikely that anything will have broken, but best to be sure.
https://review.coreboot.org/#/c/30767/7//COMMIT_MSG@18 PS7, Line 18: S3 (Resume causes a reboot) Have you tested this patch rebased on the latest master? The S3 issue might have been fixed with CB:30789.
https://review.coreboot.org/#/c/30767/7//COMMIT_MSG@26 PS7, Line 26: (realdevmaster64@gmail.com) Please change to angle brackets <...>
https://review.coreboot.org/#/c/30767/7/src/mainboard/asus/h61m-cs/Kconfig File src/mainboard/asus/h61m-cs/Kconfig:
https://review.coreboot.org/#/c/30767/7/src/mainboard/asus/h61m-cs/Kconfig@1... PS7, Line 17: select HAVE_CMOS_DEFAULT Also add `select NO_UART_ON_SUPERIO`, it will decrease boot time.
https://review.coreboot.org/#/c/30767/7/src/mainboard/asus/h61m-cs/Kconfig@3... PS7, Line 35: config DRAM_RESET_GATE_GPIO : int : default 60 This is the default, so it can be deleted.
https://review.coreboot.org/#/c/30767/7/src/mainboard/asus/h61m-cs/Kconfig@4... PS7, Line 43: config USBDEBUG_HCD_INDEX : int : default 2 It's fine if you can't test this. Please delete it, so that it doesn't give the appearance that it has been tested.
https://review.coreboot.org/#/c/30767/7/src/mainboard/asus/h61m-cs/acpi/supe... File src/mainboard/asus/h61m-cs/acpi/superio.asl:
PS7: I know PS/2 is untested, but please add
#include <drivers/pc80/pc/ps2_controller.asl>
here, as some operating systems will need it.