Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 )
Change subject: mb/dell: Add Dell Optiplex 790 ......................................................................
Patch Set 61:
(6 comments)
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 1: romstage-y += variants/$(VARIANT_DIR)/gpio.c
Missing some things, for example: […]
Done
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... PS59, Line 36: #include <southbridge/intel/bd82x6x/acpi/sleepstates.asl>
acpi/sleepstates.asl is absent in kontron/ktqm77/dsdt.asl, so I'd simply remove it.
See https://github.com/coreboot/coreboot/blob/master/src/mainboard/gigabyte/ga-h...
https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/gma-mainboard.ads:
https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optiple... PS50, Line 23: : -- For a three-pipe setup, bandwidth is shared between the 2nd and : -- the 3rd pipe. Thus, probe ports that likely have a high-resolution : -- display attached first.
Done
DONE
https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/mainboard.c:
https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optiple... PS50, Line 14:
Unburied into current patchset
DONE
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/mainboard.c:
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... PS59, Line 13:
The variables initialised here only seem to affect laptops.
This file is only used with the VBIOS anyway.
In any case, if the file is kept, please remove the double empty line here.
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... PS59, Line 50: # FIXME: What can be hotplugged? : register "pcie_hotplug_map" = "{ 0, 0, 0, 0, 0, 0, 0, 0 }" : register "pcie_port_coalesce" = "0"
What does removing these entries do? It looks like it's not correct anyway since the PCIe X1 port is […]
They are members of a struct, so their default value is zero.
Also, what do you mean with "confirmed hotplug-able" ?