coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: soc/intel/../thermal: Add support for thermal config behind PMC device
by Subrata Banik (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59209 ) Change subject: soc/intel/../thermal: Add support for thermal config behind PMC device ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/common/block/thermal/thermal.c: https://review.coreboot.org/c/coreboot/+/59209/comment/097907a8_d856fe46 PS8, Line 112: struct pmc_thermal_config { : void (*func)(uintptr_t addr, uint32_t data); : uint16_t offset; : uint32_t value; : } config[] = { : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_CTEN, : .value = PMC_PWRM_THERMAL_CTEN_CPDEN | PMC_PWRM_THERMAL_CTEN_CTENLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_ECRPTEN, : .value = PMC_PWRM_THERMAL_ECRPTEN_EN_RPT : | PMC_PWRM_THERMAL_ECRPTEN_ECRPTENLOCK, : }, : { : .func = write32p, : .offset = PMC_PWRM_THERMAL_TL, : .value = pch_get_ltt_value(dev) | PMC_PWRM_THERMAL_TL_TTEN : | PMC_PWRM_THERMAL_TL_TLLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_PHLC, : .value = PMC_PWRM_THERMAL_PHLC_PHLCLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_TLEN, : .value = PMC_PWRM_THERMAL_TLEN_TLENLOCK, : }, : }; > @Tim, any further thoughts ? Ping! Tim -- To view, visit https://review.coreboot.org/c/coreboot/+/59209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c6ae72610da39fc18ff252c440d006e83c570a0 Gerrit-Change-Number: 59209 Gerrit-PatchSet: 11 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 19:00:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add firmware-power-managed DSD property
by build bot (Jenkins) (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59479 ) Change subject: drivers/tpm: Add firmware-power-managed DSD property ...................................................................... Patch Set 4: (1 comment) File src/drivers/i2c/tpm/chip.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-133831): https://review.coreboot.org/c/coreboot/+/59479/comment/12699c41_c531eada PS4, Line 53: switch(config->power_managed_mode) { space required before the open parenthesis '(' -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 4 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 19:00:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add firmware-power-managed DSD property
by Rob Barnes (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian. Hello build bot (Jenkins), Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59479 to look at the new patch set (#4). Change subject: drivers/tpm: Add firmware-power-managed DSD property ...................................................................... drivers/tpm: Add firmware-power-managed DSD property Introduce firmware-power-managed DSD ACPI property for TPM devices. This property can be checked by the kernel TPM driver to override how the TPM power states are managed. This is a tri-state flag, true, false, or unset. So an enum used to keep the flag is unset by default. When firmware-power-managed is true, the kernel driver will not send a shutdown during s2idle/s0i3 suspend. BUG=b:200578885 BRANCH=None TEST=Shutdown is triggered on s0ix suspend on guybrush with patched kernel Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M src/drivers/i2c/tpm/chip.c M src/drivers/i2c/tpm/chip.h 2 files changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/59479/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 4 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/../thermal: Add support for thermal config behind PMC device
by Subrata Banik (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59209 ) Change subject: soc/intel/../thermal: Add support for thermal config behind PMC device ...................................................................... Patch Set 11: (1 comment) File src/soc/intel/common/block/thermal/thermal.c: https://review.coreboot.org/c/coreboot/+/59209/comment/ff22101a_d0a27657 PS8, Line 106: /* Enable thermal sensor power management using PMC PCH device */ : static void pch_pmc_thermal_configuration(void) : { : struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); : uintptr_t pmc_bar = soc_read_pmc_base(); : : struct pmc_thermal_config { : void (*func)(uintptr_t addr, uint32_t data); : uint16_t offset; : uint32_t value; : } config[] = { : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_CTEN, : .value = PMC_PWRM_THERMAL_CTEN_CPDEN | PMC_PWRM_THERMAL_CTEN_CTENLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_ECRPTEN, : .value = PMC_PWRM_THERMAL_ECRPTEN_EN_RPT : | PMC_PWRM_THERMAL_ECRPTEN_ECRPTENLOCK, : }, : { : .func = write32p, : .offset = PMC_PWRM_THERMAL_TL, : .value = pch_get_ltt_value(dev) | PMC_PWRM_THERMAL_TL_TTEN : | PMC_PWRM_THERMAL_TL_TLLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_PHLC, : .value = PMC_PWRM_THERMAL_PHLC_PHLCLOCK, : }, : { : .func = thermal_or32, : .offset = PMC_PWRM_THERMAL_TLEN, : .value = PMC_PWRM_THERMAL_TLEN_TLENLOCK, : }, : }; : : for (int i = 0; i < ARRAY_SIZE(config); i++) : config[i].func(pmc_bar + config[i].offset, config[i].value); : } : > I am really not liking the #define PCH_DEVFN_THERMAL DEADCODE thing... […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/59209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c6ae72610da39fc18ff252c440d006e83c570a0 Gerrit-Change-Number: 59209 Gerrit-PatchSet: 11 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 18:59:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/../thermal: Drop unused `dev` parameter in pch_get_ltt_value()
by Subrata Banik (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Felix Singer, Maulik V Vaghela, Tim Wawrzynczak, Angel Pons, Patrick Rudolph, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59390 ) Change subject: soc/intel/../thermal: Drop unused `dev` parameter in pch_get_ltt_value() ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/59390/comment/7804697b_de301d76 PS3, Line 7: /../ > I suggest leave it along, we can see the full path from the diff. true, i just want to avoid writing `soc/intel/thermal` because there in no such path actually like that. But I'm open for suggestion. -- To view, visit https://review.coreboot.org/c/coreboot/+/59390 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iecdf6f6c3023f896a27e212d7c59b2030a3fd116 Gerrit-Change-Number: 59390 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 18:59:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamp: Add new helper functions
by Julius Werner (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Bora Guvendik, Furquan Shaikh, Selma Bensaid, Tim Wawrzynczak, Subrata Banik, Angel Pons, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51445 ) Change subject: timestamp: Add new helper functions ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/51445/comment/d892396b_b8c88b80 PS3, Line 7: Add helper fucntions > I guess we have other options as well to create 2 cbmem tables No, we definitely don't want two separate timestamp tables, it should all be withing one, internally consistent table. The "rewind" method definitely works for that, but I think my suggestion (just using negative timestamps) would also work and be simpler. > Additionally, unlike some other SoC platform, where #1 can be collected as early after CPU reset, is not likely the case with IA platform. Rather when we are able to collect #1 (typically, at early romstage state or late might be), we already have #2 available. Right, but that shouldn't make a difference. You can add timestamps whenever you want. Note that timestamps do not need to appear in chronological order in the timestamp table... it's perfectly legal for the CSE timestamps to be added to the table after the bootblock timestamps, the cbmem utility will just sort them before printing. > I kind of get what your are suggesting but I would let Bora to share a sample timestamp for #1 and #2 above to illustrate the scenario. Want to make sure we have exact timestamp value appearing at base rather delta which may not be the exact while doing (Tn - T(n-1)) = negative number. I don't really understand what you mean here, unfortunately. Note that timestamps in the table are always stored relative (i.e. as a delta) to the base timestamp. For the CSE timestamps, in the approach I'm suggesting, that delta would simply be negative. -- To view, visit https://review.coreboot.org/c/coreboot/+/51445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6b7065ed26e231fc898ae44bcc15cba6fb42b308 Gerrit-Change-Number: 51445 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Fri, 19 Nov 2021 18:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com> Comment-In-Reply-To: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add firmware-power-managed DSD property
by Rob Barnes (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59479 ) Change subject: drivers/tpm: Add firmware-power-managed DSD property ...................................................................... Patch Set 3: (1 comment) File src/drivers/i2c/tpm/chip.c: https://review.coreboot.org/c/coreboot/+/59479/comment/f8a21eae_68ff3f07 PS2, Line 53: suspend > Technically it can apply to both S3 and S0i3. […] Switched to "firmware-power-managed" flag. -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 3 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 18:33:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Rob Barnes <robbarnes(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add firmware-power-managed DSD property
by Rob Barnes (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian. Hello Andrey Pronin, build bot (Jenkins), Raul Rangel, Christian Walter, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/59479 to look at the new patch set (#3). Change subject: drivers/tpm: Add firmware-power-managed DSD property ...................................................................... drivers/tpm: Add firmware-power-managed DSD property Introduce firmware-power-managed DSD ACPI property for TPM devices. This property can be checked by the kernel TPM driver to override how the TPM power states are managed. This is a tri-state flag, true, false, or unset. So an enum used to keep the flag is unset by default. When firmware-power-managed is true, the kernel driver will not send a shutdown during s2idle/s0i3 suspend. BUG=b:200578885 BRANCH=None TEST=Shutdown is triggered on s0ix suspend on guybrush with patched kernel Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M src/drivers/i2c/tpm/chip.c M src/drivers/i2c/tpm/chip.h 2 files changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/59479/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 3 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: Documentation: Add some notes about how to integrate FSP
by Patrick Georgi (Code Review) 19 Nov '21

19 Nov '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58886 ) Change subject: Documentation: Add some notes about how to integrate FSP ...................................................................... Documentation: Add some notes about how to integrate FSP While we don't _want_ FSP, we can't get around it sometimes. But when using it, we can still try to establish best practices to make life easier for everybody. Change-Id: I4efd273e4141dc6dc4cf8bdebda9cffd0d7cc1a1 Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58886 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M Documentation/soc/intel/fsp/index.md 1 file changed, 12 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved diff --git a/Documentation/soc/intel/fsp/index.md b/Documentation/soc/intel/fsp/index.md index 912c44b..feeb5e9 100644 --- a/Documentation/soc/intel/fsp/index.md +++ b/Documentation/soc/intel/fsp/index.md @@ -2,6 +2,18 @@ This section contains documentation about Intel-FSP in public domain. +## Integration Guidelines + +Some guiding principles when working on the glue to integrate FSP into +coreboot, e.g. on how to configure a board in devicetree when that affects +the way FSP works: + +* It should be possible to replace FSP based boot with a native coreboot + implementation for a given chipset without touching the mainboard code. +* The devicetree configures coreboot and part of what coreboot does with the + information is setting some FSP UPDs. The devicetree isn't supposed to + directly configure FSP. + ## Bugs As Intel doesn't even list known bugs, they are collected here until those are fixed. If possible a workaround is described here as well. -- To view, visit https://review.coreboot.org/c/coreboot/+/58886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4efd273e4141dc6dc4cf8bdebda9cffd0d7cc1a1 Gerrit-Change-Number: 58886 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/genesyslogic/gl9750: Add driver for Genesys Logic GL9750
by Paul Menzel (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Ben Chuang. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59429 ) Change subject: drivers/genesyslogic/gl9750: Add driver for Genesys Logic GL9750 ...................................................................... Patch Set 1: (2 comments) File src/drivers/genesyslogic/gl9750/gl9750.c: https://review.coreboot.org/c/coreboot/+/59429/comment/12b67356_7ff07c7b PS1, Line 22: /* Disable ASPM L0s support */ Please add a comment, why this needs to be disabled. File src/include/device/pci_ids.h: https://review.coreboot.org/c/coreboot/+/59429/comment/b344f066_8d666245 PS1, Line 2069: #define PCI_DEVICE_ID_GLI_9750 0x9750 Please sort the list. -- To view, visit https://review.coreboot.org/c/coreboot/+/59429 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6d60cef41baade7457a159d3ce2f8d2e6b66e71c Gerrit-Change-Number: 59429 Gerrit-PatchSet: 1 Gerrit-Owner: Ben Chuang <benchuanggli(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Greg Tu <greg.tu(a)genesyslogic.com.tw> Gerrit-CC: HsuanYang Chen <ynop77(a)gmail.com> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Ben Chuang <benchuanggli(a)gmail.com> Gerrit-Comment-Date: Fri, 19 Nov 2021 18:14:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021