Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39658 )
Change subject: mb/asrock/q1900m: Add new mainboard ......................................................................
Patch Set 16:
(6 comments)
Looks pretty good. LGTM?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... File src/mainboard/asrock/q1900m/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 108: 4e.0 I might be wrong about this... This is usually set to 0c31.0 which IIRC is just a HID in ACPI an not an actual IO port because all IO happens via memory mapped operation at CONFIG_TPM_TIS_BASE_ADDRESS. So my question is if there is really something responding at IO 4e?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... File src/mainboard/asrock/q1900m/irqroute.h:
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 10: PCI_DEV_PIRQ_ROUTE(SD_DEV, C, D, E, F), \ This is off in DT?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 13: PCI_DEV_PIRQ_ROUTE(LPE_DEV, A, B, C, D), \ : PCI_DEV_PIRQ_ROUTE(MMC_DEV, D, E, F, G), \ : PCI_DEV_PIRQ_ROUTE(SIO1_DEV, A, B, C, D), \ : PCI_DEV_PIRQ_ROUTE(TXE_DEV, A, B, C, D), \ off in DT?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 20: PCI_DEV_PIRQ_ROUTE(SIO2_DEV, B, C, D, E), \ off in DT?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... File src/mainboard/asrock/q1900m/romstage.c:
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 26: 0x03 Are there defines somewhere for this?
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 27: spd[1] spd[1] ends up unused?