Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG@10
PS1, Line 10: In
: particular, this allows depthcharge to control the state of an
: external USB mux. Some SoCs like Tiger Lake have a USB mux for Type-C
: ports that must be kept in sync with the state of the port as reported
: by the TCPC. This can be achieved by hookink into the poll routine to
: refresh the state of the USB mux.
> This explains the problem to solve. But not why it's done in […]
I think the short answer is that the mux is controlled by the Chrome EC on the board, and the driver to communicate with that is in depthcharge. And I think the reason that driver is in depthcharge originally was that it was based off GPL code that couldn't be added to libpayload.
I agree that the general situation of these driver splits is far from great (also in other cases, like SPI flash and the whole libpayload_init_default_cbfs_media() mess) and it would be much better to have most of it in libpayload. But coreboot kinda forced this situation by insisting that libpayload can only be BSD code. We have CONFIG_LP_GPL now (which didn't exist back then) which could be used to move more GPL drivers into libpayload, but this is all tied up in 10 years worth of glue code and frameworks in depthcharge by now and I don't think anyone has time to migrate the world into another repository. So we are kinda stuck where we are with this.
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 1
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 20:51:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42072 )
Change subject: drivers/usb: add a USB pre-poll hook
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/42072/1//COMMIT_MSG@10
PS1, Line 10: In
: particular, this allows depthcharge to control the state of an
: external USB mux. Some SoCs like Tiger Lake have a USB mux for Type-C
: ports that must be kept in sync with the state of the port as reported
: by the TCPC. This can be achieved by hookink into the poll routine to
: refresh the state of the USB mux.
This explains the problem to solve. But not why it's done in
depthcharge instead of libpayload?
--
To view, visit https://review.coreboot.org/c/coreboot/+/42072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6c23756f64b891b3c5683cd650c605b8630b0fb
Gerrit-Change-Number: 42072
Gerrit-PatchSet: 1
Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Thu, 04 Jun 2020 20:34:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Sugnan Prabhu S has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41624 )
Change subject: drivers/intel/mipi_camera: Support for adding camera power resource
......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41624/4/src/drivers/intel/mipi_cam…
File src/drivers/intel/mipi_camera/camera.c:
https://review.coreboot.org/c/coreboot/+/41624/4/src/drivers/intel/mipi_cam…
PS4, Line 383: if (seq->ops[i].action == ENABLE) {
: method_params[0] = config->gp_panel.gps[index].gp_num;
: acpigen_call_method("STXS", method_params, 1);
: } else if (seq->ops[i].action == DISABLE) {
: method_params[0] = config->gp_panel.gps[index].gp_num;
: acpigen_call_method("CTXS", method_params, 1);
: } else {
> There are already functions that implement this logic on a per-SoC basis. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/41624
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I31e198b50acf2c64035aff9cb054fbe3602dd83e
Gerrit-Change-Number: 41624
Gerrit-PatchSet: 11
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Daniel Kang <daniel.h.kang(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-Comment-Date: Thu, 04 Jun 2020 19:33:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-MessageType: comment
Hello Varshit B Pandya, Daniel Kang, Furquan Shaikh, Rizwan Qureshi, Tim Wawrzynczak, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41620
to look at the new patch set (#10).
Change subject: acpi: Add support for call method and store value
......................................................................
acpi: Add support for call method and store value
This change adds support function for calling methods, store value in a
variable and method to add STA function which returns an external variable.
Change-Id: Ibcb67bd756dc8ae1dfd2d40020c273325583df0f
Signed-off-by: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
---
M src/acpi/acpigen.c
M src/include/acpi/acpigen.h
2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/41620/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/41620
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibcb67bd756dc8ae1dfd2d40020c273325583df0f
Gerrit-Change-Number: 41620
Gerrit-PatchSet: 10
Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com>
Gerrit-Reviewer: Daniel Kang <daniel.h.kang(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com>
Gerrit-MessageType: newpatchset
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 )
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support
......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/i…
File src/drivers/intel/fsp2_0/include/fsp/util.h:
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/i…
PS9, Line 24: enum fsp_multi_phase_action {
: GET_NUMBER_OF_PHASES,
: EXECUTE_PHASE
: };
This is part of an ABI, it would be good to explicitly call out the integer value of each enum.
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/s…
File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/41728/9/src/drivers/intel/fsp2_0/s…
PS9, Line 45: erros
errors
--
To view, visit https://review.coreboot.org/c/coreboot/+/41728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913
Gerrit-Change-Number: 41728
Gerrit-PatchSet: 9
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Thu, 04 Jun 2020 19:28:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41441 )
Change subject: device: Probe fw_config and disable device if it is not found
......................................................................
device: Probe fw_config and disable device if it is not found
This change will set enabled=0 for a device that is not found in the fw_config
if probing is enabled and there are probe entries in devicetree.cb.
The chip enable_dev() and device enable() handlers are still executed in case
those functions need to take action if the device is not enabled.
BUG=b:147462631
TEST=boot with various fw_config values and check the boot log to see that
the expected devices were disabled.
Signed-off-by: Duncan Laurie <dlaurie(a)google.com>
Change-Id: Id3f1c7c600575096f09b23fcd07750cafc65872f
---
M src/device/pci_device.c
M src/device/root_device.c
2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/41441/1
diff --git a/src/device/pci_device.c b/src/device/pci_device.c
index 0584871..9e7a454 100644
--- a/src/device/pci_device.c
+++ b/src/device/pci_device.c
@@ -21,6 +21,7 @@
#include <device/pcix.h>
#include <device/pciexp.h>
#include <device/hypertransport.h>
+#include <fw_config.h>
#include <pc80/i8259.h>
#include <security/vboot/vbnv.h>
#include <timestamp.h>
@@ -1195,6 +1196,12 @@
/* First thing setup the device structure. */
dev = pci_scan_get_dev(bus, devfn);
+ /* Probe fw_config if it is provided for this device. */
+ if (dev && dev->probe_list && !fw_config_probe(dev->probe_list)) {
+ printk(BIOS_DEBUG, "%s disabled by fw_config\n", dev_path(dev));
+ dev->enabled = 0;
+ }
+
/* See if a device is present and setup the device structure. */
dev = pci_probe_dev(dev, bus, devfn);
diff --git a/src/device/root_device.c b/src/device/root_device.c
index 640ea50..6ab5176 100644
--- a/src/device/root_device.c
+++ b/src/device/root_device.c
@@ -3,6 +3,7 @@
#include <console/console.h>
#include <device/device.h>
#include <device/pci.h>
+#include <fw_config.h>
#include <reset.h>
const char mainboard_name[] = CONFIG_MAINBOARD_VENDOR " " CONFIG_MAINBOARD_PART_NUMBER;
@@ -32,6 +33,12 @@
for (link = bus->link_list; link; link = link->next) {
for (child = link->children; child; child = child->sibling) {
+ /* Probe fw_config if it is provided for this device */
+ if (child->probe_list && !fw_config_probe(child->probe_list)) {
+ printk(BIOS_DEBUG, "%s disabled by fw_config\n",
+ dev_path(child));
+ child->enabled = 0;
+ }
if (child->chip_ops && child->chip_ops->enable_dev)
child->chip_ops->enable_dev(child);
@@ -58,6 +65,12 @@
link->secondary = ++bus_max;
for (child = link->children; child; child = child->sibling) {
+ /* Probe fw_config if it is provided for this device */
+ if (child->probe_list && !fw_config_probe(child->probe_list)) {
+ printk(BIOS_DEBUG, "%s disabled by fw_config\n",
+ dev_path(child));
+ child->enabled = 0;
+ }
if (child->chip_ops && child->chip_ops->enable_dev)
child->chip_ops->enable_dev(child);
--
To view, visit https://review.coreboot.org/c/coreboot/+/41441
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id3f1c7c600575096f09b23fcd07750cafc65872f
Gerrit-Change-Number: 41441
Gerrit-PatchSet: 1
Gerrit-Owner: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-MessageType: newchange
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30873 )
Change subject: cpu/intel/car: Use symbols for CAR MTRR setup
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/30873
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I32d7337ccf8005c7fb65d2efea40c122093d4dd9
Gerrit-Change-Number: 30873
Gerrit-PatchSet: 7
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.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: Thu, 04 Jun 2020 18:35:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment