coreboot-gerrit April 2017

coreboot-gerrit@coreboot.org

1 participants
4151 discussions

Change in coreboot[master]: soc/intel/skylake: Set xtal bypass on low power idle
by Naresh Solanki (Code Review) 29 Apr '17

29 Apr '17

Hello Aaron Durbin, Subrata Banik, Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19442 to look at the new patch set (#7). Change subject: soc/intel/skylake: Set xtal bypass on low power idle ...................................................................... soc/intel/skylake: Set xtal bypass on low power idle When using Wake On Voice &/or DCI, it requires xtal to be active during low power idle. With xtal being active in S0ix state power impact is 1-2 mW. Hence set xtal bypass bit in CIR31C for low power idle entry. TEST= Build with s0ix enable for Poppy. Boot to OS & verify that bit 22 of CIR31C register is set. s0ix works. Change-Id: Ide2d01536f652cd1b0ac32eede89ec410c5101cf Signed-off-by: Naresh G Solanki <naresh.solanki(a)intel.com> --- M src/soc/intel/skylake/finalize.c M src/soc/intel/skylake/include/soc/pmc.h 2 files changed, 9 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/19442/7 -- To view, visit https://review.coreboot.org/19442 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ide2d01536f652cd1b0ac32eede89ec410c5101cf Gerrit-PatchSet: 7 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rajat Jain <rajatja(a)google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Roy Park <roy.mingi.park(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-Reviewer: rushikesh s kadam <rushikesh.s.kadam(a)intel.com>

1 0

Change in coreboot[master]: libpayload/gdb: fix unused variable warning
by Patrick Georgi (Code Review) 29 Apr '17

29 Apr '17

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/19482 ) Change subject: libpayload/gdb: fix unused variable warning ...................................................................... libpayload/gdb: fix unused variable warning input_underrun is defined but not used. A reasonably new compiler, enabled warnings and warnings-as-error make the build break for no good reason. Change-Id: Ibeb7ba53aad5738938093ab7b34695c9c99c9afe Signed-off-by: Patrick Georgi <pgeorgi(a)chromium.org> Reviewed-on: https://review.coreboot.org/19482 Tested-by: build bot (Jenkins) Reviewed-by: Marc Jones <marc(a)marcjonesconsulting.com> --- M payloads/libpayload/gdb/transport.c 1 file changed, 0 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Marc Jones: Looks good to me, approved diff --git a/payloads/libpayload/gdb/transport.c b/payloads/libpayload/gdb/transport.c index 8f73bb0..5b575d0 100644 --- a/payloads/libpayload/gdb/transport.c +++ b/payloads/libpayload/gdb/transport.c @@ -22,8 +22,6 @@ static const int timeout_us = 100 * 1000; static const char output_overrun[] = "GDB output buffer overrun (try " "increasing reply.size)!\n"; -static const char input_underrun[] = "GDB input message truncated (bug or " - "communication problem)?\n"; /* Serial-specific glue code... add more transport layers here when desired. */ -- To view, visit https://review.coreboot.org/19482 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibeb7ba53aad5738938093ab7b34695c9c99c9afe Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins)

1 0

Change in coreboot[master]: pci_device: Write vendor ID to subsystem vendor ID
by Philippe Mathieu-Daudé (Code Review) 29 Apr '17

29 Apr '17

Philippe Mathieu-Daudé has posted comments on this change. ( https://review.coreboot.org/19467 ) Change subject: pci_device: Write vendor ID to subsystem vendor ID ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/19467 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5027331a6adf9109767415ba22dfcb17b35ef54b Gerrit-PatchSet: 7 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in coreboot[master]: arch/x86: Share storage data structures between early stages
by Martin Roth (Code Review) 29 Apr '17

29 Apr '17

Martin Roth has posted comments on this change. ( https://review.coreboot.org/19300 ) Change subject: arch/x86: Share storage data structures between early stages ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/19300 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I20a01b850a31df9887a428bf07ca476c8410d33e Gerrit-PatchSet: 4 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in coreboot[master]: intel/common/block/i2c: Add common block for I2C
by Furquan Shaikh (Code Review) 29 Apr '17

29 Apr '17

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/19468 ) Change subject: intel/common/block/i2c: Add common block for I2C ...................................................................... Patch Set 6: (16 comments) https://review.coreboot.org/#/c/19468/6/src/soc/intel/common/block/i2c/i2c.c File src/soc/intel/common/block/i2c/i2c.c: PS6, Line 32: if (devfn >= 0) { : /* devfn -> dev */ : dev = dev_find_slot(0, devfn); : if (dev) { : /* dev -> bar0 */ : res = find_resource(dev, PCI_BASE_ADDRESS_0); : if (res) : return res->base; : } : } if (devfn < 0) return (uintptr_t)NULL; dev = dev_find_slot(0, devfn); if (!dev) return (uintptr_t)NULL; res = find_resource(dev, PCI_BASE_ADDRESS_0); if (res) return res->base; return (uintptr_t)NULL; You can avoid multiple levels of nesting blocks. PS6, Line 82: The device should already be enabled and out of reset, Should this driver take the controller out of reset and not rely on caller doing anything for it? https://review.coreboot.org/#/c/19468/6/src/soc/intel/common/block/i2c/i2c_… File src/soc/intel/common/block/i2c/i2c_early.c: PS6, Line 52: { space before { PS6, Line 53: printk(BIOS_ERR, "I2C%u not enabled for early init\n", bus); Do we want to print this? It could be intentional and not an error. https://review.coreboot.org/#/c/19468/6/src/soc/intel/common/block/i2c/lpss… File src/soc/intel/common/block/i2c/lpss_i2c.c: Line 477: } > Why is this dropped? It was moved to intel/common/block/i2c/i2c.c Line 729: } > Why did this get dropped? Moved to intel/common/block/i2c/i2c.c PS6, Line 447: int > Why was this exposed globally? I think because this is called from lpss_i2c_acpi_fill_ssdt which is now moved to a different file. PS6, Line 592: int lpss_i2c_bus_to_dev(int bus) : { : return i2c_soc_bus_to_devfn(bus); : } : : int lpss_i2c_dev_to_bus(struct device *dev) : { : return i2c_soc_devfn_to_bus(dev->path.pci.devfn); : } Why not just call i2c_soc_* functions instead of having a function call X->lpss_i2c_bus_to_dev->i2c_soc_bus_to_devfn? I think we can simply have X->i2c_soc_bus_to_devfn. PS6, Line 602: __attribute__((weak)) const struct lpss_i2c_bus_config *i2c_get_soc_cfg(void){ : printk(BIOS_ERR, "SoC does not have I2C configuration!!\n"); : return NULL; : } I think instead of having a weak function here, we would want the compilation to fail if SoC does not provide the config? https://review.coreboot.org/#/c/19468/6/src/soc/intel/common/block/i2c/lpss… File src/soc/intel/common/block/i2c/lpss_i2c.h: Line 1: /* This file has been moved. All boards including this file need to be updated. PS6, Line 91: /* Frequency represented as ticks per ns. Can also be used to calculate : * the number of ticks to meet a time target or the period. */ : struct freq { : uint32_t ticks; : uint32_t ns; : }; : : /* Control register definitions */ : enum { : CONTROL_MASTER_MODE = (1 << 0), : CONTROL_SPEED_SS = (1 << 1), : CONTROL_SPEED_FS = (1 << 2), : CONTROL_SPEED_HS = (3 << 1), : CONTROL_SPEED_MASK = (3 << 1), : CONTROL_10BIT_SLAVE = (1 << 3), : CONTROL_10BIT_MASTER = (1 << 4), : CONTROL_RESTART_ENABLE = (1 << 5), : CONTROL_SLAVE_DISABLE = (1 << 6), : }; : : /* Command/Data register definitions */ : enum { : CMD_DATA_CMD = (1 << 8), : CMD_DATA_STOP = (1 << 9), : }; : : /* Status register definitions */ : enum { : STATUS_ACTIVITY = (1 << 0), : STATUS_TX_FIFO_NOT_FULL = (1 << 1), : STATUS_TX_FIFO_EMPTY = (1 << 2), : STATUS_RX_FIFO_NOT_EMPTY = (1 << 3), : STATUS_RX_FIFO_FULL = (1 << 4), : STATUS_MASTER_ACTIVITY = (1 << 5), : STATUS_SLAVE_ACTIVITY = (1 << 6), : }; : : /* Enable register definitions */ : enum { : ENABLE_CONTROLLER = (1 << 0), : }; : : /* Interrupt status register definitions */ : enum { : INTR_STAT_RX_UNDER = (1 << 0), : INTR_STAT_RX_OVER = (1 << 1), : INTR_STAT_RX_FULL = (1 << 2), : INTR_STAT_TX_OVER = (1 << 3), : INTR_STAT_TX_EMPTY = (1 << 4), : INTR_STAT_RD_REQ = (1 << 5), : INTR_STAT_TX_ABORT = (1 << 6), : INTR_STAT_RX_DONE = (1 << 7), : INTR_STAT_ACTIVITY = (1 << 8), : INTR_STAT_STOP_DET = (1 << 9), : INTR_STAT_START_DET = (1 << 10), : INTR_STAT_GEN_CALL = (1 << 11), : }; Why do these need to be exported in the header file? I think we can put these in lpss_i2c.c https://review.coreboot.org/#/c/19468/6/src/soc/intel/common/block/include/… File src/soc/intel/common/block/include/intelblocks/lpss_i2c.h: Line 77: /* Initialize I2C Controller */ in early stages before memory is up. PS6, Line 78: int What are the expected return values? PS6, Line 80: byt by PS6, Line 82: GSPI I2C PS6, Line 102: Driver provides : * weak implementation with default I2C-bus configuration. That is not true. -- To view, visit https://review.coreboot.org/19468 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d91a04c22e181e3a995112cce6d5f0324130b81 Gerrit-PatchSet: 6 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in coreboot[master]: scarlet/gru: skip usbphy1 setup for Scarlet
by build bot (Jenkins) (Code Review) 29 Apr '17

29 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19489 ) Change subject: scarlet/gru: skip usbphy1 setup for Scarlet ...................................................................... Patch Set 3: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/52966/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/8812/ : SUCCESS -- To view, visit https://review.coreboot.org/19489 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I66e0d8a235cc9057964f7abca32bc692d41e88fd Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Philip Chen <philipchen(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Philip Chen <philipchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in coreboot[master]: scarlet/gru: skip usbphy1 setup for Scarlet
by Philip Chen (Code Review) 29 Apr '17

29 Apr '17

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19489 to look at the new patch set (#3). Change subject: scarlet/gru: skip usbphy1 setup for Scarlet ...................................................................... scarlet/gru: skip usbphy1 setup for Scarlet BUG=b:37685249 TEST=boot Scarlet, check the firmware log, and confirm no errors about USB1 Change-Id: I66e0d8a235cc9057964f7abca32bc692d41e88fd Signed-off-by: philipchen <philipchen(a)google.com> --- M src/mainboard/google/gru/mainboard.c M src/soc/rockchip/rk3399/include/soc/grf.h 2 files changed, 23 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/19489/3 -- To view, visit https://review.coreboot.org/19489 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I66e0d8a235cc9057964f7abca32bc692d41e88fd Gerrit-PatchSet: 3 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Philip Chen <philipchen(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Philip Chen <philipchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins)

1 0

Change in coreboot[master]: cbmem: Add new command line flag to dump console for one boo...
by build bot (Jenkins) (Code Review) 29 Apr '17

29 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19497 ) Change subject: cbmem: Add new command line flag to dump console for one boot only ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/52965/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/8811/ : SUCCESS -- To view, visit https://review.coreboot.org/19497 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic17383507a884d84de9a2a880380cb15b25708a1 Gerrit-PatchSet: 1 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in coreboot[master]: vboot: Separate board name and version number in FWID with a...
by Julius Werner (Code Review) 29 Apr '17

29 Apr '17

Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/19487 ) Change subject: vboot: Separate board name and version number in FWID with a dot ...................................................................... vboot: Separate board name and version number in FWID with a dot It's standard practice in vboot that the FWID consists of <board_name>.<version_number> (e.g. Google_Kevin.8785.57.0). In fact, some tools rely on this and cut the string at the first dot to separate the two. The current Kconfig default in coreboot instead leads to ugly, parser-breaking FWIDs like Google_Kevin4.5-1234-5678abcd. This patch fixes that. Change-Id: I65cd5285c69e2e485d55a41a65d735f6a2291c16 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/19487 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> Reviewed-by: Patrick Georgi <pgeorgi(a)google.com> --- M src/vboot/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Aaron Durbin: Looks good to me, approved build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved diff --git a/src/vboot/Kconfig b/src/vboot/Kconfig index 4ddf3e3..c4b4687 100644 --- a/src/vboot/Kconfig +++ b/src/vboot/Kconfig @@ -206,7 +206,7 @@ config VBOOT_FWID_VERSION string "Firmware ID version" - default "$(KERNELVERSION)" + default ".$(KERNELVERSION)" help This is the second part of the FWID written to various regions of a vboot firmware image to identify its version. -- To view, visit https://review.coreboot.org/19487 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I65cd5285c69e2e485d55a41a65d735f6a2291c16 Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins)

1 0

Change in coreboot[master]: rowan: Fix default test HWID.
by Julius Werner (Code Review) 29 Apr '17

29 Apr '17

Julius Werner has submitted this change and it was merged. ( https://review.coreboot.org/19488 ) Change subject: rowan: Fix default test HWID. ...................................................................... rowan: Fix default test HWID. Correct the default GBB_HWID to "ROWAN TEST 9387" BRANCH=chromeos-2016.05 BUG=b:35774871 TEST=emerge-rowan coreboot chromeos-bootimage, strings /build/rowan/firmware/image.bin | grep "ROWAN TEST" and look for 9387 in output Change-Id: I7851010305caf056958c8a6a328b0506bf2208cd Signed-off-by: Patrick Berny <pberny(a)chromium.org> Reviewed-on: https://review.coreboot.org/19488 Tested-by: build bot (Jenkins) Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M src/mainboard/google/oak/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Julius Werner: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/google/oak/Kconfig b/src/mainboard/google/oak/Kconfig index d727ffc..c00c622 100644 --- a/src/mainboard/google/oak/Kconfig +++ b/src/mainboard/google/oak/Kconfig @@ -93,7 +93,7 @@ default "OAK TEST 6858" if BOARD_GOOGLE_OAK default "ELM TEST 3839" if BOARD_GOOGLE_ELM default "HANA TEST 5855" if BOARD_GOOGLE_HANA - default "ROWAN TEST 5855" if BOARD_GOOGLE_ROWAN + default "ROWAN TEST 9387" if BOARD_GOOGLE_ROWAN # All Oak-derivatives count their board IDs as 0 being equivalent to Oak rev6. config BOARD_ID_ADJUSTMENT -- To view, visit https://review.coreboot.org/19488 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7851010305caf056958c8a6a328b0506bf2208cd Gerrit-PatchSet: 2 Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Owner: Patrick Berny <pberny(a)google.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)google.com> Gerrit-Reviewer: Patrick Berny <pberny(a)google.com> Gerrit-Reviewer: build bot (Jenkins)

1 0

← Newer
1
...
5
6
7
8
9
10
11
...
416
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2017