Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30987 )
Change subject: mb/asus: Add Asus A88XM-E FM2+ with documentation ......................................................................
Patch Set 45:
(6 comments)
https://review.coreboot.org/c/coreboot/+/30987/45//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/30987/45//COMMIT_MSG@29 PS45, Line 29: done incorrect way incorrect
Also, can't it be fixed?
https://review.coreboot.org/c/coreboot/+/30987/38/src/mainboard/asus/a88xm-e... File src/mainboard/asus/a88xm-e/Kconfig:
https://review.coreboot.org/c/coreboot/+/30987/38/src/mainboard/asus/a88xm-e... PS38, Line 39: default BOARD_ASUS_A88XM_E_DDR3_VOLT_150
Exactely, but I couldn't even see anything higher than 1333Mhz on my old F2A85-M.
Then, if this doesn't work, please remove it.
@Mike if your memory is one of the "LP" kits, check their SPD. DDR3-1600 might only be available on the XMP profiles. But I think it's BLT8G3D1869DT1TX0, so I am not sure.
https://review.coreboot.org/c/coreboot/+/30987/42/src/mainboard/asus/a88xm-e... File src/mainboard/asus/a88xm-e/OemCustomize.c:
https://review.coreboot.org/c/coreboot/+/30987/42/src/mainboard/asus/a88xm-e... PS42, Line 163: /* TODO: is this OK for DDR3 socket FM2? */ : /* : MEMCLK_DIS_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00), : CKE_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x05, 0x0A), : ODT_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00), : CS_TRI_MAP(ANY_SOCKET, ANY_CHANNEL, 0x01, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00), : */
Do you think this MEMCLK_DIS_MAP gem (taken from F2A85-M sources) might be a key for a Turtle RAM is […]
I would need to inspect AGESA code to understand this. In any case, you want to make sure the DRAM control signal mapping is correct.
https://review.coreboot.org/c/coreboot/+/30987/45/src/mainboard/asus/a88xm-e... File src/mainboard/asus/a88xm-e/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/30987/45/src/mainboard/asus/a88xm-e... PS45, Line 33: unused But what is it? It is definitely unused if it's set to "off". I would replace with the device names.
https://review.coreboot.org/c/coreboot/+/30987/45/src/mainboard/asus/a88xm-e... PS45, Line 38: disabled Please don't add redundant comments. One knows the device is disabled because it is "off".
Also, such comments risk rotting away: the file gets copied to another board, this device gets enabled but the comment is not updated.
https://review.coreboot.org/c/coreboot/+/30987/45/src/mainboard/asus/a88xm-e... PS45, Line 112: device pci 14.6 off end # unused : device pci 14.7 off end # unused : device pci 15.0 on end # PCIEX1_1 : device pci 15.1 off end # unused : device pci 15.2 on end # Onboard Ethernet : device pci 15.3 off end # unused Which of these are PCIe root ports? You can use comments like this:
# PCIe RP 1: PCIEX1_1 slot # PCIe RP 2: # PCIe RP 3: Onboard Ethernet
Note that comments on unused PCIe ports don't have "unused". Instead, there's nothing, because they are not connected anywhere :)