coreboot-gerrit February 2020

coreboot-gerrit@coreboot.org

1 participants
1797 discussions

Change in coreboot[master]: soc/intel/common/block: Enable PMC IPC driver
by Brandon Breitenstein (Code Review) 03 Feb '20

03 Feb '20

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 10: (2 comments) https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/bloc… File src/soc/intel/common/block/pmc/pmclib.c: https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/bloc… PS10, Line 610: if (ipcsts & PMC_IPC_STS_BUSY) : return CB_SUCCESS; > I am confused. […] I think somewhere is the moving this patch around a few times this got messed up. You are correct it should be a check of the busy bit until timeout not a return when the busy bit is hit. Will fix this. https://review.coreboot.org/c/coreboot/+/37871/10/src/soc/intel/common/bloc… PS10, Line 614: > Does there need to be a delay instead of a tight loop for checking the status register? There is no delay required but I will add a 1us delay here to avoid the tight loop hammering the register -- 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: 10 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: 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: Mon, 03 Feb 2020 22:56:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Add code for early tcss
by Furquan Shaikh (Code Review) 03 Feb '20

03 Feb '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37870 ) Change subject: soc/intel/tigerlake: Add code for early tcss ...................................................................... Patch Set 10: (3 comments) https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@10 PS10, Line 10: PMC IPC driver is needed to communicate with EC in order to : get status of the Type C ports > Kernel is re-scanning the ports once it is loaded. […] The example I mentioned is actually pre-kernel. It is still firmware. W.r.t. EC flag - is that captured in some bug? https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… File src/soc/intel/tigerlake/early_tcss.c: https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 81: google_chromeec_usb_get_pd_ports > Potentially doesn't need to be tied to Chrome EC, however with Intel EC all control of TCSS is handl […] Humm.. I understand that this is something which is very Chrome OS/Chrome EC specific. I think its okay if you want to continue with google_chromeec* calls for now. In the future, if there are more uses identified, we can potentially convert it to an abstraction layer. https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 116: BS_PAYLOAD_LOAD > This was done with the assumption that external display was not supported for bios. […] Yes, early display would be required especially on form factors which depend primarily on external display. -- To view, visit https://review.coreboot.org/c/coreboot/+/37870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e Gerrit-Change-Number: 37870 Gerrit-PatchSet: 10 Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> 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: Subrata Banik <subrata.banik(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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com> Gerrit-Comment-Date: Mon, 03 Feb 2020 22:22:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: usb: add a new function soc_get_usb_ctlr
by Tim Wawrzynczak (Code Review) 03 Feb '20

03 Feb '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38683 ) Change subject: usb: add a new function soc_get_usb_ctlr ...................................................................... usb: add a new function soc_get_usb_ctlr The new function should be provided by SoCs or mainboards, and it simply returns the 'struct device *' representing the USB root controller. Change-Id: I7dd2569dd615722a69f2dfd75622362e56a424ea Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/device/Makefile.inc A src/device/usb_device.c A src/include/device/usb.h M src/soc/intel/common/block/xhci/xhci.c 4 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/38683/1 diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc index 966ca0d..33d81b9 100644 --- a/src/device/Makefile.inc +++ b/src/device/Makefile.inc @@ -58,3 +58,5 @@ verstage-y += mmio.c romstage-y += mmio.c ramstage-y += mmio.c + +ramstage-y += usb_device.c diff --git a/src/device/usb_device.c b/src/device/usb_device.c new file mode 100644 index 0000000..723ad1b --- /dev/null +++ b/src/device/usb_device.c @@ -0,0 +1,16 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <device/device.h> +#include <device/usb.h> + +/* Return the devicetree entry for the USB controller */ +__weak struct device *soc_get_usb_ctlr(void) +{ + return NULL; +} diff --git a/src/include/device/usb.h b/src/include/device/usb.h new file mode 100644 index 0000000..bd0f190 --- /dev/null +++ b/src/include/device/usb.h @@ -0,0 +1,19 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef _DEVICE_USB_H_ +#define _DEVICE_USB_H_ + +/** + * Return the devicetree entry for the USB controller. + * + * SoC-specific by nature. + */ +struct device *soc_get_usb_ctlr(void); + +#endif /* _DEVICE_USB_H_ */ diff --git a/src/soc/intel/common/block/xhci/xhci.c b/src/soc/intel/common/block/xhci/xhci.c index 4b8a5cc..222b215 100644 --- a/src/soc/intel/common/block/xhci/xhci.c +++ b/src/soc/intel/common/block/xhci/xhci.c @@ -19,6 +19,7 @@ #include <device/device.h> #include <device/pci.h> #include <device/pci_ids.h> +#include <device/usb.h> #include <drivers/usb/acpi/chip.h> #include <intelblocks/acpi.h> #include <intelblocks/xhci.h> @@ -107,6 +108,11 @@ __weak void soc_xhci_init(struct device *dev) { /* no-op */ } +struct device *soc_get_usb_ctlr(void) +{ + return pcidev_path_on_root(PCH_DEVFN_XHCI); +} + static struct device_operations usb_xhci_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, -- To view, visit https://review.coreboot.org/c/coreboot/+/38683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7dd2569dd615722a69f2dfd75622362e56a424ea Gerrit-Change-Number: 38683 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: soc/intel/tigerlake: Add code for early tcss
by Brandon Breitenstein (Code Review) 03 Feb '20

03 Feb '20

Brandon Breitenstein has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37870 ) Change subject: soc/intel/tigerlake: Add code for early tcss ...................................................................... Patch Set 10: (6 comments) https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@10 PS10, Line 10: PMC IPC driver is needed to communicate with EC in order to : get status of the Type C ports > What if the state of the ports changes after this configuration is done in coreboot? […] Kernel is re-scanning the ports once it is loaded. We are currently working on an EC flag that will allow Kernel to check if Coreboot already connected the port or not https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/M… File src/soc/intel/tigerlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/M… PS10, Line 33: $(CONFIG_TGL_CHROME_EC) > Do we really need to restrict this to just Chrome EC? I think instead we can have a chip config that […] will change this to early_tcss config flag https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… File src/soc/intel/tigerlake/early_tcss.c: https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 18: #include <console/post_codes.h> > Is this used? This was being used in an earlier iteration will remove https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 81: google_chromeec_usb_get_pd_ports > Should we really be tying this to Chrome EC here? Or should this be a more general interface that al […] Potentially doesn't need to be tied to Chrome EC, however with Intel EC all control of TCSS is handled by PMC so there isn't really a use case for this without Chrome EC at least for now. I'm not opposed to abstracting it out and allowing this to be used for any case though. https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 115: BS_OS_RESUME > Why is this configuration required on OS resume? There is no payload involved in that case. My understanding is that coreboot needed to reconnect the devices on resume. You are probably right here though I will do some testing and verify that this is actually doing nothing. https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 116: BS_PAYLOAD_LOAD > Isn't this too late for some cases? Example external display. […] This was done with the assumption that external display was not supported for bios. Can move this to earlier if we expand this code for early display as well -- To view, visit https://review.coreboot.org/c/coreboot/+/37870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e Gerrit-Change-Number: 37870 Gerrit-PatchSet: 10 Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> 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: Subrata Banik <subrata.banik(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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com> Gerrit-Comment-Date: Mon, 03 Feb 2020 21:40:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definiti...
by Furquan Shaikh (Code Review) 03 Feb '20

03 Feb '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 ) Change subject: soc/intel/{common,skl,cnl,icl,apl,tgl}: Move HFSTS1 register definition to SoC ...................................................................... Patch Set 27: Code-Review+2 (2 comments) https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/cannonlake/… File src/soc/intel/cannonlake/include/soc/me.h: https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/cannonlake/… PS27, Line 34: u32 reserved0: 1; Isn't this debug_mode for CML? Does this need to be defined differently based on SOC_INTEL_COMETLAKE? I haven't checked what happens for WHL. https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/common/bloc… File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/35546/27/src/soc/intel/common/bloc… PS27, Line 253: union me_hfsts1 hfs1; : hfs1.data = me_read_config32(PCI_ME_HFSTS1); : if (hfs1.fields.operation_mode == ME_HFS1_COM_NORMAL) : return true; : return false; nit: You can also add a helper function: bool cse_check_hfs1_com(int mode) { union me_hfsts1 hfs1; hfs1.data = me_read_config32(PCI_ME_HFSTS1); return hfs1.fields.operation_mode == mode; } bool cse_is_hfs1_com_normal(void) { return cse_check_hfs1_com(ME_HFS1_COM_NORMAL); } bool cse_is_hfs1_com_secover_mei_msg(void) { return cse_check_hfs1_com(ME_HFS1_COM_SECOVER_MEI_MSG); } bool cse_is_hfs1_com_soft_temp_disable(void) { return cse_check_hfs1_com(ME_HFS1_COM_SOFT_TEMP_DISABLE); } -- To view, visit https://review.coreboot.org/c/coreboot/+/35546 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7ea6043d7b5473d0c16e83d7b2d4b620c125652 Gerrit-Change-Number: 35546 Gerrit-PatchSet: 27 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <riz.pro(a)gmail.com> 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-Comment-Date: Mon, 03 Feb 2020 20:32:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add bootblock common stage file
by Angel Pons (Code Review) 03 Feb '20

03 Feb '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18457 ) Change subject: soc/intel/common: Add bootblock common stage file ...................................................................... Patch Set 38: (2 comments) https://review.coreboot.org/c/coreboot/+/18457/37/src/soc/intel/common/base… File src/soc/intel/common/basecode/bootblock/bootblock.c: https://review.coreboot.org/c/coreboot/+/18457/37/src/soc/intel/common/base… PS37, Line 39: bootblock_soc_early_init(void) > The platform you mention doesn't support this kind of runtime detection. […] Great, then it should really make use of the build-time approach :) https://review.coreboot.org/c/coreboot/+/18457/37/src/soc/intel/common/base… PS37, Line 102: (CONFIG(PAGING_IN_CACHE_AS_RAM)) > you consider 3 lines of code as too different? w.r.t. APL/GLK, I am talking about the hardware. Intel "true" SoCs consist of a single die, and their internal bus architecture is primarily IOSF. IOSF has existed for about six or seven SoC generations: Lincroft, Cedarview, Bay Trail, Braswell, Quark, Apollo Lake and Gemini Lake. From what I could gather, IOSF is similar to AMBA on ARM-based platforms (hell, even Quark has an IOSF-to-AHB bridge). To initialize the memory, one has to poke the various IOSF units around the SoC. For example, Bay Trail has: D-Unit (DRAM unit), B-Unit (Memory arbiter), R-Unit (REUT unit)... On the other hand, the big core platforms use a two-die solution, either using two different packages (S and H series) or a single MCM package (U and Y series). Both dies are connected using either DMI or OPI, which is heavily based on PCIe. To initialize the memory, one would usually poke the MCHBAR. Also, DMI needs to be initialized using DMIBAR (processor side) and RCBA (PCH side). -- To view, visit https://review.coreboot.org/c/coreboot/+/18457 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If84c08d33f6f8fd3cd9722ee893653f1d1ae90c1 Gerrit-Change-Number: 18457 Gerrit-PatchSet: 38 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: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> 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: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.corp-partner.google.com> Gerrit-Reviewer: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Varun Joshi <varun.joshi(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 03 Feb 2020 20:15:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

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

03 Feb '20

Vijay P Hiremath 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 7: (1 comment) https://review.coreboot.org/c/coreboot/+/37867/7/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c: https://review.coreboot.org/c/coreboot/+/37867/7/src/ec/google/chromeec/ec.… PS7, Line 1492: void *num_port so that all functions are consistent with return value -- 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: 7 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: Mon, 03 Feb 2020 18:45:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Add code for early tcss
by Furquan Shaikh (Code Review) 03 Feb '20

03 Feb '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37870 ) Change subject: soc/intel/tigerlake: Add code for early tcss ...................................................................... Patch Set 10: (8 comments) https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@10 PS10, Line 10: PMC IPC driver is needed to communicate with EC in order to : get status of the Type C ports What if the state of the ports changes after this configuration is done in coreboot? One example that comes to mind: Chrome OS requests all USB devices be disconnected when booting into recovery and then the devices can be connected after recovery screen is shown. Wouldn't this be a problem? https://review.coreboot.org/c/coreboot/+/37870/10//COMMIT_MSG@12 PS10, Line 12: Can you please add relevant partner bug? https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/M… File src/soc/intel/tigerlake/Makefile.inc: https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/M… PS10, Line 33: $(CONFIG_TGL_CHROME_EC) Do we really need to restrict this to just Chrome EC? I think instead we can have a chip config that mainboard can select if it wants to do early_tcss. (BTW, there is already a Kconfig for CHROMEEC which can be used in case this really has to be done only for Chrome EC) https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… File src/soc/intel/tigerlake/early_tcss.c: https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 18: #include <console/post_codes.h> Is this used? https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 37: bool usb; : bool polarity; : bool ufp; : bool acc; : uint8_t usb3_port; : uint8_t usb2_port; Can you please add comments indicating what these members really mean? https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 81: google_chromeec_usb_get_pd_ports Should we really be tying this to Chrome EC here? Or should this be a more general interface that allows any driver to provide these APIs? i.e. tcss_get_usb_pd_count_count(), tcss_get_usb_pd_port_info(). And in case of Chrome EC, it can implement those. Thoughts? https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 115: BS_OS_RESUME Why is this configuration required on OS resume? There is no payload involved in that case. https://review.coreboot.org/c/coreboot/+/37870/10/src/soc/intel/tigerlake/e… PS10, Line 116: BS_PAYLOAD_LOAD Isn't this too late for some cases? Example external display. Wouldn't you want the mux to be configured before FSP-S runs? -- To view, visit https://review.coreboot.org/c/coreboot/+/37870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I45c3fe9d4a2ec2f2f51b78cca2bd7e623540c00e Gerrit-Change-Number: 37870 Gerrit-PatchSet: 10 Gerrit-Owner: Shaunak Saha <shaunak.saha(a)intel.com> Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Keith Short <keithshort(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> 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: Subrata Banik <subrata.banik(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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-CC: Vijay P Hiremath <vijay.p.hiremath(a)intel.com> Gerrit-Comment-Date: Mon, 03 Feb 2020 18:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/hatch: Override board_get_usb_typec_port_paths on Kohaku
by Tim Wawrzynczak (Code Review) 03 Feb '20

03 Feb '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38618 ) Change subject: mb/google/hatch: Override board_get_usb_typec_port_paths on Kohaku ...................................................................... mb/google/hatch: Override board_get_usb_typec_port_paths on Kohaku Override board_get_usb_typec_port_paths on Kohaku so that the SSDT generator for the EC can add references to the USB port objects in the _DSD for each connector object. BUG=b:146506369 BRANCH=none TEST=Disassemble SSDT and verify Change-Id: Ide244ac47c439b8fae90cc842c0805cd17640556 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/hatch/variants/kohaku/ramstage.c 1 file changed, 24 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/38618/1 diff --git a/src/mainboard/google/hatch/variants/kohaku/ramstage.c b/src/mainboard/google/hatch/variants/kohaku/ramstage.c index 9b919fc..9cd0f7e 100644 --- a/src/mainboard/google/hatch/variants/kohaku/ramstage.c +++ b/src/mainboard/google/hatch/variants/kohaku/ramstage.c @@ -17,6 +17,7 @@ #include <gpio.h> #include <baseboard/variants.h> #include <soc/gpio.h> +#include <ec/google/chromeec/ec.h> void variant_ramstage_init(void) { @@ -30,3 +31,26 @@ mdelay(1); gpio_output(GPP_A12, 1); } + +static struct device_path usb_port_paths[][2] = +{ + { + {.type=DEVICE_PATH_USB, {.usb={ .port_type = 2, .port_id = 0 }}}, + {.type=DEVICE_PATH_USB, {.usb={ .port_type = 3, .port_id = 0 }}}, + }, + { + {.type=DEVICE_PATH_USB, {.usb={ .port_type = 2, .port_id = 1 }}}, + {.type=DEVICE_PATH_USB, {.usb={ .port_type = 3, .port_id = 1 }}}, + } +}; + +struct device_path *board_get_usb_typec_port_paths(int port, int *devices) +{ + if (port < 0 || port > 1) + return NULL; + + /* 2 devices for each port */ + *devices = 2; + + return usb_port_paths[port]; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/38618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide244ac47c439b8fae90cc842c0805cd17640556 Gerrit-Change-Number: 38618 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

3 8

Change in coreboot[master]: Update vboot submodule to upstream master
by Julius Werner (Code Review) 03 Feb '20

03 Feb '20

Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38604 ) Change subject: Update vboot submodule to upstream master ...................................................................... Update vboot submodule to upstream master Updating from commit id 6ef33b99: 2019-11-22 Hung-Te Lin futility: updater: refactor: unify getting temp files for firmware images to commit id 0e97e25e: 2020-01-23 Julius Werner 2lib: Fix struct vb2_hash the way it was meant to be Change-Id: I539aba2f283804f67ff3ff4f98324b3d10b2bb54 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M 3rdparty/vboot 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/38604/1 diff --git a/3rdparty/vboot b/3rdparty/vboot index f5367d5..0e97e25 160000 --- a/3rdparty/vboot +++ b/3rdparty/vboot @@ -1 +1 @@ -Subproject commit f5367d598a985520a8c935f68ac90d295c7b8d8e +Subproject commit 0e97e25e85f0499e23b09a31a2c7116759f191d5 -- To view, visit https://review.coreboot.org/c/coreboot/+/38604 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I539aba2f283804f67ff3ff4f98324b3d10b2bb54 Gerrit-Change-Number: 38604 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

4 3

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2020