Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39133 )
Change subject: mb/kontron/mal10: Add COMe-mAL10 minimal support ......................................................................
Patch Set 9:
(10 comments)
Thanks for the review
https://review.coreboot.org/c/coreboot/+/39133/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39133/8//COMMIT_MSG@7 PS8, Line 7: md
mb?
Done
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
Removed
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?
Sorry, I didn't notice flash chip "W25Q128" (16384 kB, SPI) Yes it is supported fixed
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. […]
Done
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 32: Scope (_SB) { : Device (PCI0)
Can use: […]
Done
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
Done
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... File src/mainboard/kontron/mal10/gpio.h:
https://review.coreboot.org/c/coreboot/+/39133/8/src/mainboard/kontron/mal10... PS8, Line 161: #if 0 : PAD_CFG_NF_IOSSTATE(GPIO_196, NATIVE, DEEP, NF1, MASK), /* PNL1_VDDEN */ : PAD_CFG_NF_IOSSTATE(GPIO_197, NATIVE, DEEP, NF1, MASK), /* PNL1_BKLTEN */ : PAD_CFG_NF_IOSSTATE(GPIO_198, NATIVE, DEEP, NF1, MASK), /* PNL1_BKLTCTL */
why dead code?
Sorry, I forgot to delete this lvds is currently disabled for this board There is no device to test this interface
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.
Yes you are right. And since I don't use VGA_ROM_RUN, call install_intel_vga_int15_handler isn't needed. https://github.com/coreboot/coreboot/blob/ef7a3267870f126cc2f815812cfe545008... Removed
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.
Done, thanks
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
Done