Attention is currently required from: Tim Wawrzynczak, Julius Werner, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56726 )
Change subject: soc/intel: Add new Kconfig INTEL_ALLOW_SOC_DEBUG_HOOKS
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56726/comment/4a5e5644_1590e8f5
PS4, Line 9: The intent of this Kconfig is, when selected, to compile in any
: Intel-specific hooks that may be available.
> Well, I asked Tim to move it into soc/intel (see above).
Aah. I didn't realize that there was a discussion on the location of this config already. Humm..
> It seems to be a very Intel-specific mechanism so I think that's appropriate? I still don't really know what exactly "debug hooks" refers to or how this "crashlog" works but it seems to be a completely vendor-specific mechanism?
Crashlog --> yes, that is very Intel specific. The top-level config is primarily to allow mainboard developers to simply enable a single config that will then auto-select any SoC specific debug infrastructure (Kconfigs). When I say SoC specific debug infrastructure, I am thinking of tracehubs or any other hardware debug support that a SoC provides. For Intel, it would be crashlog. For ARM platforms, it might be some other hardware controller. If you think this is something not really useful for other platforms, we can definitely start with just Intel and when required move it to a more common location.
(BTW, if this gets moved to a common location, we would need two Kconfigs and not just one i.e. SoC indicating support for debug hooks something like SOC_SUPPORTS_DEBUG_HOOKS which then allows mainboard to select ENABLE_DEBUG_HOOKS only on supported platforms).
--
To view, visit https://review.coreboot.org/c/coreboot/+/56726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0e7f076bb7e3b630c3ab9d28071312d3e21e927
Gerrit-Change-Number: 56726
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 10 Aug 2021 21:13:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56726 )
Change subject: soc/intel: Add new Kconfig INTEL_ALLOW_SOC_DEBUG_HOOKS
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56726/comment/1d9f6480_7cb1175d
PS4, Line 9: The intent of this Kconfig is, when selected, to compile in any
: Intel-specific hooks that may be available.
> Also, do we want this to be Intel-specific config or should this be a common config in src/Kconfig that can be used as required by each SoC?
Well, I asked Tim to move it into soc/intel (see above). It seems to be a very Intel-specific mechanism so I think that's appropriate? I still don't really know what exactly "debug hooks" refers to or how this "crashlog" works but it seems to be a completely vendor-specific mechanism?
At least for any of the SoCs I've been working on I wouldn't know what "enabling debug hooks" could mean, so that leads me to think this doesn't belong in src/Kconfig.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0e7f076bb7e3b630c3ab9d28071312d3e21e927
Gerrit-Change-Number: 56726
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 10 Aug 2021 20:53:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Boris Mittelberg.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56840 )
Change subject: mb/google/dedede: allow MKBP devices
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56840/comment/dff001ed_62f4fa46
PS3, Line 9: MKBP
Alright, thanks for the information. How about adding the acronym expansion between parentheses?
> Enable MKBP (Matrix Keyboard Protocol) interface for all dedede ...
--
To view, visit https://review.coreboot.org/c/coreboot/+/56840
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9d1f43e4dd56318af4c1d5f5c1c3a2c237a05c5f
Gerrit-Change-Number: 56840
Gerrit-PatchSet: 3
Gerrit-Owner: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 20:23:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Boris Mittelberg <bmbm(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56867 )
Change subject: soc/intel/tgl: Allow setting PCIe subsystem IDs after FSP-S
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56867
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I391b9fd0dc9dda925c1c8fe52bff153fe044d73e
Gerrit-Change-Number: 56867
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 10 Aug 2021 19:48:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Julius Werner, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56726 )
Change subject: soc/intel: Add new Kconfig INTEL_ALLOW_SOC_DEBUG_HOOKS
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56726/comment/49798b78_b46c15ee
PS4, Line 9: The intent of this Kconfig is, when selected, to compile in any
: Intel-specific hooks that may be available.
If the intent of the Kconfig is to compile in any Intel-specific hooks, then shouldn't the SoCs add something like:
```
config INTEL_ALLOW_SOC_DEBUG_HOOKS
select X
select Y
```
rather than
```
config X
depends on INTEL_ALLOW_SOC_DEBUG_HOOKS
```
In the follow up CLs, it looks like the config is used to establish a dependency of SoC-specific configs on INTEL_ALLOW_SOC_DEBUG_HOOKS so that the SoC-specific hooks are available to be selected only when INTEL_ALLOW_SOC_DEBUG_HOOKS is selected.
Also, do we want this to be Intel-specific config or should this be a common config in src/Kconfig that can be used as required by each SoC?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0e7f076bb7e3b630c3ab9d28071312d3e21e927
Gerrit-Change-Number: 56726
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 10 Aug 2021 19:35:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Angel Pons.
Boris Mittelberg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56840 )
Change subject: mb/google/dedede: allow MKBP devices
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56840/comment/d167bdce_a6501359
PS3, Line 9: MKBP
> What does MKBP mean? I'd appreciate if this information could be added to the commit message
It stands for Matrix Keyboard Protocol - one of the 2 "keyboard" interfaces on chromeos devices. MKBP is more generic than 8042, and it allows non-keyboard events to be delivered from EC (Embedded Controller) to the host. Before this change 8042 was used for other events like Power button, Lid switch, etc, which are not really part of the keyboard.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56840
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9d1f43e4dd56318af4c1d5f5c1c3a2c237a05c5f
Gerrit-Change-Number: 56840
Gerrit-PatchSet: 3
Gerrit-Owner: Boris Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 19:30:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: EricR Lai.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56877 )
Change subject: drivers/generic/alc1015: Add HID to support alc1019
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56877/comment/11aa8247_4d4c1c07
PS2, Line 13: TEST=:
TEST=
Patchset:
PS1:
> Since kernel driver still use alc1015. Won't change the name.
Please provide a link (or commit hash) to the Linux kernel driver in the git commit message.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3e98297f3a39048b24d61e61ca95c60cd2037eb5
Gerrit-Change-Number: 56877
Gerrit-PatchSet: 2
Gerrit-Owner: EricR Lai <ericr_lai(a)compal.corp-partner.google.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: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 18:59:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph.
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/56727
to look at the new patch set (#4).
Change subject: {adl,tgl}: Make SOC_INTEL_CRASHLOG depend on INTEL_ALLOW_SOC_DEBUG_HOOKS
......................................................................
{adl,tgl}: Make SOC_INTEL_CRASHLOG depend on INTEL_ALLOW_SOC_DEBUG_HOOKS
The Crashlog feature is an SoC debug hook, therefore guard it with
INTEL_ALLOW_SOC_DEBUG_HOOKS.
BUG=b:190626785
Change-Id: Idc3ca88a8d4b665a4c954183757542cd9779066d
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
---
M src/mainboard/google/brya/Kconfig.name
M src/soc/intel/alderlake/Kconfig
M src/soc/intel/tigerlake/Kconfig
3 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/56727/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/56727
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idc3ca88a8d4b665a4c954183757542cd9779066d
Gerrit-Change-Number: 56727
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Idwer Vollering.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56677 )
Change subject: util/kconfig: detect ncurses on FreeBSD
......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS4:
> It’s not a big deal, as it can be easily reverted/improved, but as the long term goal is to reduce o […]
Sorry Paul, I thought we were done discussing things after Idwer's comment about how it works to compile the linux kernel. It sat for a day with all of the comments closed before I submitted.
Feel free to revert if you want. I really don't see the issue with this change, but I didn't mean to merge it before we were done with discussions.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56677
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4344ba2116c0b8618357db4248d993509cbb666e
Gerrit-Change-Number: 56677
Gerrit-PatchSet: 6
Gerrit-Owner: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 17:04:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin Roth <martinroth(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment