Attention is currently required from: Arthur Heymans.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65880 )
Change subject: drivers/elog/gsmi.c: Fix compiling for 64bit
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/65880
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic8e04ae043145a3633c0b8379a797724f95fd7ea
Gerrit-Change-Number: 65880
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:42:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, V Sowmya, Nick Vaccaro, Usha P, Tim Wawrzynczak.
Selma Bensaid has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65987 )
Change subject: mb/intel/adlrvp: Enable Cr50 TPM over SPI for adlrvp_rpl
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/65987
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1b4119373f69954d620dc09e637a7571312a5fc1
Gerrit-Change-Number: 65987
Gerrit-PatchSet: 9
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: Usha P <usha.p(a)intel.com>
Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Usha P <usha.p(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:27:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jamie Ryu, Subrata Banik, Kapil Porwal, Ivy Jian, Angel Pons, Arthur Heymans, Lean Sheng Tan, Eric Lai, Raj Astekar.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67273 )
Change subject: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> > > > This seems like a bug in depthcharge then, as it seems to be making an assumption about the st […]
Fair point, I'll buy that. depthcharge indeed just uses the framebuffer straight from the coreboot table
--
To view, visit https://review.coreboot.org/c/coreboot/+/67273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ad9eee8379b7ea1e50224e3fabb347e5f14c25b
Gerrit-Change-Number: 67273
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:25:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Jamie Ryu, Tim Wawrzynczak, Kapil Porwal, Ivy Jian, Angel Pons, Arthur Heymans, Lean Sheng Tan, Eric Lai, Raj Astekar.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67273 )
Change subject: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> > > This seems like a bug in depthcharge then, as it seems to be making an assumption about the state of the HW (i.e. BM bit set) before it uses it? If depthcharge is using DMA then IMHO it is responsible for configuring it correctly, including setting BM.
> >
> > Isn't that the case with depthcharge always? I remember one issue in CML days with SATA when FSP missed to configure the BM and we were seeing booting issue. Finally, we are seeing the similar issue in MTL again with IGD. Its better we configure required setting in coreboot being independent of FSP is my intention here.
>
> We should consider the option of "why not just fix depthcharge" as well. That being said, if depthcharge doesn't "know" that this BM bit is not set (e.g. depthcharge tries to use the framebuffer through the info in coreboot tables and doesn't touch the PCI device), then I'd say coreboot should enable BM. However, in the case of SATA controllers, payloads often scan PCI devices to find SATA controllers, so setting the BM bit on these SATA controllers is the payload's responsibility.
I agree with Angel.
--
To view, visit https://review.coreboot.org/c/coreboot/+/67273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ad9eee8379b7ea1e50224e3fabb347e5f14c25b
Gerrit-Change-Number: 67273
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:21:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Jamie Ryu, Subrata Banik, Tim Wawrzynczak, Kapil Porwal, Ivy Jian, Arthur Heymans, Lean Sheng Tan, Eric Lai, Raj Astekar.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67273 )
Change subject: soc/intel/cmn/graphics: Use pci_dev_request_bus_master for BM enabling
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> > This seems like a bug in depthcharge then, as it seems to be making an assumption about the state of the HW (i.e. BM bit set) before it uses it? If depthcharge is using DMA then IMHO it is responsible for configuring it correctly, including setting BM.
>
> Isn't that the case with depthcharge always? I remember one issue in CML days with SATA when FSP missed to configure the BM and we were seeing booting issue. Finally, we are seeing the similar issue in MTL again with IGD. Its better we configure required setting in coreboot being independent of FSP is my intention here.
We should consider the option of "why not just fix depthcharge" as well. That being said, if depthcharge doesn't "know" that this BM bit is not set (e.g. depthcharge tries to use the framebuffer through the info in coreboot tables and doesn't touch the PCI device), then I'd say coreboot should enable BM. However, in the case of SATA controllers, payloads often scan PCI devices to find SATA controllers, so setting the BM bit on these SATA controllers is the payload's responsibility.
--
To view, visit https://review.coreboot.org/c/coreboot/+/67273
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9ad9eee8379b7ea1e50224e3fabb347e5f14c25b
Gerrit-Change-Number: 67273
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:17:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Jason Nien, EricKY Cheng, Martin Roth, Karthikeyan Ramasubramanian.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67209 )
Change subject: mb/google/skyrim/var/winterhold: Add gpio override settings
......................................................................
Patch Set 4:
(3 comments)
File src/mainboard/google/skyrim/variants/winterhold/gpio.c:
https://review.coreboot.org/c/coreboot/+/67209/comment/d6259396_c14c6932
PS4, Line 10: // NC
Remove these extra comments. The PAD_NC already makes it clear
https://review.coreboot.org/c/coreboot/+/67209/comment/81818e82_6d167530
PS4, Line 22: /* CLK_REQ1_L / EMMC */
How about this?
https://review.coreboot.org/c/coreboot/+/67209/comment/b5681c41_8949e17e
PS4, Line 25: __weak
You don't want to make this weak
--
To view, visit https://review.coreboot.org/c/coreboot/+/67209
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2086c326cbf46ba6378d18d37dcbbe9fafa6b2bc
Gerrit-Change-Number: 67209
Gerrit-PatchSet: 4
Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:03:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Nien, Matt DeVillier, Rob Barnes, Karthik Ramasubramanian.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67246 )
Change subject: mb/google/guybrush: enable display backlight in ramstage
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/67246/comment/04b357c5_50ebc524
PS1, Line 141: PAD_GPO
> Martin's recollection was that the ACPI code is required in order to have the backlight level restor […]
The pin seems to be in the S5 domain, so it should retain its value
--
To view, visit https://review.coreboot.org/c/coreboot/+/67246
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2519d779954ed89486045aa7de0b18f1c31a4374
Gerrit-Change-Number: 67246
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 01 Sep 2022 16:00:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-MessageType: comment