coreboot-gerrit June 2024

coreboot-gerrit@coreboot.org

1 participants
3527 discussions

[S] Change in coreboot[main]: i2c/drivers/generic: Add support for including a CDM
by Matt DeVillier (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Sean Rhodes. Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/81409?usp=email ) Change subject: i2c/drivers/generic: Add support for including a CDM ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: ACPI spec says _CDM is the clock domain, I can't find anything on MS' site about chip direct mapping - got a link you can include? File src/drivers/i2c/generic/chip.h: https://review.coreboot.org/c/coreboot/+/81409/comment/deac63b0_949b8646?us… : PS2, Line 86: in is? -- To view, visit https://review.coreboot.org/c/coreboot/+/81409?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If70c25288d835df7064b4051c43abeb2d6531f3b Gerrit-Change-Number: 81409 Gerrit-PatchSet: 2 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:49:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XL] Change in coreboot[main]: mb/starlabs/byte_adl: Add Alder Lake N Byte Mk II
by Matt DeVillier (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Sean Rhodes. Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/80705?usp=email ) Change subject: mb/starlabs/byte_adl: Add Alder Lake N Byte Mk II ...................................................................... Patch Set 7: Code-Review+1 (3 comments) File src/mainboard/starlabs/byte_adl/cmos.default: https://review.coreboot.org/c/coreboot/+/80705/comment/9381936c_2380622c?us… : PS7, Line 9: Balanced for a device which is always connected to external power, any reason to not use performance here? File src/mainboard/starlabs/byte_adl/variants/mk_ii/devicetree.cb: https://review.coreboot.org/c/coreboot/+/80705/comment/bca2112d_90d35455?us… : PS7, Line 4: SaGv_Enabled necessary/desired for a desktop device? setting to highest freq only should save some RAM training time https://review.coreboot.org/c/coreboot/+/80705/comment/d23a5170_ec790b4f?us… : PS7, Line 143: end EC interface isn't used for fan control etc? -- To view, visit https://review.coreboot.org/c/coreboot/+/80705?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idff2d883a8c29f0fee9d633708aac92061a45132 Gerrit-Change-Number: 80705 Gerrit-PatchSet: 7 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:44:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/intel/common: Add fpt_support to override CSME and HECI disables
by Matt DeVillier (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Sean Rhodes. Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/83265?usp=email ) Change subject: soc/intel/common: Add fpt_support to override CSME and HECI disables ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/83265/comment/63d8666a_b637b9e8?us… : PS1, Line 1328: me_state variable should be `cmos_me_state` -- To view, visit https://review.coreboot.org/c/coreboot/+/83265?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I695fa96c5057303730492139904dfcc1d989880b Gerrit-Change-Number: 83265 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:33:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow PL1/PL2 configuration
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Anastasios Koutian, Patrick Rudolph. Angel Pons has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83269?usp=email ) Change subject: cpu/intel/model_206ax: Allow PL1/PL2 configuration ...................................................................... Patch Set 1: (2 comments) File src/cpu/intel/model_206ax/chip.h: https://review.coreboot.org/c/coreboot/+/83269/comment/4bfe339f_4309048c?us… : PS1, Line 46: int pl1; /* Long-term power limit*/ : int pl2; /* Short-term power limit*/ > These are in microwatts. Agree that units should be made clear. […] Huh, that's a pretty small unit. Out of curiosity, any reason to use microwatts instead of watts? In any case, I'd suggest something like this: ```suggestion unsigned int pl1_uw; /* Long-term power limit, in microwatts */ unsigned int pl2_uw; /* Short-term power limit, in microwatts */ ``` File src/northbridge/intel/sandybridge/northbridge.c: https://review.coreboot.org/c/coreboot/+/83269/comment/efc11a45_c815f9c4?us… : PS1, Line 354: 28 > Yes, although it should be made clear that the lowest of the two power limits would apply when both […] Actually, this is the time parameter, Tau. I'm not sure if there's any benefit to configuring it, so I wouldn't worry. -- To view, visit https://review.coreboot.org/c/coreboot/+/83269?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I064af25ec4805fae755eea52c4c9c6d4386c0aee Gerrit-Change-Number: 83269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:27:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasios Koutian <akoutian2(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[L] Change in coreboot[main]: [WIP] OptiPlex 3050 port
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Felix Singer, Máté Kukri. Angel Pons has posted comments on this change by Máté Kukri. ( https://review.coreboot.org/c/coreboot/+/82053?usp=email ) Change subject: [WIP] OptiPlex 3050 port ...................................................................... Patch Set 11: (2 comments) File src/mainboard/dell/optiplex_3050/acpi/dptf.asl: PS11: > i think i saw a dmesg about it, but otherwise no. I would remove it then. I'm not sure if DPTF is of much use on desktops anyway. https://review.coreboot.org/c/coreboot/+/82053/comment/01e81f5b_f4921f2f?us… : PS11, Line 13: Package () { \_SB.PCI0.B0D4, \_SB.PCI0.B0D4, 100, 50, 0, 0, 0, 0 }, I mean... This device is not enabled in the devicetree. -- To view, visit https://review.coreboot.org/c/coreboot/+/82053?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8d443e39ee684a4eaa19c835a945cfe569c051e2 Gerrit-Change-Number: 82053 Gerrit-PatchSet: 11 Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Máté Kukri <kukri.mate(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:14:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Máté Kukri <kukri.mate(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow package power limit clamping
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Anastasios Koutian. Angel Pons has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83270?usp=email ) Change subject: cpu/intel/model_206ax: Allow package power limit clamping ...................................................................... Patch Set 2: (1 comment) File src/cpu/intel/model_206ax/model_206ax_init.c: https://review.coreboot.org/c/coreboot/+/83270/comment/790c7874_b980b590?us… : PS2, Line 160: if(conf->pl2_clamp) { > Sorry for these formatting errors, is there an auto-formatting hook that is used by coreboot? This i […] I guess you can run checkpatch locally and fix up the warnings before pushing. I think it should already run if you've ran `make gitconfig`. Note that you should fix formatting issues in the commits that introduced them, otherwise Jenkins will still complain. Yes, you have to amend the commits (this is perfectly fine when using Gerrit). -- To view, visit https://review.coreboot.org/c/coreboot/+/83270?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id0c0aedc29aca121d0fd1d8f8826089e13a026be Gerrit-Change-Number: 83270 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:12:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasios Koutian <akoutian2(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow PL1/PL2 configuration
by Nico Huber (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Angel Pons, Patrick Rudolph. Nico Huber has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83269?usp=email ) Change subject: cpu/intel/model_206ax: Allow PL1/PL2 configuration ...................................................................... Patch Set 1: (1 comment) File src/cpu/intel/model_206ax/model_206ax_init.c: https://review.coreboot.org/c/coreboot/+/83269/comment/dd67c44c_a558f02d?us… : PS1, Line 99: dev->upstream->dev->upstream->children->chip_info; > Uh, I'm confused. From chipset.cb: […] Yeah, you're right. I didn't know we model a single chip as two in the devicetree. -- To view, visit https://review.coreboot.org/c/coreboot/+/83269?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I064af25ec4805fae755eea52c4c9c6d4386c0aee Gerrit-Change-Number: 83269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:11:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow turbo boost ratio limit configuration
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Anastasios Koutian. Angel Pons has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83271?usp=email ) Change subject: cpu/intel/model_206ax: Allow turbo boost ratio limit configuration ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/83271/comment/c475a51d_f5184d09?us… : PS2, Line 9: Tested on ThinkPad T420 with the i7-3940XM. > This branch is intended to enable setting the values in devicetree.cb. […] Sounds good. I would suggest making a follow-up commit that adds the vendor defaults for the T420, since those values wouldn't change anyway. -- To view, visit https://review.coreboot.org/c/coreboot/+/83271?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1c65a129478e8ac2c4f66eb3c6aa2507358f82ad Gerrit-Change-Number: 83271 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:10:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasios Koutian <akoutian2(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow PL1/PL2 configuration
by Anastasios Koutian (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Angel Pons, Nico Huber, Patrick Rudolph. Anastasios Koutian has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83269?usp=email ) Change subject: cpu/intel/model_206ax: Allow PL1/PL2 configuration ...................................................................... Patch Set 1: (4 comments) File src/cpu/intel/model_206ax/chip.h: https://review.coreboot.org/c/coreboot/+/83269/comment/86de19a3_21f010be?us… : PS1, Line 46: int pl1; /* Long-term power limit*/ : int pl2; /* Short-term power limit*/ > nit: space before comment end […] These are in microwatts. Agree that units should be made clear. Could also add a comment as per lines 49-50 below. Also good idea about using unsigned. Perhaps we should do the same for current limits below, and the TCC offset above. File src/cpu/intel/model_206ax/model_206ax_init.c: https://review.coreboot.org/c/coreboot/+/83269/comment/7b1871cc_310a2766?us… : PS1, Line 99: dev->upstream->dev->upstream->children->chip_info; > `dev->chip_info` should work as well as it's all one chip, northbridge & CPU. Thanks, will use https://review.coreboot.org/c/coreboot/+/83269/comment/d03b5102_cf6292f8?us… : PS1, Line 138: dev_path(dev) > This is a bit confusing because it would print the northbridge's path from CPU code. Ack, will remove File src/northbridge/intel/sandybridge/northbridge.c: https://review.coreboot.org/c/coreboot/+/83269/comment/61257c86_6b3c9d1b?us… : PS1, Line 354: 28 > Would it make sense to turn this into another devicetree setting? Doesn't have to be in this change. Yes, although it should be made clear that the lowest of the two power limits would apply when both are specified. Can do on tip of branch. -- To view, visit https://review.coreboot.org/c/coreboot/+/83269?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I064af25ec4805fae755eea52c4c9c6d4386c0aee Gerrit-Change-Number: 83269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:08:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[S] Change in coreboot[main]: cpu/intel/model_206ax: Allow PL1/PL2 configuration
by Angel Pons (Code Review) 30 Jun '24

30 Jun '24

Attention is currently required from: Anastasios Koutian, Nico Huber, Patrick Rudolph. Angel Pons has posted comments on this change by Anastasios Koutian. ( https://review.coreboot.org/c/coreboot/+/83269?usp=email ) Change subject: cpu/intel/model_206ax: Allow PL1/PL2 configuration ...................................................................... Patch Set 1: (1 comment) File src/cpu/intel/model_206ax/model_206ax_init.c: https://review.coreboot.org/c/coreboot/+/83269/comment/9acfe911_240b8884?us… : PS1, Line 99: dev->upstream->dev->upstream->children->chip_info; > That being said, giving an alias to the `cpu_cluster` should allow it to be easily referenced. CB:83275 -- To view, visit https://review.coreboot.org/c/coreboot/+/83269?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I064af25ec4805fae755eea52c4c9c6d4386c0aee Gerrit-Change-Number: 83269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasios Koutian <akoutian2(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Sun, 30 Jun 2024 14:05:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

← Newer
1
2
3
4
5
6
7
8
9
...
353
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2024