Mike Banon 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 25: Code-Review+1
(5 comments)
Now the A88XM-E sources are of the same quality as F2A85-M, which is really good, but you need to explain the source differences (i.e. buildOpts.c and devicetree.cb, see below) , and - if there are known problems which have caused these differences - describe these known problems at your commit message.
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... File src/mainboard/asus/a88xm-e/buildOpts.c:
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... PS25, Line 146: # F2A85-M had it as CpbModeDisabled, but here it's CpbModeAuto. Why?
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... PS25, Line 184: # Why BLDCFG_IOMMU_SUPPORT is disabled? F2A85-M had it enabled
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... PS25, Line 325: # F2A85-M had it as FALSE, but here it's TRUE. It will be nice if you could find all such differences, i.e. " ~/coreboot$ kdiff3 ./src/mainboard/asus/f2a85-m/ ./src/mainboard/asus/a88xm-e/ & " , and explain them to us to make the reviewing easy.
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... File src/mainboard/asus/a88xm-e/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... PS25, Line 29: d Why IOMMU is disabled? F2A85-M had it enabled
https://review.coreboot.org/c/coreboot/+/30987/25/src/mainboard/asus/a88xm-e... PS25, Line 33: end F2A85-M had "device pci 4.0 on end # PCIE 4x black". Please tell why you have removed it, maybe in known problems. Also, these "off" devices from F2A85-M perhaps should be returned to this devicetree.cb to avoid losing knowledge what is at these PCI lane numbers, and maybe enable some of them later if their problem will be resolved.