Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34603 ) Change subject: mb/asus: Add ASUS H110M-E/M.2 mainboard ...................................................................... Patch Set 64: (18 comments) https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/Kconfig: https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... PS56, Line 36: config DEVICETREE
No need to re-define the default value, so this config can be removed. Done
https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/acpi_tables.c: PS50:
This blank file shouldn't be needed anymore Done
https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/devicetree.cb: https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... PS56, Line 203: Enable
done Done
https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/devicetree.cb: https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 204: register "PcieRpClkReqNumber[7]" = "0" This isn't needed if Clock Request is disabled https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/dsdt.asl: https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... PS50, Line 34: // CPU
This comment should be removed, it says nothing useful *poke*
https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... PS50, Line 50: // Chipset specific sleep states
This comment should be removed *poke*
https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/dsdt.asl: https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... PS56, Line 17: */
now it is there Done
https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... PS56, Line 22: /
done Done
https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/dsdt.asl: https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 34: // CPU This comment should go away. https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 50: // Chipset specific sleep states This one as well https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/hda_verb.c: https://review.coreboot.org/c/coreboot/+/34603/56/src/mainboard/asus/h110m-e... PS56, Line 26: 0x0
done Done
https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/hda_verb.c: https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 27: 0x0 Um, these ones should also be updated. All the `AZALIA_PIN_CFG(0x0` should just use `0` https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 52: 0x2 2 https://review.coreboot.org/c/coreboot/+/34603/64/src/mainboard/asus/h110m-e... PS64, Line 55: 0x2 2 https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/romstage.c: PS50:
So it doesn't need to be filled at all, I guess? Ack
https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... PS50, Line 26: {200, 81, 162}
No schematics for either asus nor asrock desktop boards. […] LGA1151 CPUs have the RCOMP resistors on the underside of the CPU, and their values are indeed 121, 75 and 100.
https://review.coreboot.org/c/coreboot/+/34603/50/src/mainboard/asus/h110m-e... PS50, Line 33: 100, 40, 40, 23, 40
sorry, 2x SO-DIMM was 2x per channel. but I guess your board has only 1 per […] Done, I guess?
https://review.coreboot.org/c/coreboot/+/34603/53/src/mainboard/asus/h110m-e... File src/mainboard/asus/h110m-e_m2/romstage.c: https://review.coreboot.org/c/coreboot/+/34603/53/src/mainboard/asus/h110m-e... PS53, Line 54: mem_cfg->MemorySpdPtr00 = (uintptr_t)blk.spd_array[0]; : mem_cfg->MemorySpdPtr10 = (uintptr_t)blk.spd_array[2]; : mem_cfg->MemorySpdPtr01 = (uintptr_t)blk.spd_array[1]; : mem_cfg->MemorySpdPtr11 = (uintptr_t)blk.spd_array[3];
some intel board with two DIMMs does exactly that Done, I guess.
-- To view, visit https://review.coreboot.org/c/coreboot/+/34603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8fa41ecccaa8dba8dc2158ce62d328c7928e05c Gerrit-Change-Number: 34603 Gerrit-PatchSet: 64 Gerrit-Owner: Pavlushka Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Maxim Polyakov <m.poliakov@yahoo.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Pavlushka Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Patrick Rudolph <siro@das-labor.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 26 Feb 2020 22:11:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Comment-In-Reply-To: Pavlushka Gerrit-MessageType: comment