Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39133 )
Change subject: md/kontron/mal10: Add COMe-mAL10 minimal support ......................................................................
Patch Set 8:
(9 comments)
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/acpi_tables.c:
PS8: This shouldn't be needed anymore
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/board_info.txt:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 7: Flashrom support: n Not supported at all?
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 29: /* CPU */ This comment does not add much value. I would remove it
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 32: Scope (_SB) { : Device (PCI0) Can use:
Device (_SB.PCI0)
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 40: /* Chipset specific sleep states */ This comment was removed not too long ago
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/mainboard.c:
PS8: This seems to be dead code.
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/mal10.fmd:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 13: 0xb7d000 If you remove this size, the region will expand as much as possible. It helps to prevent gaps.
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/romstage.c:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 52: mupd->FspmConfig.Ch0_RankEnable = 0; : mupd->FspmConfig.Ch0_DeviceWidth = 0; : mupd->FspmConfig.Ch0_DramDensity = 0; : mupd->FspmConfig.Ch0_Option = 0; : mupd->FspmConfig.Ch1_RankEnable = 0; : mupd->FspmConfig.Ch1_DeviceWidth = 0; : mupd->FspmConfig.Ch1_DramDensity = 0; : mupd->FspmConfig.Ch1_Option = 0; : mupd->FspmConfig.Ch2_RankEnable = 0; : mupd->FspmConfig.Ch2_DeviceWidth = 0; : mupd->FspmConfig.Ch2_DramDensity = 0; : mupd->FspmConfig.Ch2_Option = 0; : mupd->FspmConfig.Ch3_RankEnable = 0; : mupd->FspmConfig.Ch3_DeviceWidth = 0; : mupd->FspmConfig.Ch3_DramDensity = 0; : mupd->FspmConfig.Ch3_Option = 0; Huh?
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 75: IGPU iGPU
-
Angel Pons (Code Review)