coreboot-gerrit October 2018

coreboot-gerrit@coreboot.org

1 participants
1285 discussions

Change in coreboot[master]: sb/intel/common: Add common reset handler
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28862 ) Change subject: sb/intel/common: Add common reset handler ...................................................................... Patch Set 2: Ok, to clear this up a little. In the current definition by reset.h: * cpu_reset() was a cpu reset (for Intel) but dropped * soft_reset() is a system reset (for Intel) * hard_reset() is a full reset (for Intel) * global_reset() is not well specified but I bet always the same for Intel (either let the ME do it or do a ME+system reset) Deviating from this was ok as long as only platform code called the reset functions (it knew what was implemented as what). But if you call them from common code (e.g. vboot) they have to match. -- To view, visit https://review.coreboot.org/28862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e359b0c4d5a1060edd0940d24c2f78dfed8a590 Gerrit-Change-Number: 28862 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Alexandru Gagniuc <alexandrux.gagniuc(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 14:02:37 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: sb/intel/common: Add common reset handler
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28862 ) Change subject: sb/intel/common: Add common reset handler ...................................................................... Patch Set 2: (1 comment) Now I'm wondering: With nb/intel/fsp_rangeley fixed, would it build w/o a do_soft_reset() implementation? I've never seen any Intel chipset that would not do a hard reset if it detects a prior soft reset. So having a do_soft_reset() for Intel doesn't make much sense. The situation is even more annyoing if you look at soc/intel/common/. They just redefined the meaning of the different resets. https://review.coreboot.org/#/c/28862/2/src/southbridge/intel/fsp_rangeley/… File src/southbridge/intel/fsp_rangeley/reset.c: https://review.coreboot.org/#/c/28862/2/src/southbridge/intel/fsp_rangeley/… PS2, Line 23: Guess who seems to be the only one calling soft_reset()? it's nb/intel/fsp_rangeley... That call should be fixed, I guess. -- To view, visit https://review.coreboot.org/28862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e359b0c4d5a1060edd0940d24c2f78dfed8a590 Gerrit-Change-Number: 28862 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Alexandru Gagniuc <alexandrux.gagniuc(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 13:48:38 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: sb/intel/common: Add common reset handler
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28862 ) Change subject: sb/intel/common: Add common reset handler ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/#/c/28862/2/src/southbridge/intel/common/reset.c File src/southbridge/intel/common/reset.c: https://review.coreboot.org/#/c/28862/2/src/southbridge/intel/common/reset.… PS2, Line 28: FULL_RST This would issue a power-cycle reset, which is usually unwanted. I'm also not sure if it's valid to set it w/o SYS_RST. Also worth to mention: I've seen different patterns regarding this register. Older platforms set the configuration bits (FULL_RST and or SYS_RST) first and then again with RST_CPU in a second outb(). It's still unclear (also not documented explicitly) if this is needed, but it doesn't hurt. So I'd prefer two outb() calls unless _all_ affected platforms did a single call before. -- To view, visit https://review.coreboot.org/28862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e359b0c4d5a1060edd0940d24c2f78dfed8a590 Gerrit-Change-Number: 28862 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Alexandru Gagniuc <alexandrux.gagniuc(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 13:40:12 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: sb/intel/common: Add common reset handler
by Patrick Rudolph (Code Review) 03 Oct '18

03 Oct '18

Hello Alexandru Gagniuc, Felix Held, Arthur Heymans, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/28862 to look at the new patch set (#2). Change subject: sb/intel/common: Add common reset handler ...................................................................... sb/intel/common: Add common reset handler Add common reset.c and remove all other duplicated files. Include reset.c in all stages to build with VBOOT enabled. Change-Id: I8e359b0c4d5a1060edd0940d24c2f78dfed8a590 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M src/southbridge/intel/bd82x6x/Makefile.inc D src/southbridge/intel/bd82x6x/reset.c M src/southbridge/intel/common/Makefile.inc R src/southbridge/intel/common/reset.c M src/southbridge/intel/fsp_bd82x6x/Makefile.inc M src/southbridge/intel/fsp_i89xx/Makefile.inc D src/southbridge/intel/fsp_i89xx/reset.c M src/southbridge/intel/fsp_rangeley/Makefile.inc D src/southbridge/intel/fsp_rangeley/reset.c M src/southbridge/intel/i82801dx/Makefile.inc D src/southbridge/intel/i82801dx/reset.c M src/southbridge/intel/i82801gx/Makefile.inc D src/southbridge/intel/i82801gx/reset.c M src/southbridge/intel/i82801ix/Makefile.inc M src/southbridge/intel/i82801jx/Makefile.inc M src/southbridge/intel/ibexpeak/Makefile.inc M src/southbridge/intel/lynxpoint/Makefile.inc D src/southbridge/intel/lynxpoint/reset.c 18 files changed, 11 insertions(+), 198 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/28862/2 -- To view, visit https://review.coreboot.org/28862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8e359b0c4d5a1060edd0940d24c2f78dfed8a590 Gerrit-Change-Number: 28862 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Alexandru Gagniuc <alexandrux.gagniuc(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in coreboot[master]: src/nb/i945: Fix "CONFIG_DEFAULT_CONSOLE_LOGLEVEL > 8" if statement
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28878 ) Change subject: src/nb/i945: Fix "CONFIG_DEFAULT_CONSOLE_LOGLEVEL > 8" if statement ...................................................................... Patch Set 3: >> Using #if is a bad habit. Also, what are the outer parentheses for? > > If I'm not wrong, '#if' is used in (romstage) to bypass the code if > the statement is not true. You can use it like that. But that's not specific to any stage. And it's a bad choice because the guarded code won't be compile tested. There is no other way to do this in romcc bootblock code, maybe that is what you had in mind? -- To view, visit https://review.coreboot.org/28878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ff87c76b3aef4af93d88dd9c02a743656aacfc0 Gerrit-Change-Number: 28878 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 11:41:56 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: src/mb: Remove dead code
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28876 ) Change subject: src/mb: Remove dead code ...................................................................... Patch Set 4: Ah, looks like my doubts were misplaced. It should have compiled. The errors in PS#3 were introduced by the patch... I wouldn't object if we drop the code but if, we should also drop the then obsolete implementation. I would ack it, if you think it's the right thing. But I can't ack it if you are just following my examples what could be done. -- To view, visit https://review.coreboot.org/28876 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I318de3e7c807bb5b5efdf61fef387d34225a8149 Gerrit-Change-Number: 28876 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 11:34:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: console: Enable CONSOLE_USB by default
by Nico Huber (Code Review) 03 Oct '18

03 Oct '18

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/28892 Change subject: console: Enable CONSOLE_USB by default ...................................................................... console: Enable CONSOLE_USB by default It seems to be the only user of the USB debug driver. So having to enable it separately seems wrong. It still depends on the selection of the EHCI debug driver. Change-Id: I5f5f38a912423d9b8f1e71ae875b6a14fdee651c Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M src/console/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/28892/1 diff --git a/src/console/Kconfig b/src/console/Kconfig index a7a16f0..febed95 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -157,7 +157,7 @@ config CONSOLE_USB bool "USB dongle console output" depends on USBDEBUG - default n + default y help Send coreboot debug output to USB. -- To view, visit https://review.coreboot.org/28892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5f5f38a912423d9b8f1e71ae875b6a14fdee651c Gerrit-Change-Number: 28892 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in coreboot[master]: soc/intel/cannonlake: Move the FSP related callbacks to separate files
by build bot (Jenkins) (Code Review) 03 Oct '18

03 Oct '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/28661 ) Change subject: soc/intel/cannonlake: Move the FSP related callbacks to separate files ...................................................................... Patch Set 7: (2 comments) https://review.coreboot.org/#/c/28661/7/src/soc/intel/cannonlake/fsp_params… File src/soc/intel/cannonlake/fsp_params.c: https://review.coreboot.org/#/c/28661/7/src/soc/intel/cannonlake/fsp_params… PS7, Line 82: for (i = 0; i < ARRAY_SIZE(params->Usb2OverCurrentPin); i++) { braces {} are not necessary for single statement blocks https://review.coreboot.org/#/c/28661/7/src/soc/intel/cannonlake/fsp_params… PS7, Line 86: for (i = 0; i < ARRAY_SIZE(params->Usb3OverCurrentPin); i++) { braces {} are not necessary for single statement blocks -- To view, visit https://review.coreboot.org/28661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iffadc57f6986e688aa1bbe4e5444d105386ad92e Gerrit-Change-Number: 28661 Gerrit-PatchSet: 7 Gerrit-Owner: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Boon Tiong Teo <boon.tiong.teo(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 09:09:44 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: soc/intel/cannonlake: Add CNP PCH-H gpio pin definitions
by build bot (Jenkins) (Code Review) 03 Oct '18

03 Oct '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/28890 ) Change subject: soc/intel/cannonlake: Add CNP PCH-H gpio pin definitions ...................................................................... Patch Set 2: (10 comments) https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… File src/soc/intel/cannonlake/include/soc/gpio_soc_defs.h: https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 592: #define GPP_I0 189 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 593: #define GPP_I1 190 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 594: #define GPP_I2 191 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 595: #define GPP_I3 192 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 596: #define GPP_I4 193 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 597: #define GPP_I5 194 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 598: #define GPP_I6 195 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 599: #define GPP_I7 196 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 600: #define GPP_I8 197 please, no space before tabs https://review.coreboot.org/#/c/28890/2/src/soc/intel/cannonlake/include/so… PS2, Line 601: #define GPP_I9 198 please, no space before tabs -- To view, visit https://review.coreboot.org/28890 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I496ec059de125b97c646581bbd3b8bfe6ffa641e Gerrit-Change-Number: 28890 Gerrit-PatchSet: 2 Gerrit-Owner: PraveenX Hodagatta Pranesh <praveenx.hodagatta.pranesh(a)intel.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 03 Oct 2018 08:59:36 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: mb/google/octopus: Enable DRAM_PART_NUM_IN_CBI feature for Bobba
by Pan Sheng-Liang (Code Review) 03 Oct '18

03 Oct '18

Pan Sheng-Liang has uploaded this change for review. ( https://review.coreboot.org/28891 Change subject: mb/google/octopus: Enable DRAM_PART_NUM_IN_CBI feature for Bobba ...................................................................... mb/google/octopus: Enable DRAM_PART_NUM_IN_CBI feature for Bobba Enable DRAM_PART_NUM_IN_CBI feature to get DRAM part number from CBI and set DRAM_PART_IN_CBI_BOARD_ID_MIN to 2 for DVT. BUG=b:115697578 TEST=verified it in Bobba EVT board which rework ram id. Signed-off-by: Pan Sheng-Liang <sheng-liang.pan(a)quanta.corp-partner.google.com> Change-Id: I0fb457d8772f5038e5d90188d7682956ddfad46b --- M src/mainboard/google/octopus/Kconfig 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/28891/1 diff --git a/src/mainboard/google/octopus/Kconfig b/src/mainboard/google/octopus/Kconfig index d245a62..7cb6a6d 100644 --- a/src/mainboard/google/octopus/Kconfig +++ b/src/mainboard/google/octopus/Kconfig @@ -117,6 +117,7 @@ default y if BOARD_GOOGLE_PHASER default y if BOARD_GOOGLE_MEEP default y if BOARD_GOOGLE_AMPTON + default y if BOARD_GOOGLE_BOBBA config DRAM_PART_NUM_ALWAYS_IN_CBI bool @@ -130,7 +131,7 @@ default 255 if BOARD_GOOGLE_BIP default 2 if BOARD_GOOGLE_PHASER default 9 if BOARD_GOOGLE_FLEEX - default 9 if BOARD_GOOGLE_BOBBA + default 2 if BOARD_GOOGLE_BOBBA default 1 if BOARD_GOOGLE_MEEP default 255 if BOARD_GOOGLE_AMPTON default 255 if BOARD_GOOGLE_OCTOPUS -- To view, visit https://review.coreboot.org/28891 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0fb457d8772f5038e5d90188d7682956ddfad46b Gerrit-Change-Number: 28891 Gerrit-PatchSet: 1 Gerrit-Owner: Pan Sheng-Liang <sl.pan.quantw(a)gmail.com> Gerrit-Reviewer: Sheng-Liang Pan <sheng-liang.pan(a)quanta.corp-partner.google.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2018