coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[S] Change in coreboot[main]: vendorcode/google/chromeos: Use unsigned int for "factory_config"
by Julius Werner (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Eric Lai, Kapil Porwal, Shelley Chen, Subrata Banik. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79769?usp=email ) Change subject: vendorcode/google/chromeos: Use unsigned int for "factory_config" ...................................................................... Patch Set 3: (1 comment) File src/vendorcode/google/chromeos/chromeos.h: https://review.coreboot.org/c/coreboot/+/79769/comment/1d98e418_2bd73f21 : PS3, Line 20: CHROMEOS_ERROR > > Please name this something that more clearly connects it to factory config, this name is too gener […] But it is not something that should be used more widely? It is specifically only for functions that return a 64-bit unsigned bitfield and want to encode an error in there, and you need to pick a value that you know cannot be a valid representation for those bit fields, so this kind of error value pretty much always needs to be tied to the specific function. Other functions should generally be using `enum cb_err` to return errors, or maybe they return a size_t and use 0 to represent errors, or any of those other more normal error conventions. -- To view, visit https://review.coreboot.org/c/coreboot/+/79769?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3021b8646de4750b4c8e2a2981f42500894fa2d0 Gerrit-Change-Number: 79769 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Thu, 04 Jan 2024 17:24:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF
by Matt DeVillier (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79826?usp=email ) Change subject: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79826?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id976734c64efe7e0c3d8b073c8009849be291241 Gerrit-Change-Number: 79826 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Jan 2024 17:16:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/common/emmc: add Kconfig option to skip powering off eMMC
by Matt DeVillier (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79825?usp=email ) Change subject: soc/amd/common/emmc: add Kconfig option to skip powering off eMMC ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79825?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0dbe819222972d9bf0789671b031ad83648e8917 Gerrit-Change-Number: 79825 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Jan 2024 17:15:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF
by Felix Held (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier. Hello Fred Reitberger, Jason Glenesk, Matt DeVillier, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79826?usp=email to look at the new patch set (#2). Change subject: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF ...................................................................... soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF Commit 850b6c6254ab ("soc/amd/picasso: add eMMC MMIO device to devicetree") broke both S3 resume on Morphius SKUs that use an NVMe SSD instead of an eMMC and boot on the currently out-of-tree ASRock X370 Killer SLI board. In the latter case, commenting out the power_off_aoac_device call inside the emmc_enable function fixed things. TEST=This fixes S3 resume on Morphius with NVMe SSD and an equivalent change discussed in the patch mentioned above that caused the regression also fixed boot on the ASRock board. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Change-Id: Id976734c64efe7e0c3d8b073c8009849be291241 --- M src/soc/amd/picasso/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/79826/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79826?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id976734c64efe7e0c3d8b073c8009849be291241 Gerrit-Change-Number: 79826 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: vendorcode/google/chromeos: Use unsigned int for "factory_config"
by Subrata Banik (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Eric Lai, Julius Werner, Kapil Porwal, Shelley Chen. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79769?usp=email ) Change subject: vendorcode/google/chromeos: Use unsigned int for "factory_config" ...................................................................... Patch Set 3: (1 comment) File src/vendorcode/google/chromeos/chromeos.h: https://review.coreboot.org/c/coreboot/+/79769/comment/40895840_c70fd955 : PS3, Line 20: CHROMEOS_ERROR > Please name this something that more clearly connects it to factory config, this name is too generic (e.g. UNDEFINED_FACTORY_CONFIG). i thought of using it more widely rather just limited to FACTORY_CONFIG hence, using this as CHROMEOS_ERROR. WDYT ? -- To view, visit https://review.coreboot.org/c/coreboot/+/79769?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3021b8646de4750b4c8e2a2981f42500894fa2d0 Gerrit-Change-Number: 79769 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Thu, 04 Jan 2024 16:51:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/amd/picasso: add eMMC MMIO device to devicetree
by Felix Held (Code Review) 04 Jan '24

04 Jan '24

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77990?usp=email ) Change subject: soc/amd/picasso: add eMMC MMIO device to devicetree ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > just for the record, I told you on libera but for any readers: I'm not using any blobs from the vend […] talked to Matt and it seems that this also broke s3 resume on morphius with nvme ssd instead of an emmc, so that's maybe a more general issue for picasso. CB:79825 and CB:79826 might fix this; will have Matt verify that on Morphius -- To view, visit https://review.coreboot.org/c/coreboot/+/77990?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5453b69df776d2ce1f3be11e37cd26c8c64f0cd5 Gerrit-Change-Number: 77990 Gerrit-PatchSet: 6 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Marty E. Plummer <hanetzer(a)startmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 04 Jan 2024 16:41:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Marty E. Plummer <hanetzer(a)startmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF
by Felix Held (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Fred Reitberger, Jason Glenesk, Matt DeVillier. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79826?usp=email ) Change subject: soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF ...................................................................... soc/amd/picasso/Kconfig: select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF Commit 850b6c6254ab ("soc/amd/picasso: add eMMC MMIO device to devicetree") broke both S3 resume on Morphius SKUs that use an NVMe SSD instead of an eMMC and boot on the currently out-of-tree ASRock X370 Killer SLI board. In the latter case, commenting out the power_off_aoac_device call inside the emmc_enable function fixed things. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Id976734c64efe7e0c3d8b073c8009849be291241 --- M src/soc/amd/picasso/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/79826/1 diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 796fe4e..ee5e130 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -38,6 +38,7 @@ select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION select SOC_AMD_COMMON_BLOCK_EMMC + select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF select SOC_AMD_COMMON_BLOCK_GRAPHICS select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_HDA -- To view, visit https://review.coreboot.org/c/coreboot/+/79826?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id976734c64efe7e0c3d8b073c8009849be291241 Gerrit-Change-Number: 79826 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: soc/amd/common/emmc: add Kconfig option to skip powering off eMMC
by Felix Held (Code Review) 04 Jan '24

04 Jan '24

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79825?usp=email ) Change subject: soc/amd/common/emmc: add Kconfig option to skip powering off eMMC ...................................................................... soc/amd/common/emmc: add Kconfig option to skip powering off eMMC Add a Kconfig option to skip powering off the eMMC controller via the AOAC block in the case where the eMMC controller is disabled in the devicetree. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I0dbe819222972d9bf0789671b031ad83648e8917 --- M src/soc/amd/common/block/emmc/Kconfig M src/soc/amd/common/block/emmc/emmc.c 2 files changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/79825/1 diff --git a/src/soc/amd/common/block/emmc/Kconfig b/src/soc/amd/common/block/emmc/Kconfig index 96bbada..6316841 100644 --- a/src/soc/amd/common/block/emmc/Kconfig +++ b/src/soc/amd/common/block/emmc/Kconfig @@ -2,3 +2,10 @@ bool help Select this option to use AMD common EMMC driver support. + +config SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF + bool + depends on SOC_AMD_COMMON_BLOCK_EMMC + help + Select this option to try to switch off the power of the eMMC + controller via the AOAC registers. diff --git a/src/soc/amd/common/block/emmc/emmc.c b/src/soc/amd/common/block/emmc/emmc.c index 09d2350..9242d71 100644 --- a/src/soc/amd/common/block/emmc/emmc.c +++ b/src/soc/amd/common/block/emmc/emmc.c @@ -12,7 +12,7 @@ static void emmc_enable(struct device *dev) { - if (!dev->enabled) + if (!dev->enabled && !CONFIG(SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF)) power_off_aoac_device(FCH_AOAC_DEV_EMMC); } -- To view, visit https://review.coreboot.org/c/coreboot/+/79825?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0dbe819222972d9bf0789671b031ad83648e8917 Gerrit-Change-Number: 79825 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: vendorcode/google/chromeos: Use unsigned int for "factory_config"
by Julius Werner (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Eric Lai, Kapil Porwal, Shelley Chen, Subrata Banik. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79769?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: vendorcode/google/chromeos: Use unsigned int for "factory_config" ...................................................................... Patch Set 3: Code-Review+2 (2 comments) Patchset: PS3: LGTM after one issue File src/vendorcode/google/chromeos/chromeos.h: https://review.coreboot.org/c/coreboot/+/79769/comment/3ec83106_3690f203 : PS3, Line 20: CHROMEOS_ERROR Please name this something that more clearly connects it to factory config, this name is too generic (e.g. UNDEFINED_FACTORY_CONFIG). -- To view, visit https://review.coreboot.org/c/coreboot/+/79769?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3021b8646de4750b4c8e2a2981f42500894fa2d0 Gerrit-Change-Number: 79769 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Thu, 04 Jan 2024 16:35:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: vendorcode/google/chromeos: Use unsigned int for "factory_config"
by Tim Van Patten (Code Review) 04 Jan '24

04 Jan '24

Attention is currently required from: Eric Lai, Julius Werner, Kapil Porwal, Shelley Chen, Subrata Banik. Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79769?usp=email ) Change subject: vendorcode/google/chromeos: Use unsigned int for "factory_config" ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/79769?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3021b8646de4750b4c8e2a2981f42500894fa2d0 Gerrit-Change-Number: 79769 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Thu, 04 Jan 2024 16:29:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024