Michael Büchler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44167 )
Change subject: [WIP] mb/acer/m3800: add Acer Aspire M3800 desktop ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44167/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44167/3//COMMIT_MSG@7 PS3, Line 7: [WIP] mb/acer/m3800: add Acer Aspire M3800 desktop I added the WIP label due to a new unresolved issue with the PS/2 keyboard and 3VSB for Suspend-to-RAM
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/Kc... File src/mainboard/acer/m3800/Kconfig:
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/Kc... PS1, Line 12: BOARD_ROMSIZE_KB_2048
Just wondering, does it have an ME region?
Yes it does, ifdtool reports "Flash Region 2 (Intel ME): 00006000 - 000fffff"
https://review.coreboot.org/c/coreboot/+/44167/2/src/mainboard/acer/m3800/Kc... File src/mainboard/acer/m3800/Kconfig:
https://review.coreboot.org/c/coreboot/+/44167/2/src/mainboard/acer/m3800/Kc... PS2, Line 34: _
this underscore can be a space
I had trouble flashing back and forth when flashrom complained about a mismatch in the mainboard model. It was expecting "ASPIRE_M3800". "Aspire_M3800" works without boardmismatch=force, "Aspire M3800" doesn't.
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/Kc... File src/mainboard/acer/m3800/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/Kc... PS1, Line 2: M3800
I'd use: "Aspire M3800 (G43T-AM3)"
Ah, at one I was so eager to get the "Aspire" part out of the path that I lost track of where it's useful. This looks nice.
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/cm... File src/mainboard/acer/m3800/cmos.layout:
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/cm... PS1, Line 55: 1024 144 r 0 recv_enable_results
Drop this. It's unused now.
Done
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/ds... File src/mainboard/acer/m3800/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/ds... PS1, Line 17: Scope (_SB) { : Device (PCI0) : {
I don't really care. […]
I didn't even notice but now that you mention it it bothers me too. Resolved by applying your initial suggestion.
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/ea... File src/mainboard/acer/m3800/early_init.c:
https://review.coreboot.org/c/coreboot/+/44167/1/src/mainboard/acer/m3800/ea... PS1, Line 9: RCBA32(0x3414) &= ~BUC_LAND; // BUC := Backed Up Control -> LAN is enabled
Is this really needed?
This line is taken from intel/dg43gt. Not disabling LAN seemed useful, but then again this intel board is the only other board calling this. I will test without it.