coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: lib/cbfs,commonlib/mem_pool: Make cbfs_free take const pointer
by Raul Rangel (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Julius Werner. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56002 ) Change subject: lib/cbfs,commonlib/mem_pool: Make cbfs_free take const pointer ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > While pretty harmless, I'm not sure this is the right thing to do from a design purity point of view […] No technical reason. I just had a const pointer variable and wasn't able to free it. I can just drop the const in my code, or avoid calling free altogether. -- To view, visit https://review.coreboot.org/c/coreboot/+/56002 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie570825f9255d6499e977f6eace58589115e2552 Gerrit-Change-Number: 56002 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 02 Jul 2021 14:38:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Subrata Banik (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 ) Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... Patch Set 5: (2 comments) File src/soc/intel/common/block/cpu/car/cache_as_ram.S: https://review.coreboot.org/c/coreboot/+/51374/comment/22d750bc_d3c75e02 PS5, Line 520: mov %ecx, %edi > suggestion: […] Ack https://review.coreboot.org/c/coreboot/+/51374/comment/9eb88c4f_97aed9d3 PS5, Line 524: (1 << data_ways) > If that is how the register should be programmed, then yeah I guess so 😊 […] result of two below operations would be different 1. SF_MASK_1 = (1 << (SFWayCnt - data_ways)) - 1 2. SF_MASK_1 = (1 << SFWayCnt) - (1 << data_ways) - 1 Assume SFWayCnt = 0xC and data_ways = 1 then #1 SF_MASK_1 = ( 1 << (0xC - 1)) - 1 = ( 1 << 0xB) - 1 = 7ff #2 SF_MASK_1 = (1 << 0xC) - (1 << 0x1) - 1 = 0x1000 - 2 - 1 = 0xffd I guess #2 is correct, but I'm seeing a hang, Looking at cache document, I guess formula is as below. SF_MASK_1 = ((1 << SFWayCnt) - 1) - ((1 << data_ways) - 1) SF_MASK_1 = ((1 << 0xC) - 1) - ((1 << 0x1) - 1) = (0x1000 - 1) - (2 - 1) = 0xFFF - 1 = 0xFFE I need to test the code and update the CL again. Suddenly my remote board connect got drop. I will resume debug on Monday. Sorry for delay. -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 5 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: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.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: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Fri, 02 Jul 2021 14:37:01 +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> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
by Subrata Banik (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai. Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Nick Vaccaro, Patrick Rudolph, EricR Lai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51374 to look at the new patch set (#6). Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ...................................................................... soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 MSR IA_SF_QOS_INFO (0xc87) has been introduced since TGL is used to find out the NUM_SNOOP_FILTER_WAYS. Bit[5:0] of MSR 0xc87 indicates the maximum numbers of bits that may be set in any of the SF MASK register. Hence, this patch calculates SF way count to program SF Mask#1 using below logic: 1. Calcuate SFWayCnt = (MSR 0xC87) & 0x3f 2. Set SF_MASK_1 = ((1 << SFWayCnt) - 1) - ((1 << data_ways) - 1) Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- M src/include/cpu/x86/msr.h M src/soc/intel/common/block/cpu/car/cache_as_ram.S 2 files changed, 9 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/51374/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/51374 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941 Gerrit-Change-Number: 51374 Gerrit-PatchSet: 6 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: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.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: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/abuild: Fix overriding results with the default configuration
by Arthur Heymans (Code Review) 02 Jul '21

02 Jul '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56041 ) Change subject: util/abuild: Fix overriding results with the default configuration ...................................................................... util/abuild: Fix overriding results with the default configuration I a file in configs/* has no suffix, then the default configuration will override the results of the build generated by the configfile from configs/*. Fix this by adding a '_' to the buildname. Change-Id: Ic47105fafca41f1905a6569943079623bec5405a Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/abuild/abuild 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/56041/1 diff --git a/util/abuild/abuild b/util/abuild/abuild index c25584c..3965360 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -545,6 +545,13 @@ BUILD_NAME="${config##*/}" BUILD_NAME="${BUILD_NAME##config.}" BUILD_NAME=$(echo "${BUILD_NAME}" | tr '[:lower:]' '[:upper:]') + echo $BUILD_NAME $MAINBOARD + # If the file in configs/ results in the same build_name as the default config + # append a '_' to differentiate. Otherwise the default configuration would + # override the results. + if [ "${MAINBOARD}" = "${BUILD_NAME}" ]; then + BUILD_NAME=${BUILD_NAME}"_" + fi echo "Building config $BUILD_NAME" build_dir=$TARGET/${BUILD_NAME} build_config "$MAINBOARD" "$build_dir" "$BUILD_NAME" "$config" -- To view, visit https://review.coreboot.org/c/coreboot/+/56041 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic47105fafca41f1905a6569943079623bec5405a Gerrit-Change-Number: 56041 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: configs: Build test x86_64 on Sandy Bridge
by Patrick Rudolph (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Patrick Rudolph, Angel Pons, Arthur Heymans. Hello build bot (Jenkins), Angel Pons, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55472 to look at the new patch set (#11). Change subject: configs: Build test x86_64 on Sandy Bridge ...................................................................... configs: Build test x86_64 on Sandy Bridge Add defconfig to build test x86_64 code on Sandy Bridge. Change-Id: I2c18af8bfa87636c68741e4759059276c287d052 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- A configs/config.hp_compaq_8200_elite_sff_pc.x86_64 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/55472/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/55472 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2c18af8bfa87636c68741e4759059276c287d052 Gerrit-Change-Number: 55472 Gerrit-PatchSet: 11 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/spi: Update number of sectors details for winbond
by Paul Menzel (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Ravi kumar, Sajida Bhanu. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56038 ) Change subject: drivers/spi: Update number of sectors details for winbond ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/56038/comment/29a67d61_999e38eb PS1, Line 7: drivers/spi: Update number of sectors details for winbond Maybe: > Increase sector number to 14 for Winbond W25Q512NW-IM https://review.coreboot.org/c/coreboot/+/56038/comment/3a950e9b_6e3db477 PS1, Line 9: Update proper number of sectors info for winbond W25Q512NW-IM chip Where did you get the number from? Please mention the source, for example the datasheet name and revision. -- To view, visit https://review.coreboot.org/c/coreboot/+/56038 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12a22321bb9180e32cd47faa6ac3960ba5b2dfb8 Gerrit-Change-Number: 56038 Gerrit-PatchSet: 1 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Attention: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-Comment-Date: Fri, 02 Jul 2021 14:06:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [TESTONLY]See if Jenkins reacts differently.
by Arthur Heymans (Code Review) 02 Jul '21

02 Jul '21

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56040 ) Change subject: [TESTONLY]See if Jenkins reacts differently. ...................................................................... [TESTONLY]See if Jenkins reacts differently. Change-Id: Ia4fe574f114dac80b2eda77553dafb5b2a1b7049 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- R configs/config.pcengines_apu1_TEST 1 file changed, 0 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/56040/1 diff --git a/configs/config.pcengines_apu1 b/configs/config.pcengines_apu1_TEST similarity index 100% rename from configs/config.pcengines_apu1 rename to configs/config.pcengines_apu1_TEST -- To view, visit https://review.coreboot.org/c/coreboot/+/56040 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia4fe574f114dac80b2eda77553dafb5b2a1b7049 Gerrit-Change-Number: 56040 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: util/abuild: Check if buildtest configurations having invalid options
by Arthur Heymans (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Martin Roth, Arthur Heymans. Hello build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56037 to look at the new patch set (#4). Change subject: util/abuild: Check if buildtest configurations having invalid options ...................................................................... util/abuild: Check if buildtest configurations having invalid options Change-Id: I83a316dad31f8567fad68171b68ee546bfe548f0 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M util/abuild/abuild 1 file changed, 20 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/56037/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/56037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I83a316dad31f8567fad68171b68ee546bfe548f0 Gerrit-Change-Number: 56037 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: HACK: SKU2 Limit SPI NOR size to 8MB HACK
by Ravi kumar (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Sajida Bhanu. Hello Sajida Bhanu, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/56039 to review the following change. Change subject: HACK: SKU2 Limit SPI NOR size to 8MB HACK ...................................................................... HACK: SKU2 Limit SPI NOR size to 8MB HACK Limit SPI NOR size to 8MB to match with coreboot rom size by changing number of sectors. Change-Id: I2386f9c52677b263a972dc4041a2643d36130a1b Signed-off-by: Shaik Sajida Bhanu <sbhanu(a)codeaurora.org> --- M src/drivers/spi/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/56039/1 diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index f9abc43..9fe0492 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -193,7 +193,7 @@ { /* W25Q512NW-IM */ .id[0] = 0x8020, - .nr_sectors_shift = 14, + .nr_sectors_shift = 11, .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, -- To view, visit https://review.coreboot.org/c/coreboot/+/56039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2386f9c52677b263a972dc4041a2643d36130a1b Gerrit-Change-Number: 56039 Gerrit-PatchSet: 1 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-Attention: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/spi: Update number of sectors details for winbond
by Ravi kumar (Code Review) 02 Jul '21

02 Jul '21

Attention is currently required from: Sajida Bhanu. Hello Sajida Bhanu, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/56038 to review the following change. Change subject: drivers/spi: Update number of sectors details for winbond ...................................................................... drivers/spi: Update number of sectors details for winbond Update proper number of sectors info for winbond W25Q512NW-IM chip BUG=b:182963902 TEST=Validated on qualcomm sc7280 development board Change-Id: I12a22321bb9180e32cd47faa6ac3960ba5b2dfb8 Signed-off-by: Shaik Sajida Bhanu <sbhanu(a)codeaurora.org> --- M src/drivers/spi/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/56038/1 diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9fe0492..f9abc43 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -193,7 +193,7 @@ { /* W25Q512NW-IM */ .id[0] = 0x8020, - .nr_sectors_shift = 11, + .nr_sectors_shift = 14, .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, -- To view, visit https://review.coreboot.org/c/coreboot/+/56038 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I12a22321bb9180e32cd47faa6ac3960ba5b2dfb8 Gerrit-Change-Number: 56038 Gerrit-PatchSet: 1 Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org> Gerrit-Reviewer: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-Attention: Sajida Bhanu <sbhanu(a)codeaurora.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021