coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1797 discussions

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by build bot (Jenkins) (Code Review) 05 Feb '20

05 Feb '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... Patch Set 62: (1 comment) https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/gpio.c: https://review.coreboot.org/c/coreboot/+/21774/62/src/mainboard/dell/optipl… PS62, Line 110: .gpio39 = GPIO_MODE_GPIO, /* A_FP_PRES# */ 'PRES' may be misspelled - perhaps 'PRESS'? -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 62 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 19:03:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Angel Pons (Code Review) 05 Feb '20

05 Feb '20

Angel Pons has uploaded a new patch set (#62) to the change originally created by Christoph Pomaska. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... mb/dell: Add Dell Optiplex 790 This port was generated using autoport. The Optiplex 790 comes in four different sizes: MT, DT, SFF and USFF. The mainboard PCBs for MT and DT are identical, the only difference is that MT has an extra SATA port. The other PCBs are slightly different, but they are designed with intercompatibility in mind. Therefore, the four mainboard configurations are: - MT: 4 RAM slots, 4x SATA, PEG, PCIe x1, PCI, PCIe x16 - DT: 4 RAM slots, 3x SATA, PEG, PCIe x1, PCI, PCIe x16 - SFF: 4 RAM slots, 3x SATA, PEG, PCIe x16 - USFF: 2 RAM slots, 2x SATA, mPCIe This port has been tested with: SFF (needs retest) , USFF What works: - Booting Arch Linux with kernel 5.2.3-arch1-1-ARCH - PCIe (Hotplug on PCIe X1 only!) - Onboard graphics with libgfxinit (via VGA) - Hard drive activity LED - Onboard sound - Onboard Ethernet - SATA (hotplug) - IOMMU - Turning on and off (S5) - Suspend/resume (S3) - EHCI debug (rear side, bottom port on the block with ethernet) What does not work: - SuperI/O (Chip is a SCH5544-NS) - Serial port - PS/2 - Fan control (fans run at full speed) - Front LEDs (signals code 234, corrupted or defective BIOS) [1] - VBT is missing Untested: - DisplayPort? - VGA BIOS? Further notes: - Default IFD settings block reads/writes to some regions. This can be bypassed by plugging the SERVICE_MODE jumper. BIOS version A05 does not set any protected ranges, so internal flashing is possible (use a layout if the SERVICE_MODE jumper is not plugged). - The mainboard has two flash chips: SOIC16 8 MiB and SOIC8 2 MiB. They are recognized as one "Opaque flash chip" of 10 MiB in size. [1] http://www.dell.com/support/article/us/en/04/sln284978/a-reference-guide-to… Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Signed-off-by: Christoph Pomaska <github(a)aufmachen.jetzt> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A Documentation/mainboard/dell/optiplex790.md A Documentation/mainboard/dell/servicemode.md M Documentation/mainboard/index.md A src/mainboard/dell/Kconfig A src/mainboard/dell/Kconfig.name A src/mainboard/dell/optiplex_790/Kconfig A src/mainboard/dell/optiplex_790/Kconfig.name A src/mainboard/dell/optiplex_790/Makefile.inc A src/mainboard/dell/optiplex_790/acpi/ec.asl A src/mainboard/dell/optiplex_790/acpi/platform.asl A src/mainboard/dell/optiplex_790/acpi/superio.asl A src/mainboard/dell/optiplex_790/acpi_tables.c A src/mainboard/dell/optiplex_790/board_info.txt A src/mainboard/dell/optiplex_790/devicetree.cb A src/mainboard/dell/optiplex_790/dsdt.asl A src/mainboard/dell/optiplex_790/early_init.c A src/mainboard/dell/optiplex_790/gma-mainboard.ads A src/mainboard/dell/optiplex_790/gpio.c A src/mainboard/dell/optiplex_790/hda_verb.c A src/mainboard/dell/optiplex_790/mainboard.c A src/mainboard/dell/optiplex_790/variants/mt_dt/overridetree.cb A src/mainboard/dell/optiplex_790/variants/sff/overridetree.cb A src/mainboard/dell/optiplex_790/variants/usff/overridetree.cb 23 files changed, 821 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/21774/62 -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 62 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/cse: Add boot partition related APIs
by Sridhar Siricilla (Code Review) 05 Feb '20

05 Feb '20

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35402 ) Change subject: soc/intel/common/block/cse: Add boot partition related APIs ...................................................................... Patch Set 60: (23 comments) https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse_bp.c: https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 21: #define MKHI_GROUP_ID_BUP_COMMON 0xf0 > Can we keep all the MKHI_GROUP_ID* in one place? Maybe in cse. […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 23: /* Boot partition info and set boot partition info command ids */ : #define MKHI_GET_BOOT_PARTITION_INFO_CMD_REQ 0x1C : #define MKHI_SET_BOOT_PARTITION_CMD_REQ 0x1D > Same as MKHI_GROUP_ID*. I think it would be good to have all of these in a single place. […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 37: boot_partition_status > This hasn't been addressed yet? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 40: bp_ > What do you think about skipping prefix bp_ here since this is embedded within boot_partition_info_e […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 64: static struct cse_boot_partition_info cse_bp_info; > Ack Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 81: memset(&info_req, 0, sizeof(info_req)); > Is this really required? If you initialize the structure while declaring it above, then memset might […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 109: Boot partition update failed > This is very confusing. At first, it seems like update to boot partition failed. […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 116: cse_send_set_boot_partition_info_cmd > Not addressed? The command name is SET_BOOT_PARTITION_INFO. So, I will rename function as cse_set_bp_info() https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 132: " > %d\n", partition_id); […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 133: goto failed; > Just return false; here directly. There is nothing happening under "failed:" label. Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 136: Info > I don't think adding "Info" here is correct. It's correct as command name is SET BOOT PARTITION INFO, https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 137: switch_req.next_bp = partition_id; > Not addressed? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 142: memset(&switch_resp, 0, sw_resp_sz); > Why is this required? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 144: goto failed; > return false; Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 149: goto failed; > return false; Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 164: * > Not addressed? Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 212: cse_bp_info.bp_entries[boot_partition].bp_status; > Before doing this shouldn't you also check if cse_bp_info. […] Done https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 254: if ((bp_info_flags & BP_INFO_MIN_RECOV_MODE_EN) != BP_INFO_MIN_RECOV_MODE_EN) { : printk(BIOS_INFO, "Minimum Recovery Mode is not enabled\n"); : *min_rec_mode_en = 0; : } else { : printk(BIOS_INFO, "Minimum Recovery Mode is enabled\n"); : *min_rec_mode_en = 1; : } : : return true; > Not addressed? Suggested code looks good but it does not consider error scenario! https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 295: bp2_status | bp3_status > So, if BP2 is valid and BP3 is invalid, me_rw_status would be both valid and invalid? If any boot partition is not valid, then it indicates RW partion is invalid. https://review.coreboot.org/c/coreboot/+/35402/44/src/soc/intel/common/bloc… PS44, Line 312: 2 > This should be cse_bp_info. […] Done https://review.coreboot.org/c/coreboot/+/35402/55/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse_bp.c: https://review.coreboot.org/c/coreboot/+/35402/55/src/soc/intel/common/bloc… PS55, Line 101: if (!cse_update_boot_partition_info()) { > braces {} are not necessary for single statement blocks Done https://review.coreboot.org/c/coreboot/+/35402/59/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse_bp.c: https://review.coreboot.org/c/coreboot/+/35402/59/src/soc/intel/common/bloc… PS59, Line 31: boot_partition_status > Please see comment on patchset 44. Done https://review.coreboot.org/c/coreboot/+/35402/59/src/soc/intel/common/bloc… PS59, Line 65: ; > Since the memset is removed now, you would need the initialization here: […] Thanks..I missed it .. -- To view, visit https://review.coreboot.org/c/coreboot/+/35402 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaa62409c0616d5913d21374a8a6804f82258eb4f Gerrit-Change-Number: 35402 Gerrit-PatchSet: 60 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Petrov <anpetrov(a)fb.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 05 Feb 2020 17:36:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86/include/arch: Add ROM_TASK_STATE_SEG
by Name of user not set (Code Review) 05 Feb '20

05 Feb '20

cedarhouse1(a)comcast.net has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38657 ) Change subject: arch/x86/include/arch: Add ROM_TASK_STATE_SEG ...................................................................... arch/x86/include/arch: Add ROM_TASK_STATE_SEG This define is used to set up the STM SMM Descriptor table tr entry. Signed-off-by: Eugene D. Myers <edmyers(a)tycho.nsa.gov> Change-Id: Iddb1f45444d03465a66a4ebb9fde5f206dc5b300 --- M src/arch/x86/include/arch/rom_segs.h 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/38657/1 diff --git a/src/arch/x86/include/arch/rom_segs.h b/src/arch/x86/include/arch/rom_segs.h index a19d3de..05cef4d 100644 --- a/src/arch/x86/include/arch/rom_segs.h +++ b/src/arch/x86/include/arch/rom_segs.h @@ -17,5 +17,6 @@ #define ROM_CODE_SEG 0x08 #define ROM_DATA_SEG 0x10 #define ROM_CODE_SEG64 0x18 +#define ROM_TASK_STATE_SEG 0x20 #endif /* ROM_SEGS_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/38657 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iddb1f45444d03465a66a4ebb9fde5f206dc5b300 Gerrit-Change-Number: 38657 Gerrit-PatchSet: 1 Gerrit-Owner: cedarhouse1(a)comcast.net Gerrit-MessageType: newchange

6 11

Change in coreboot[master]: src/ec/google/chromeec: Get Type-C Mux info from EC (TCPM)
by Keith Short (Code Review) 05 Feb '20

05 Feb '20

Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37867 ) Change subject: src/ec/google/chromeec: Get Type-C Mux info from EC (TCPM) ...................................................................... Patch Set 11: (1 comment) https://review.coreboot.org/c/coreboot/+/37867/11/src/ec/google/chromeec/ec… File src/ec/google/chromeec/ec_commands.h: https://review.coreboot.org/c/coreboot/+/37867/11/src/ec/google/chromeec/ec… PS11, Line 5253: uint8_t control_flags; /* USB_PD_CTRL_*flags */ The comment refers to USB_PD_CTRL* flags, but they are still called USB_PD_MUX* above. Prefer you switch the defines above to USB_PD_CTRL to match the file stored in the EC code. -- To view, visit https://review.coreboot.org/c/coreboot/+/37867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I96bfb6036f4340ba42a078cfd7ecaae777a3ed00 Gerrit-Change-Number: 37867 Gerrit-PatchSet: 11 Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Divya S Sasidharan <divya.s.sasidharan(a)intel.com> Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: caveh jalali <caveh(a)chromium.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com> Gerrit-Comment-Date: Wed, 05 Feb 2020 17:23:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Christoph Pomaska (Code Review) 05 Feb '20

05 Feb '20

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Christoph Pomaska (Code Review) 05 Feb '20

05 Feb '20

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Angel Pons (Code Review) 05 Feb '20

05 Feb '20

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/optipl… File src/mainboard/dell/optiplex_790/Makefile.inc: https://review.coreboot.org/c/coreboot/+/21774/60/src/mainboard/dell/optipl… 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/optipl… File src/mainboard/dell/optiplex_790/dsdt.asl: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… 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-… https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/gma-mainboard.ads: https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optipl… 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/optipl… File src/mainboard/dell/optiplex_790/mainboard.c: https://review.coreboot.org/c/coreboot/+/21774/50/src/mainboard/dell/optipl… PS50, Line 14: > Unburied into current patchset DONE https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/mainboard.c: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… 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/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/devicetree.cb: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… 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" ? -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 61 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 16:50:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Christoph Pomaska <github(a)slrie.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Christoph Pomaska (Code Review) 05 Feb '20

05 Feb '20

Christoph Pomaska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 ) Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... Patch Set 61: (14 comments) https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/Kconfig: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 22: optiplex_790_usff > Rename the variants so that they are just the form factor. […] Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 34: config VGA_BIOS_FILE : string : default "pci8086,0102.rom" : : config VGA_BIOS_ID : string : default "8086,0102" > it's pointless, the board has a socket. Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 42: config MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID : hex : default 0x4ad : : config MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID : hex : default 0x1028 > delet Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/Makefile.inc: PS59: > Fix this for C_ENV_BB (s/romstage.c/early_init. […] Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/acpi_tables.c: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 22: /* Disable USB ports in S3 by default */ : gnvs->s3u0 = 0; : gnvs->s3u1 = 0; : : /* Disable USB ports in S5 by default */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0; > Drop Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/dsdt.asl: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 16: #define ACPI_VIDEO_DEVICE \_SB.PCI0.GFX0 > Is unused Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 23: // DSDT revision: ACPI 2.0 and up > Use C comments for consistency. Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 36: #include <southbridge/intel/bd82x6x/acpi/sleepstates.asl> > Some of these headers need to be corrected! acpi/sleepstates.asl is absent in kontron/ktqm77/dsdt.asl, so I'd simply remove it. https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/mainboard.c: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 13: > Double empty line. Decide if this file is worth keeping. The variables initialised here only seem to affect laptops. https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… File src/mainboard/dell/optiplex_790/variants/optiplex_790_mt-sff/devicetree.cb: https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 17: register "gfx.did" = "{ 0x80000100, 0x80000240, 0x80000410 }" : register "gfx.link_frequency_270_mhz" = "0" : register "gfx.ndid" = "3" > take out the trash Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 23: 0x0 > 0 Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… 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" > Remove What does removing these entries do? It looks like it's not correct anyway since the PCIe X1 port is confirmed hotplug-able. https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 62: on # > align Done https://review.coreboot.org/c/coreboot/+/21774/59/src/mainboard/dell/optipl… PS59, Line 78: d > MT/DT probably want this on Done -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 61 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 05 Feb 2020 16:37:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/dell: Add Dell Optiplex 790
by Christoph Pomaska (Code Review) 05 Feb '20

05 Feb '20

Hello Kyösti Mälkki, Patrick Rudolph, Felix Held, Angel Pons, Mimoja, Arthur Heymans, Paul Menzel, Jonathan Neuschäfer, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/21774 to look at the new patch set (#61). Change subject: mb/dell: Add Dell Optiplex 790 ...................................................................... mb/dell: Add Dell Optiplex 790 This port was generated by autoport and has been tweaked (see below) There are (at least) three different mainboards: - DT: 4 RAM slots, PEG, PCIe x1, PCI, PCIe x16 - MT: 4 RAM slots, PEG, PCIe x1, PCI, PCIe x16 - SFF: 4 RAM slots, PEG, PCIe x16 - USFF: 2 RAM slots, mPCIe The variants have different PCI/PCIe configurations and are not exactly compatible towards each other. This port has been tested with: SFF, USFF What works: - Booting Arch Linux with kernel 5.2.3-arch1-1-ARCH - PCIe (Hotplug on PCIe X1 only!) - Onboard graphics with libgfxinit (via VGA) - Turning on and off (S5) - Harddrive activity LED - Onboard sound - Onboard ethernet - SATA (hotplug) - IOMMU - Suspend/resume (S3) - EHCI debug (rear side, bottom port on the block with ethernet) What does not work: - SuperI/O (Chip is a SCH5544-NS) - Serial port - PS/2 - Fan control (fans go to full speed) - VBT is missing Further notes: - Default IFD settings block reads/writes to some regions. This can be bypassed by plugging the SERVICE_MODE jumper. BIOS version A05 does not set any protected ranges, so internal flashing is possible (use a layout if the SERVICE_MODE jumper is not plugged). - Setting the jumper slows down the boot process of coreboot significantly, as coreboot waits 900ms for the ME to report an OK DRAM (which doesn't happen with the jumper set) - The controller that controls the POST code LEDs on the front of the case (likely the SuperIO) stays on slow blinking POST 234, corrupt or defect BIOS according to [1]. - The mainboard has one SOIC16 8192KiB and one SOIC8 2048KiB BIOS chip that are recognized as one "Opaque flash chip" of 10240K in size. [1] http://www.dell.com/support/article/us/en/04/sln284978/a-reference-guide-to… Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Signed-off-by: Christoph Pomaska <github(a)aufmachen.jetzt> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A Documentation/mainboard/dell/optiplex790.md A Documentation/mainboard/dell/servicemode.md M Documentation/mainboard/index.md A src/mainboard/dell/Kconfig A src/mainboard/dell/Kconfig.name A src/mainboard/dell/optiplex_790/Kconfig A src/mainboard/dell/optiplex_790/Kconfig.name A src/mainboard/dell/optiplex_790/Makefile.inc A src/mainboard/dell/optiplex_790/acpi/ec.asl A src/mainboard/dell/optiplex_790/acpi/platform.asl A src/mainboard/dell/optiplex_790/acpi/superio.asl A src/mainboard/dell/optiplex_790/acpi_tables.c A src/mainboard/dell/optiplex_790/board_info.txt A src/mainboard/dell/optiplex_790/dsdt.asl A src/mainboard/dell/optiplex_790/early_init.c A src/mainboard/dell/optiplex_790/gma-mainboard.ads A src/mainboard/dell/optiplex_790/hda_verb.c A src/mainboard/dell/optiplex_790/mainboard.c A src/mainboard/dell/optiplex_790/variants/mt-sff/devicetree.cb A src/mainboard/dell/optiplex_790/variants/mt-sff/gpio.c A src/mainboard/dell/optiplex_790/variants/mt-sff/hda_verb.c A src/mainboard/dell/optiplex_790/variants/usff/devicetree.cb A src/mainboard/dell/optiplex_790/variants/usff/gpio.c A src/mainboard/dell/optiplex_790/variants/usff/hda_verb.c 24 files changed, 1,193 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/21774/61 -- To view, visit https://review.coreboot.org/c/coreboot/+/21774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If3d3a13163d5da1368259a7498019d42fb3ed57f Gerrit-Change-Number: 21774 Gerrit-PatchSet: 61 Gerrit-Owner: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christoph Pomaska <github(a)slrie.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020