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 62:
(26 comments)
https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@11 PS60, Line 11: There are (at least) three different mainboards:
The Optiplex 790 comes in four different sizes: MT, DT, SFF and USFF. […]
Done
https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@13 PS60, Line 13: - DT: 4 RAM slots, PEG, PCIe x1, PCI, PCIe x16
Add the SATA port info here: […]
Ack (and USFF has 2x SATA)
https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@18 PS60, Line 18: The variants have different PCI/PCIe configurations and are not
The three mainboard types have different PCI/PCIe configurations, so they need to be different varia […]
Ack
https://review.coreboot.org/c/coreboot/+/21774/60//COMMIT_MSG@21 PS60, Line 21: This port has been tested with: SFF, USFF
need to test DT/MT, I think? I should retest SFF as well
Ack
https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@23 PS62, Line 23: SFF (needs retest) , USFF FIXME
https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@48 PS62, Line 48: DisplayPort? FIXME
https://review.coreboot.org/c/coreboot/+/21774/62//COMMIT_MSG@49 PS62, Line 49: VGA BIOS? FIXME
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/Kconfig.name:
PS59:
nit: split the three form factors out, to have prettier mainboard part numbers
Had to do it anyway to correctly handle PCI/PCIe
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>
See https://github.com/coreboot/coreboot/blob/master/src/mainboard/gigabyte/ga-h.... […]
Corrected the includes.
https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/gpio.c:
https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optiple... PS62, Line 110: .gpio39 = GPIO_MODE_GPIO, /* A_FP_PRES# */
'PRES' may be misspelled - perhaps 'PRESS'?
nope, it means PRESence
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:
This file is only used with the VBIOS anyway. […]
Removed the empty space, VBIOS still needs testing
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/romstage.c:
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 17: : #include <stdint.h> : #include <string.h> : #include <timestamp.h> : #include <arch/byteorder.h> : #include <arch/io.h> : #include <device/mmio.h> : #include <device/pci_ops.h> : #include <device/pnp_ops.h> : #include <cpu/x86/lapic.h> : #include <console/console.h> : #include <northbridge/intel/sandybridge/sandybridge.h> : #include <northbridge/intel/sandybridge/raminit_native.h> : #include <southbridge/intel/bd82x6x/pch.h> : #include <southbridge/intel/common/gpio.h>
include only what you use. […]
Done, I think?
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:
PS59:
use overridetrees
Done
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"
The hotplugability worked fine on vendor firmware.
In any case, the hotpluggability ended up landing on USFF, which does not have PCIe x1. Moved it to MT/DT.
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... PS59, Line 54: 0x4
This is what makes SATA not work on USFF
Enabled all 4 SATA ports that all variants used.
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/hda_verb.c:
PS59:
Fix this mess, as done with other boards.
Done
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 24: 0x0000000b
should be decimal
Done
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 25: /* NID 0x01: Subsystem ID. */
All the "NID" comments are of no use
they went poof
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 26: 0x0
decimal too
Done
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 57: 0x10ec0887, /* Codec Vendor / Device ID: Realtek */
Pack the AZALIA_PIN_CFG macros together, and leave some space between codecs
Done
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 60: 0x0000000f
decimal
it went poof
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 62: 0x2
decimal
it went poof
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 108: 0x00000004
decimal
Done
https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optiple... PS60, Line 110: 0x3
decimal
Done
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/variants/optiplex_790_usff/devicetree.cb:
PS59:
See the other devicetree
it went poof
https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optiple... File src/mainboard/dell/optiplex_790/variants/optiplex_790_usff/hda_verb.c:
PS59:
See the other file. Also, compare for differences.
it went poof