Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37871 )
Change subject: soc/intel/common/block: Enable PMC IPC driver
......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37871/16/src/soc/intel/common/bloc…
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/37871/16/src/soc/intel/common/bloc…
PS16, Line 46: #define PMC_IPC_USBC_CMD_ID 0xA7
> unused?
Used in early TCSS
https://review.coreboot.org/c/coreboot/+/37871/16/src/soc/intel/common/bloc…
PS16, Line 610: check_ips_sts
> nit. Typo? check_ipc_sts. Also should it be pmc_check_ipc_sts for consistency.
this is what it was on the internal patch so I kept it...but it should be ipc you are right I'll update
--
To view, visit https://review.coreboot.org/c/coreboot/+/37871
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd3ed262fc700ccc891ec68997a108f5bfbaf9ed
Gerrit-Change-Number: 37871
Gerrit-PatchSet: 16
Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com>
Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: Vijay P Hiremath <vijay.p.hiremath(a)intel.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
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-Comment-Date: Fri, 07 Feb 2020 19:24:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Divya Sasidharan <divya.s.sasidharan(a)intel.corp-partner.google.com>
Gerrit-MessageType: comment
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21774 )
Change subject: mb/dell: Add Dell Optiplex 790
......................................................................
Patch Set 63:
(3 comments)
https://review.coreboot.org/c/coreboot/+/21774/63/Documentation/mainboard/d…
File Documentation/mainboard/dell/optiplex_790.md:
https://review.coreboot.org/c/coreboot/+/21774/63/Documentation/mainboard/d…
PS63, Line 5: ## TODO
remove todo or add possibly missing info?
https://review.coreboot.org/c/coreboot/+/21774/63/Documentation/mainboard/d…
File Documentation/mainboard/dell/servicemode.md:
https://review.coreboot.org/c/coreboot/+/21774/63/Documentation/mainboard/d…
PS63, Line 7:
is the jumper connected to the Flash Descriptor Override Pin Strap?
https://review.coreboot.org/c/coreboot/+/21774/63/Documentation/mainboard/d…
PS63, Line 8: #TODO: Add a picture of the service mode jumper inside an OptiPlex 790
i'd rather add the picture to the optiplex 790 page, since the picture is specific to that device
--
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: 63
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: Fri, 07 Feb 2020 18:24:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Paul Menzel 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 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37867/14//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/37867/14//COMMIT_MSG@9
PS14, Line 9: EC being the TCPM decides the mux configuration
: after negotiating with the port partner on the
: Type-C port. The APIS added here will give the
: current essential mux state information for a
: given port.
Please reflow for 75 characters per line.
https://review.coreboot.org/c/coreboot/+/37867/14//COMMIT_MSG@17
PS14, Line 17: USB-TypeC flash drive
Please give the model.
https://review.coreboot.org/c/coreboot/+/37867/14//COMMIT_MSG@18
PS14, Line 18: on TGLRVP
Ditto.
--
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: 14
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: Fri, 07 Feb 2020 12:28:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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 63:
(1 comment)
https://review.coreboot.org/c/coreboot/+/21774/63/src/mainboard/dell/optipl…
File src/mainboard/dell/optiplex_790/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/21774/63/src/mainboard/dell/optipl…
PS63, Line 23: register "c1_battery" = "1"
Do we actually need these "battery" states?
--
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: 63
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: Fri, 07 Feb 2020 09:20:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22214 )
Change subject: nb/intel/sandybridge/raminit: Add ECC detection support
......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
File src/northbridge/intel/sandybridge/raminit.c:
https://review.coreboot.org/c/coreboot/+/22214/9/src/northbridge/intel/sand…
PS9, Line 407: ctrl.ecc_forced ? "yes" : "no");
> > I was wondering why we don't have to read SPDs on this path (see next
> > line)... the only way to get here is to run through `if (!fastboot)` above,
> > so we gathered/printed this information already. Code flow is a mess :-/
> > it seems somebody tried to save indentation levels.
>
> The argument about code structure and necessity of (re-?)reading the SPD is in my opinion outside the scope of this change.
Sure. I just wanted to mention how I came to the conclusion.
> > Also, this seems informational, BIOS_INFO?
>
> Almost all the other printks in this file are BIOS_DEBUG. I don't think this one is more significant than those.
Yep, a common issue: We have 9 log levels, but act as if there were only 3.
IMHO, `ecc_forced = 1` would even deserve BIOS_NOTICE.
--
To view, visit https://review.coreboot.org/c/coreboot/+/22214
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5b7599746195cfa996a48320404a8dbe6820483a
Gerrit-Change-Number: 22214
Gerrit-PatchSet: 9
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Jonathan Kollasch <jakllsch(a)kollasch.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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-Comment-Date: Fri, 07 Feb 2020 09:00:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jonathan Kollasch <jakllsch(a)kollasch.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38475 )
Change subject: mainboard/hatch: Fix puff display on cold boots (WIP)
......................................................................
mainboard/hatch: Fix puff display on cold boots (WIP)
BUG=b:XXX
BRANCH=none
TEST=none
Change-Id: I19d40056e58f1737f87fd07d62b07a723a63d610
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M src/mainboard/google/hatch/variants/puff/mainboard.c
1 file changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/38475/1
diff --git a/src/mainboard/google/hatch/variants/puff/mainboard.c b/src/mainboard/google/hatch/variants/puff/mainboard.c
index 9c2b5fb..cc1dd39 100644
--- a/src/mainboard/google/hatch/variants/puff/mainboard.c
+++ b/src/mainboard/google/hatch/variants/puff/mainboard.c
@@ -15,9 +15,48 @@
#include <baseboard/variants.h>
#include <chip.h>
+#include <gpio.h>
#include <device/device.h>
#include <ec/google/chromeec/ec.h>
+#define GPIO_HDMI_HPD GPP_E13
+#define GPIO_DP_HPD GPP_E14
+
+/* TODO: This can be moved to common directory */
+static void wait_for_hpd(gpio_t gpio, long timeout)
+{
+ struct stopwatch sw;
+
+ printk(BIOS_INFO, "Waiting for HPD\n");
+ gpio_input(gpio);
+
+ stopwatch_init_msecs_expire(&sw, timeout);
+ while (!gpio_get(gpio)) {
+ if (stopwatch_expired(&sw)) {
+ printk(BIOS_WARNING,
+ "HPD not ready after %ldms. Abort.\n", timeout);
+ return;
+ }
+ mdelay(200);
+ }
+ printk(BIOS_INFO, "HPD ready after %lu ms\n",
+ stopwatch_duration_msecs(&sw));
+}
+
+void variant_ramstage_init(void)
+{
+ static const long display_timeout_ms = 3000;
+
+ /* This is reconfigured back to whatever FSP-S expects by
+ gpio_configure_pads. */
+ gpio_input(GPIO_HDMI_HPD);
+ if (display_init_required() && !gpio_get(GPIO_HDMI_HPD)) {
+ /* This has to be done before FSP-S runs. */
+ if (google_chromeec_wait_for_displayport(display_timeout_ms))
+ wait_for_hpd(GPIO_DP_HPD, display_timeout_ms);
+ }
+}
+
/*
* For type-C chargers, set PL2 to 90% of max power to account for
* cable loss and FET Rdson loss in the path from the source.
--
To view, visit https://review.coreboot.org/c/coreboot/+/38475
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I19d40056e58f1737f87fd07d62b07a723a63d610
Gerrit-Change-Number: 38475
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange