Attention is currently required from: Marc Jones, Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Rocky Phagura, Tim Wawrzynczak, David Hendricks, Morgan Jang, Patrick Rudolph, Tim Chu.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52090 )
Change subject: src/intel/xeon_sp: add hardware error support (HEST)
......................................................................
Patch Set 18: -Code-Review
(2 comments)
File src/soc/intel/common/block/acpi/acpi/globalnvs.asl:
https://review.coreboot.org/c/coreboot/+/52090/comment/057720ab_d5cdb1ba
PS18, Line 27: , 8, // 0x40 - 0x48 Hest log buffer
> runtime ASL code does not use it. But runtime SMM uses GNVS to retrieve the hest log address.
Where in SMM is this being used? I can't see it.
File src/soc/intel/xeon_sp/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/52090/comment/c9b403ed_1218951c
PS18, Line 19: ifeq ($(CONFIG_SOC_INTEL_XEON_RAS),y)
: subdirs-y += ras
: endif
Much simpler:
subdirs-$(CONFIG_SOC_INTEL_XEON_RAS) += ras
--
To view, visit https://review.coreboot.org/c/coreboot/+/52090
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76b2af153616182cc053ca878f30fe056e9c8bd
Gerrit-Change-Number: 52090
Gerrit-PatchSet: 18
Gerrit-Owner: Rocky Phagura
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Rocky Phagura
Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 28 May 2021 08:23:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Rocky Phagura
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Marc Jones, Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Rocky Phagura, Tim Wawrzynczak, David Hendricks, Morgan Jang, Patrick Rudolph, Tim Chu.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52090 )
Change subject: src/intel/xeon_sp: add hardware error support (HEST)
......................................................................
Patch Set 18:
(2 comments)
File src/soc/intel/common/block/acpi/acpi/globalnvs.asl:
https://review.coreboot.org/c/coreboot/+/52090/comment/d4bfc9b7_62d19d30
PS18, Line 27: , 8, // 0x40 - 0x48 Hest log buffer
> runtime ASL code does not use it. But runtime SMM uses GNVS to retrieve the hest log address.
Ok. You might want to add a comment about this there or in the c code.
File src/soc/intel/xeon_sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/52090/comment/746262da_4eda182b
PS18, Line 149: * +--------------------------+ cbmem_top
: * | HEST ELOG | (CONFIG_ERROR_LOG_BUFFER_SIZE)
: * +--------------------------+
Here the HEST ELOG is below cbmem_top, but in the code it looks like it is above cbmem_top.
Is there any particular reason that HEST has to be allocated above cbmem and below DPR? This log is generated in SMM right? If it just has to be in DRAM, then allocating it in cbmem is a much better idea.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52090
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If76b2af153616182cc053ca878f30fe056e9c8bd
Gerrit-Change-Number: 52090
Gerrit-PatchSet: 18
Gerrit-Owner: Rocky Phagura
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Marc Jones <marc(a)marcjonesconsulting.com>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Rocky Phagura <rphagura(a)fb.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Rocky Phagura
Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Fri, 28 May 2021 08:22:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Rocky Phagura
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Tao Xia has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55035 )
Change subject: mb/google/dedede/var/sasukette: Modify the touch pad slave address
......................................................................
mb/google/dedede/var/sasukette: Modify the touch pad slave address
There are two touch pads that Sasukette used have the same slave address.
It will cause one of the touch pad hid identification error.
BUG=b:189520603
BRANCH=dedede
TEST=Verify OK
Signed-off-by: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com>
Change-Id: I09bc12eeca7f34f6e9424e5cbb7f812f3bbfe0de
Change-Id: If0bd80baa27dfeb7bcb43f0ca4b02e1228e372a6
---
M src/mainboard/google/dedede/variants/sasukette/overridetree.cb
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/55035/1
diff --git a/src/mainboard/google/dedede/variants/sasukette/overridetree.cb b/src/mainboard/google/dedede/variants/sasukette/overridetree.cb
index 8d1b95b..904b4d9 100644
--- a/src/mainboard/google/dedede/variants/sasukette/overridetree.cb
+++ b/src/mainboard/google/dedede/variants/sasukette/overridetree.cb
@@ -125,7 +125,7 @@
register "generic.wake" = "GPE0_DW0_03"
register "generic.probed" = "1"
register "hid_desc_reg_offset" = "0x20"
- device i2c 2c on end
+ device i2c 2a on end
end
end #I2C 0
device pci 19.0 on
--
To view, visit https://review.coreboot.org/c/coreboot/+/55035
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0bd80baa27dfeb7bcb43f0ca4b02e1228e372a6
Gerrit-Change-Number: 55035
Gerrit-PatchSet: 1
Gerrit-Owner: Tao Xia <xiatao5(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55024 )
Change subject: arch/x8/include/bert_storage: introduce and use bert_generate_ssdt
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/55024
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia955f627c190ea38e05b5aaedc7cb2d030274e83
Gerrit-Change-Number: 55024
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 28 May 2021 06:57:33 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55023 )
Change subject: arch/x86/acpi_bert_storage: change return type of bert_errors_present
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/55023
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I13d6472deeb26ba92d257761df069e32d9b2e5d4
Gerrit-Change-Number: 55023
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 28 May 2021 06:56:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55022 )
Change subject: mb/google/guybrush: Unassign I2C1 and set GPIOs to input/no-pull
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/55022
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I244b398c695683fa4e217d83aa0498bcd8f0fbbe
Gerrit-Change-Number: 55022
Gerrit-PatchSet: 2
Gerrit-Owner: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Comment-Date: Fri, 28 May 2021 06:56:19 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Christian Walter, Kyösti Mälkki.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55016 )
Change subject: Apply more uses for Kconfig TPM
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/55016
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I54b296563940cd46fe9da9fe789b746f2fc1987d
Gerrit-Change-Number: 55016
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Fri, 28 May 2021 06:54:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Mario Scheithauer, Subrata Banik, Lean Sheng Tan, Patrick Rudolph.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55034 )
Change subject: soc/intel/elkhartlake: Update FSP-S UPD related configs part 3
......................................................................
Patch Set 3:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55034/comment/2887895b_4b497e48
PS3, Line 7: soc/intel/elkhartlake: Update FSP-S UPD related configs part 3
Add least here, you can mention RP and USB in the commit message summary instead of using *part 3*.
File src/soc/intel/elkhartlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/55034/comment/f5c4867b_37d6a32f
PS3, Line 184: params->Usb2OverCurrentPin[i] = 0xff;
I find the ternary operator in these cases, where the assignment of only one variable depends on the condition.
params->Usb2OverCurrentPin[i] = config->usb2_ports[i].enable ? config->usb2_ports[i].ocpin : 0xff;
https://review.coreboot.org/c/coreboot/+/55034/comment/6e939a1f_5c63f9d8
PS3, Line 226: params->PcieRpLtrMaxNoSnoopLatency[i] = 0x1003; //Max No Snoop Latency
Please add a space after `//`.
Is the comment necessary? It’s already in the variable name.
https://review.coreboot.org/c/coreboot/+/55034/comment/471f2ca4_57955710
PS3, Line 227: params->PcieRpLtrMaxSnoopLatency[i] = 0x1003; //Max Snoop Latency
Ditto.
--
To view, visit https://review.coreboot.org/c/coreboot/+/55034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I60afb78a7997b8465dd6318f3abee28f95a65100
Gerrit-Change-Number: 55034
Gerrit-PatchSet: 3
Gerrit-Owner: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 28 May 2021 06:47:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Paul Menzel, Mario Scheithauer, Subrata Banik, Patrick Rudolph.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54960 )
Change subject: soc/intel/elkhartlake: Update FSP-S UPD related configs part 2
......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/54960/comment/ea1eb87c_d3a47e5e
PS8, Line 7: part 2
> I am not a fan of this, as it does not help a lot, when reading the commit log/history. […]
I dont fully understood what do you mean. Do you mean i should submit the whole FSP-S as a big commit? What doe you mean by grouped by a word? How can this be confusing to you?
--
To view, visit https://review.coreboot.org/c/coreboot/+/54960
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I657f44f8506640c23049614b2db9d1837e6d44ed
Gerrit-Change-Number: 54960
Gerrit-PatchSet: 8
Gerrit-Owner: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 28 May 2021 06:44:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment