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? -- To view, visit https://review.coreboot.org/c/coreboot/+/39658 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id029074e4231db231a68bc92a4210dc052bba1c9 Gerrit-Change-Number: 39658 Gerrit-PatchSet: 16 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 31 Aug 2020 11:48:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment