Attention is currently required from: Mario Scheithauer, Sean Rhodes.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75852?usp=email )
Change subject: soc/intel/apollolake: Switch to snake case for SataPortsHotPlug
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/75852?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8fc8b30ac2c182ffaf2dee37e0116e27071b6a2c
Gerrit-Change-Number: 75852
Gerrit-PatchSet: 1
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 22:16:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer, Sean Rhodes, Werner Zeh.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75820?usp=email )
Change subject: soc/intel/apollolake: Fix FSP SATA speed limit configuraion
......................................................................
Patch Set 7:
(3 comments)
Patchset:
PS7:
Is there anything on which the SATA speed depends on or what could limit it?
File src/soc/intel/apollolake/ahci.c:
https://review.coreboot.org/c/coreboot/+/75820/comment/11040ce8_865b1a1a :
PS7, Line 12: if (speed == SATA_DEFAULT)
:
nit: I wouldn't care about default values and just write the bits in any case.
File src/soc/intel/apollolake/include/soc/ahci.h:
https://review.coreboot.org/c/coreboot/+/75820/comment/f62be4b8_9d93031b :
PS7, Line 11: 0x00F00000
make it lowercase
--
To view, visit https://review.coreboot.org/c/coreboot/+/75820?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6f55f40941fa618e7de13a5cefe9e17ae34c5c99
Gerrit-Change-Number: 75820
Gerrit-PatchSet: 7
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jan Samek <jan.samek(a)siemens.com>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 22:12:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Jeff Daly, Lance Zhao, Matt DeVillier, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75729?usp=email )
Change subject: UNTESTED acpi: Add ECAM region below PNP0C02 device in SSDT
......................................................................
Patch Set 3:
(1 comment)
File src/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/75729/comment/8d08f99b_09a6e04d :
PS2, Line 587: struct device *dev = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
: assert(dev != NULL);
> > Anyway, a different thought: This is probably not the right place to decide […]
Hmmm, looks like we always set `.ops` for the domain? Technically, it's like
any other pointer in there. The difference is that the others are already
set (all 21 times). Even if it feels 21 lines too much, it would also feel
odd to handle this one pointer differently. Or maybe I misunderstand.
That dummy device you have in mind, could that be added by sconfig?
Runtime code seems overkill.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75729?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I852e393726a1b086cf582f4d2d707e7cde05cbf4
Gerrit-Change-Number: 75729
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 15 Jun 2023 22:05:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Martin L Roth, Nico Huber, Patrick Rudolph, Paul Menzel, Tim Chu, Ziang Wang.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75740?usp=email )
Change subject: soc/intel/xeon_sp/spr: Add weak method to get memory capability
......................................................................
Patch Set 10:
(1 comment)
File src/soc/intel/xeon_sp/spr/romstage.c:
https://review.coreboot.org/c/coreboot/+/75740/comment/0ed89d7b_a4daba5f :
PS10, Line 45: on SKUs with persistent memory support
So, there is already a capability or an indication in hardware whether memory capacity is 4TB or 6TB? Why don't we read that during runtime and return the appropriate value?
--
To view, visit https://review.coreboot.org/c/coreboot/+/75740?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieb892d3b148291b35808d634d63b58b90d456767
Gerrit-Change-Number: 75740
Gerrit-PatchSet: 10
Gerrit-Owner: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 21:47:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Martin L Roth, Nico Huber, Patrick Rudolph, Paul Menzel, Tim Chu, Ziang Wang.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75740?usp=email )
Change subject: soc/intel/xeon_sp/spr: Add weak method to get memory capability
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
> Why not use a devicetree register setting or Kconfig option for this value instead? A weak function […]
As I understand it, memory capacity is CPU specific. I think it should be automatically configured by detecting the CPU model. It's not mainboard specific and so devicetree feels like the wrong place. Kconfig option might be confusing when you have the wrong CPU model.
--
To view, visit https://review.coreboot.org/c/coreboot/+/75740?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieb892d3b148291b35808d634d63b58b90d456767
Gerrit-Change-Number: 75740
Gerrit-PatchSet: 10
Gerrit-Owner: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Reviewer: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 21:40:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Raul Rangel, Subrata Banik.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58421?usp=email )
Change subject: ChromeOS: Move CRHW device object
......................................................................
Patch Set 12:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58421/comment/5abae073_2685bbd6 :
PS12, Line 15: Change
> Have you tested this on real hardware? overall LGTM
I don't run ChromeOS, nor have I patched its kernel the way TimW described it would be necessary.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58421?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I449d0206fbaee94cea99f745cf297ad293e8306c
Gerrit-Change-Number: 58421
Gerrit-PatchSet: 12
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 21:29:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org>
Gerrit-MessageType: comment