Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39658 ) Change subject: mb/asrock/q1900m: Add new mainboard ...................................................................... Patch Set 17: (7 comments) 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. […] Probably not, definitely not on TPM 2.0 stuff. I haven't tested this, but Asrock B85M Pro4 works just fine (even better than on vendor BIOS) with the same code and a TPM 2.0.
I could try using `device mmio 0xfed40000` instead (the actual device address). https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... File src/mainboard/asrock/q1900m/fadt.c: https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 7: acpi_create_fadt
Please, why don't you use the one we already have here: src/acpi/acpi. […] Why does this even build in the first place? Wait, did anyone select HAVE_ACPI_TABLES somewhere?
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? Sure. I haven't checked these at all, so they are completely wrong.
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? I want to enable TXE, but it doesn't respond and mei_txe shits a brick. The rest shall go away.
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 8: static uint8_t spd[2][256]; #define SPD_SIZE 256 static uint8_t spd[NUM_CHANNELS][SPD_SIZE]; https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 26: 0x03
Are there defines somewhere for this? probably in src/include/device/dram/ddr3.h
https://review.coreboot.org/c/coreboot/+/39658/16/src/mainboard/asrock/q1900... PS16, Line 27: spd[1]
spd[1] ends up unused? No, MRC will access this data even though the two dram_data pointers are the same. See the comment on line 32. It accesses the contents of spd[0] through dram_data[0] and the contents of spd[1] through dram_data[1].
I can change `spd[0]` to `spd` on lines 33 and 34, though. -- 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: 17 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: Wed, 02 Sep 2020 16:47:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-MessageType: comment