coreboot-gerrit April 2019

coreboot-gerrit@coreboot.org

1 participants
1219 discussions

Change in ...coreboot[master]: sdm845: Add QCLib to RomStage to perform IP init
by Julius Werner (Code Review) 15 Apr '19

15 Apr '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25208 ) Change subject: sdm845: Add QCLib to RomStage to perform IP init ...................................................................... Patch Set 72: (7 comments) Thanks for the update! The move to common looks good, but please note that there are still many older comments still applying to this version that need to be addressed. https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/include/so… File src/soc/qualcomm/common/include/soc/qclib_common.h: https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/include/so… PS72, Line 79: void soc_blob_load(void); Please prefix all external functions with qclib_ or something like that. https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/qclib.c File src/soc/qualcomm/common/qclib.c: https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/qclib.c@135 PS72, Line 135: printk(BIOS_INFO, "Board Rev(0x%x)\n", board_id()); This is already printed in ramstage once, do we need it here? https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/qclib.c@193 PS72, Line 193: Private IP That sounds a bit weird. Isn't DDR always going to be part of this (at least with the current code)? Let's just say "initialize DDR" because that's the most important part here. https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/common/qclib.c@209 PS72, Line 209: ret_code = doit(&qclib_cb_if_table, NULL); Now that we only have one argument, can we switch to using prog_run() for this? https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/sdm845/soc_blob_l… File src/soc/qualcomm/sdm845/soc_blob_load.c: https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/sdm845/soc_blob_l… PS72, Line 1: /* This should probably also be called qclib_execute.c to make clear that it belongs to the common file of the same name. https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/sdm845/soc_blob_l… PS72, Line 29: printk(BIOS_INFO, "soc_blob_load(sdm845)\n"); > Prefer using '"%s... […] This print is probably unneeded. https://review.coreboot.org/#/c/25208/72/src/soc/qualcomm/sdm845/soc_blob_l… PS72, Line 57: die("Couldn't run soc_blob_load.\n"); > Prefer using '"%s... […] Probably cleaner to return -1 from here and have all the die()s in the caller. -- To view, visit https://review.coreboot.org/c/coreboot/+/25208 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I073186674a1a593547d1ee1d15c7cd4fd8ad5bc1 Gerrit-Change-Number: 25208 Gerrit-PatchSet: 72 Gerrit-Owner: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: T Michael Turney <mturney(a)codeaurora.org> Gerrit-Reviewer: T.Michael Turney <tturne(a)codeaurora.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 15 Apr 2019 22:47:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/cannonlake: Implement soc side VMX support
by Furquan Shaikh (Code Review) 15 Apr '19

15 Apr '19

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31928 ) Change subject: soc/intel/cannonlake: Implement soc side VMX support ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31928 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I33dbffa6301afabd688080751ba3b85a43e00156 Gerrit-Change-Number: 31928 Gerrit-PatchSet: 7 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: RONAK KANABAR <ronak199323(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 15 Apr 2019 22:44:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src: include <assert.h> when appropriate
by Stefan Reinauer (Code Review) 15 Apr '19

15 Apr '19

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/26796 ) Change subject: src: include <assert.h> when appropriate ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/26796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib843eb7144b7dc2932931b9e8f3f1d816bcc1e1a Gerrit-Change-Number: 26796 Gerrit-PatchSet: 14 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 15 Apr 2019 22:32:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: google/kukui: Include panel identifier (LCM ID) into SKU ID
by Hung-Te Lin (Code Review) 15 Apr '19

15 Apr '19

Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32263 Change subject: google/kukui: Include panel identifier (LCM ID) into SKU ID ...................................................................... google/kukui: Include panel identifier (LCM ID) into SKU ID Kukui is using MIPI display panel and needs some identifier to tell payloads which panel is installed, and select right kernel device tree. Following Scarlet, the decision is to embed panel ID as part of SKU ID. BUG=b:129299873 TEST=make -j; boots on Kukui P2 unit. Change-Id: Ib0c00bc8ce3c71c445c5c4561403ce8ef4dd5844 Signed-off-by: Hung-Te Lin <hungte(a)chromium.org> --- M src/mainboard/google/kukui/boardid.c 1 file changed, 17 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/32263/1 diff --git a/src/mainboard/google/kukui/boardid.c b/src/mainboard/google/kukui/boardid.c index ff910fe..84a0663 100644 --- a/src/mainboard/google/kukui/boardid.c +++ b/src/mainboard/google/kukui/boardid.c @@ -66,24 +66,37 @@ uint32_t sku_id(void) { + uint32_t sku = BOARD_ID_INIT, lcm = BOARD_ID_INIT; static uint32_t cached_sku_id = BOARD_ID_INIT; + if (cached_sku_id != BOARD_ID_INIT) + return cached_sku_id; + /* On Flapjack, getting the SKU via CBI. */ if (CONFIG(BOARD_GOOGLE_FLAPJACK)) { - if (cached_sku_id == BOARD_ID_INIT && - google_chromeec_cbi_get_sku_id(&cached_sku_id)) + if (google_chromeec_cbi_get_sku_id(&cached_sku_id)) cached_sku_id = FLAPJACK_UNDEF_SKU_ID; return cached_sku_id; } /* Quirk for KUKUI: All P1/SKU0 had incorrectly set SKU=1. */ if (CONFIG(BOARD_GOOGLE_KUKUI)) { - if (cached_sku_id == BOARD_ID_INIT && board_id() == 1) { + if (board_id() == 1) { cached_sku_id = 0; return cached_sku_id; } } - return get_index(4, &cached_sku_id); + + /* + * The SKU is combined from: + * ADC2[4bit] = Panel ID (LCM_ID). + * ADC4[4bit] = SKU ID from board straps. + */ + get_index(2, &lcm); + get_index(4, &sku); + cached_sku_id = lcm << 4 || sku; + + return cached_sku_id; } uint32_t ram_code(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/32263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0c00bc8ce3c71c445c5c4561403ce8ef4dd5844 Gerrit-Change-Number: 32263 Gerrit-PatchSet: 1 Gerrit-Owner: Hung-Te Lin <hungte(a)chromium.org> Gerrit-MessageType: newchange

5 27

Change in ...coreboot[master]: cbfstool: Fix cbfs_file_next_attr
by Julius Werner (Code Review) 15 Apr '19

15 Apr '19

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31494 ) Change subject: cbfstool: Fix cbfs_file_next_attr ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31494 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I08df073158a0f285f96048c92aa8066fa4f57e6f Gerrit-Change-Number: 31494 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jens Drenhaus <jens.drenhaus(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kai Michaelis <kai.michaelis(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Roy Wen <rgzwen(a)arista.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 15 Apr 2019 20:25:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: timestamps: denote "end of romstage" right before loading ramstage
by Stefan Reinauer (Code Review) 15 Apr '19

15 Apr '19

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27329 ) Change subject: timestamps: denote "end of romstage" right before loading ramstage ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/#/c/27329/1/src/arch/x86/cbfs_and_run.c File src/arch/x86/cbfs_and_run.c: https://review.coreboot.org/#/c/27329/1/src/arch/x86/cbfs_and_run.c@22 PS1, Line 22: timestamp_add_now(TS_END_ROMSTAGE); should this go into run_ramstage instead, right before the jump? -- To view, visit https://review.coreboot.org/c/coreboot/+/27329 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3524a757f5274815d62a17c58a18ed6b9aa726bf Gerrit-Change-Number: 27329 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 15 Apr 2019 20:08:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: src: include <assert.h> when appropriate
by HAOUAS Elyes (Code Review) 15 Apr '19

15 Apr '19

Hello Patrick Rudolph, Aaron Durbin, Felix Held, Vanny E, Julius Werner, Philipp Deppenwiese, build bot (Jenkins), David Guckian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/26796 to look at the new patch set (#13). Change subject: src: include <assert.h> when appropriate ...................................................................... src: include <assert.h> when appropriate Change-Id: Ib843eb7144b7dc2932931b9e8f3f1d816bcc1e1a Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/arch/arm64/arm_tf.c M src/arch/x86/acpigen.c M src/arch/x86/include/arch/acpigen.h M src/commonlib/storage/pci_sdhci.c M src/commonlib/storage/sd.c M src/commonlib/storage/sd_mmc.c M src/commonlib/storage/sdhci.c M src/commonlib/storage/sdhci_adma.c M src/commonlib/storage/storage.c M src/cpu/intel/hyperthreading/intel_sibling.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c M src/drivers/spi/winbond.c M src/lib/gpio.c M src/mainboard/google/dragonegg/romstage_fsp_params.c M src/mainboard/intel/galileo/vboot.c M src/northbridge/amd/pi/00630F01/northbridge.c M src/security/vboot/vbnv_flash.c M src/soc/cavium/cn81xx/ecam0.c M src/soc/cavium/cn81xx/twsi.c M src/soc/cavium/common/ecam.c M src/soc/intel/apollolake/acpi.c M src/soc/intel/apollolake/gpio_apl.c M src/soc/intel/apollolake/gpio_glk.c M src/soc/intel/apollolake/uart.c M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/cannonlake/uart.c M src/soc/intel/common/block/sgx/sgx.c M src/soc/intel/common/block/uart/uart.c M src/soc/intel/denverton_ns/gpio.c M src/soc/intel/icelake/romstage/romstage.c M src/soc/intel/icelake/uart.c M src/soc/intel/quark/spi.c M src/soc/intel/skylake/cpu.c M src/soc/intel/skylake/gpio.c M src/soc/mediatek/common/include/soc/rtc_common.h M src/soc/mediatek/common/pmic_wrap.c M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c M src/soc/mediatek/mt8173/emi.c M src/soc/mediatek/mt8173/pmic_wrap.c M src/soc/mediatek/mt8183/mt6358.c M src/soc/qualcomm/ipq40xx/i2c.c M src/soc/qualcomm/ipq806x/i2c.c M src/soc/rockchip/common/edp.c M src/soc/rockchip/common/pwm.c M src/soc/rockchip/rk3288/hdmi.c M src/soc/rockchip/rk3288/tsadc.c M src/soc/rockchip/rk3399/bl31_plat_params.c M src/soc/rockchip/rk3399/mipi.c M src/soc/rockchip/rk3399/tsadc.c M src/soc/samsung/exynos5250/alternate_cbfs.c M src/soc/samsung/exynos5250/pinmux.c M src/soc/samsung/exynos5420/alternate_cbfs.c M src/soc/samsung/exynos5420/pinmux.c M src/superio/renesas/m3885x/superio.c M src/superio/smsc/lpc47n227/early_serial.c 56 files changed, 4 insertions(+), 53 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/26796/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/26796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib843eb7144b7dc2932931b9e8f3f1d816bcc1e1a Gerrit-Change-Number: 26796 Gerrit-PatchSet: 13 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: David Guckian <david.guckian(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: util/genbuild_h: Prefer iasl from .xcompile over a hard code
by Patrick Georgi (Code Review) 15 Apr '19

15 Apr '19

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32300 Change subject: util/genbuild_h: Prefer iasl from .xcompile over a hard code ...................................................................... util/genbuild_h: Prefer iasl from .xcompile over a hard code build.h provides iasl's version but right now assumes that it's kept in util/crossgcc/xgcc/bin. Often true, but not always, so use the one found in .xcompile to query the version as that's the version that is used in coreboot builds. Change-Id: Iaeedc22e0e14fa96b4f2a68127f405c7f0c9d5cc Signed-off-by: Patrick Georgi <pgeorgi(a)google.com> --- M util/genbuild_h/genbuild_h.sh 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/32300/1 diff --git a/util/genbuild_h/genbuild_h.sh b/util/genbuild_h/genbuild_h.sh index bafdbc9..c264b74 100755 --- a/util/genbuild_h/genbuild_h.sh +++ b/util/genbuild_h/genbuild_h.sh @@ -52,6 +52,9 @@ esac } +IASL=util/crossgcc/xgcc/bin/iasl +eval $(grep ^IASL:= .xcompile 2>/dev/null | sed s,:=,=,) + #Print out the information that goes into build.h printf "/* build system definitions (autogenerated) */\n" printf "#ifndef __BUILD_H\n" @@ -73,5 +76,5 @@ printf "#define COREBOOT_DMI_DATE \"$(our_date "$DATE" +%m/%d/%Y)\"\n" printf "\n" printf "#define COREBOOT_COMPILE_TIME \"$(our_date "$DATE" +%T)\"\n" -printf "#define ASL_VERSION 0x%d\n" `./util/crossgcc/xgcc/bin/iasl -v | grep version | sed 's/.*version //'` +printf "#define ASL_VERSION 0x%d\n" `$IASL -v | grep version | sed 's/.*version //'` printf "#endif\n" -- To view, visit https://review.coreboot.org/c/coreboot/+/32300 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaeedc22e0e14fa96b4f2a68127f405c7f0c9d5cc Gerrit-Change-Number: 32300 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 5

Change in ...coreboot[master]: cbfstool: Add ifittool
by build bot (Jenkins) (Code Review) 15 Apr '19

15 Apr '19

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31493 ) Change subject: cbfstool: Add ifittool ...................................................................... Patch Set 9: (21 comments) https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c File util/cbfstool/fit.c: https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@273 PS9, Line 273: if (!mcode_file) { braces {} are not necessary for single statement blocks https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@282 PS9, Line 282: ERROR("Couldn't allocate memory for microcode update entries.\n"); line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@392 PS9, Line 392: * bootblock, one Type 0x09 entry is needed to contain the failsafe hash. line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@531 PS9, Line 531: switch(fit->entries[i].type_checksum_valid) { switch and case should be at the same indent https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@531 PS9, Line 531: switch(fit->entries[i].type_checksum_valid) { space required before the open parenthesis '(' https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@574 PS9, Line 574: fit_entry_size_bytes(&fit->entries[i]) ); space prohibited before that close parenthesis ')' https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@604 PS9, Line 604: switch(type) { space required before the open parenthesis '(' https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@647 PS9, Line 647: switch(type) { space required before the open parenthesis '(' https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@773 PS9, Line 773: ERROR("No room, blob mcus = %zd, total entries = %d\n", line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c File util/cbfstool/ifittool.c: https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@85 PS9, Line 85: ERROR("Invalid topswap_size %zd, topswap can be 64K|128K|256K|512K|1M\n", line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@97 PS9, Line 97: static unsigned convert_to_from_absolute_top_aligned( Prefer 'unsigned int' to bare use of 'unsigned' https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@98 PS9, Line 98: const struct buffer *region, unsigned offset) Prefer 'unsigned int' to bare use of 'unsigned' https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@113 PS9, Line 113: static unsigned convert_to_from_top_aligned(const struct buffer *region, Prefer 'unsigned int' to bare use of 'unsigned' https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@114 PS9, Line 114: unsigned offset) Prefer 'unsigned int' to bare use of 'unsigned' https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@122 PS9, Line 122: if ((offset > 0) && (offset < region->size)) { braces {} are not necessary for single statement blocks https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@185 PS9, Line 185: ERROR("specified multiple operations at once\n"); line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@192 PS9, Line 192: ERROR("specified multiple operations at once\n"); line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@199 PS9, Line 199: ERROR("specified multiple operations at once\n"); line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@210 PS9, Line 210: ERROR("specified multiple operations at once\n"); line over 80 characters https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@224 PS9, Line 224: if (!*optarg || (suffix && *suffix)) { suspect code indent for conditional statements (24, 40) https://review.coreboot.org/#/c/31493/9/util/cbfstool/ifittool.c@267 PS9, Line 267: ERROR("Adding FIT entry, but no CBFS file / region name given\n"); line over 80 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/31493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0fe8cd70611d58823aca1147d5b830722ed72bd5 Gerrit-Change-Number: 31493 Gerrit-PatchSet: 9 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jens Drenhaus <jens.drenhaus(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kai Michaelis <kai.michaelis(a)9elements.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Roy Wen <rgzwen(a)arista.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 15 Apr 2019 14:19:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: cbfstool: Fix cbfs_file_next_attr
by Patrick Rudolph (Code Review) 15 Apr '19

15 Apr '19

Hello Amol N Sukerkar, Aaron Durbin, Roy Wen, Julius Werner, Subrata Banik, David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Kai Michaelis, Furquan Shaikh, Jens Drenhaus, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31494 to look at the new patch set (#8). Change subject: cbfstool: Fix cbfs_file_next_attr ...................................................................... cbfstool: Fix cbfs_file_next_attr The last attribute was never returned. Fix size compare to retrieve all attributes. Manually tested and seen all attributes, including the last one. Change-Id: I08df073158a0f285f96048c92aa8066fa4f57e6f Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> --- M util/cbfstool/cbfs_image.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/31494/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/31494 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I08df073158a0f285f96048c92aa8066fa4f57e6f Gerrit-Change-Number: 31494 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Amol N Sukerkar <amol.n.sukerkar(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jens Drenhaus <jens.drenhaus(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kai Michaelis <kai.michaelis(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Roy Wen <rgzwen(a)arista.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2019