Attention is currently required from: Angel Pons, Arthur Heymans, Benjamin Doron, Christian Walter, Lean Sheng Tan, Maximilian Brune, Naresh Solanki.
David Milosevic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79086?usp=email )
Change subject: mainboard/emulation/qemu-sbsa: Add qemu-sbsa board ......................................................................
Patch Set 5:
(15 comments)
File src/mainboard/emulation/qemu-sbsa/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/79086/comment/5e01df8a_e3f81f16 : PS1, Line 419: Device (RES0) : { : Name (_HID, "PNP0C02" /* PNP Motherboard Resources */) // _HID: Hardware ID : Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings : { : QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, : 0x0000000000000000, // Granularity : SBSA_PCIE_ECAM_BASE, // Range Minimum : SBSA_PCIE_ECAM_LIMIT, // Range Maximum : 0x0000000000000000, // Translation Offset : SBSA_PCIE_ECAM_SIZE, // Length : ,, , AddressRangeMemory, TypeStatic) : }) : Method (_STA) { : Return (0xF) : } : }
Drop this. It's in dsdt_top.asl now.
Done
File src/mainboard/emulation/qemu-sbsa/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/79086/comment/e8832792_ab1c3222 : PS3, Line 10: Method (_STA) { \ : Return (0xF) \ : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/ef157a5d_cb8e9e4a : PS3, Line 15: Method (_DIS) { } \
If _DIS does nothing, should it even exist?
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/6d2d0177_50902241 : PS3, Line 21: }
nit: closing brace is indented a bit too much
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/06832e8d_0d5bf313 : PS3, Line 31: // OEM revision
Most instances of this comment are copy-pasta from other boards. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/a8f2799d_34982e7b : PS3, Line 45: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/c4dc3a35_1a2345f1 : PS3, Line 63: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/49cac825_47aedd3c : PS3, Line 70: Name (_HID, "LNRO0D20")
Indentation went up an extra level here and in the following blocks
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/38a5f462_1f3f745a : PS3, Line 72: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/6ecaa002_00493f57 : PS3, Line 86: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/3848b792_21ef0b00 : PS3, Line 117: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/d631c4dc_99387424 : PS3, Line 190: Method (_STA) { : Return (0xF) : }
_STA that always returns 0xf can be removed: https://uefi. […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/438ff087_a79df4a6 : PS3, Line 205: PRT_ENTRY(0x0000FFFF, 0, GSI0), : PRT_ENTRY(0x0000FFFF, 0, GSI1), : PRT_ENTRY(0x0000FFFF, 0, GSI2), : PRT_ENTRY(0x0000FFFF, 0, GSI3),
Pin is always 0 for these 4 entries, is this correct? It seems odd.
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/759be827_155649eb : PS3, Line 210: PRT_ENTRY(0x0001FFFF, 0, GSI1), : PRT_ENTRY(0x0001FFFF, 1, GSI2), : PRT_ENTRY(0x0001FFFF, 2, GSI3), : PRT_ENTRY(0x0001FFFF, 3, GSI0),
Idea: For the sake of brevity, you could define another helper function: […]
Done
https://review.coreboot.org/c/coreboot/+/79086/comment/67860bfb_cd254ba4 : PS3, Line 453: Store (CDW2,SUPP) : Store (CDW3,CTRL)
Should be equivalent: […]
Done