coreboot-gerrit October 2023

coreboot-gerrit@coreboot.org

1 participants
3353 discussions

[L] Change in coreboot[main]: clang-format: Update configuration for version 16+
by Martin L Roth (Code Review) 01 Nov '23

01 Nov '23

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78832?usp=email ) Change subject: clang-format: Update configuration for version 16+ ...................................................................... clang-format: Update configuration for version 16+ As we look at unifying the format of coreboot code (/src, excluding src/vendorcode), we need a code-beautifier configuration that works well with the coreboot style. This patch is an attempt to match the existing code styles as much as possible. There are going to be some trade-offs in any code formatter. Tables which have been hand-formatted probably won't look as good. These can be specifically marked to be excluded from the formatter, however this should be the exception, not the rule. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I70341d77e167c145f447594b6b0bef628cea83c6 --- M .clang-format 1 file changed, 236 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/78832/1 diff --git a/.clang-format b/.clang-format index 5c8aa3c..3be6f3f 100644 --- a/.clang-format +++ b/.clang-format @@ -1,21 +1,236 @@ -BasedOnStyle: LLVM -Language: Cpp -IndentWidth: 8 -UseTab: Always -BreakBeforeBraces: Linux -AllowShortIfStatementsOnASingleLine: false -IndentCaseLabels: false -SortIncludes: false -ContinuationIndentWidth: 8 -ColumnLimit: 96 -AlwaysBreakBeforeMultilineStrings: true -AllowShortLoopsOnASingleLine: false -AllowShortFunctionsOnASingleLine: false -AlignEscapedNewlinesLeft: false -AlignTrailingComments: true -AllowAllParametersOfDeclarationOnNextLine: false -AlignAfterOpenBracket: true -SpaceAfterCStyleCast: false -MaxEmptyLinesToKeep: 2 -BreakBeforeBinaryOperators: NonAssignment -BreakStringLiterals: false +# SPDX-License-Identifier: GPL-2.0 +# +# clang-format configuration file. Intended for clang-format >= 16. +# +# For more information, see: +# +# https://clang.llvm.org/docs/ClangFormat.html +# https://clang.llvm.org/docs/ClangFormatStyleOptions.html +# https://clang-format-configurator.site/ +# + +--- +Language: Cpp +AccessModifierOffset: -4 +AlignAfterOpenBracket: Align +AlignArrayOfStructures: None +AlignConsecutiveAssignments: + Enabled: false + AcrossEmptyLines: false + AcrossComments: true + AlignCompound: false + PadOperators: true +AlignConsecutiveBitFields: + Enabled: true + AcrossEmptyLines: false + AcrossComments: false + AlignCompound: false + PadOperators: true +AlignConsecutiveDeclarations: + Enabled: false + AcrossEmptyLines: false + AcrossComments: false + AlignCompound: false + PadOperators: true +AlignConsecutiveMacros: + Enabled: true + AcrossEmptyLines: false + AcrossComments: false + AlignCompound: false + PadOperators: true +AlignEscapedNewlines: Left +AlignOperands: Align +AlignTrailingComments: + Kind: Always + OverEmptyLines: 0 +AllowAllArgumentsOnNextLine: true +AllowAllParametersOfDeclarationOnNextLine: false +AllowShortBlocksOnASingleLine: Never +AllowShortCaseLabelsOnASingleLine: false +AllowShortEnumsOnASingleLine: true +AllowShortFunctionsOnASingleLine: None +AllowShortIfStatementsOnASingleLine: Never +AllowShortLambdasOnASingleLine: All +AllowShortLoopsOnASingleLine: false +AlwaysBreakAfterDefinitionReturnType: None +AlwaysBreakAfterReturnType: None +AlwaysBreakBeforeMultilineStrings: false +AlwaysBreakTemplateDeclarations: MultiLine + +# git grep -h '^#define [^[:space:]]*__.*[^[:space:]]*__attribute__' | grep -v vendorcode | sed "s,^#define $[^[:space:]]*__[^([:space:]]*$.*$, - '\1'," | LC_ALL=C sort -u +AttributeMacros: + - '__aligned' + - '__always_inline' + - '__always_unused' + - '__cpu_driver' + - '__fallthrough' + - '__maybe_unused' + - '__must_check' + - '__noreturn' + - '__packed' + - '__pci_driver' + - '__printf' + - '__weak' +BinPackArguments: true +BinPackParameters: true +BitFieldColonSpacing: Both +BraceWrapping: + AfterCaseLabel: false + AfterClass: false + AfterControlStatement: Never + AfterEnum: false + AfterExternBlock: false + AfterFunction: true + AfterNamespace: true + AfterObjCDeclaration: false + AfterStruct: false + AfterUnion: false + BeforeCatch: false + BeforeElse: false + BeforeLambdaBody: false + BeforeWhile: false + IndentBraces: false + SplitEmptyFunction: true + SplitEmptyRecord: true + SplitEmptyNamespace: true +BreakAfterAttributes: Never +BreakAfterJavaFieldAnnotations: false +BreakArrays: false +BreakBeforeBinaryOperators: None +BreakBeforeConceptDeclarations: Always +BreakBeforeBraces: Custom +BreakBeforeInlineASMColon: OnlyMultiline +BreakBeforeTernaryOperators: false +BreakConstructorInitializers: AfterColon +BreakInheritanceList: AfterColon +BreakStringLiterals: false +ColumnLimit: 96 +CommentPragmas: '^ IWYU pragma:' +CompactNamespaces: false +ConstructorInitializerIndentWidth: 8 +ContinuationIndentWidth: 16 +Cpp11BracedListStyle: true +DerivePointerAlignment: false +DisableFormat: false +EmptyLineAfterAccessModifier: Never +EmptyLineBeforeAccessModifier: LogicalBlock +ExperimentalAutoDetectBinPacking: false +FixNamespaceComments: false + +# git grep -h '^#define [^[:space:]]*for_each[^[:space:]]*(' | grep -v vendorcode | sed "s,^#define $[^[:space:]]*for_each[^[:space:]]*$(.*$, - '\1'," | LC_ALL=C sort -u +ForEachMacros: + - 'expr_list_for_each_sym' + - 'list_for_each' + - 'list_for_each_entry' + - 'list_for_each_entry_safe' + +# git grep -ih '^#define \+if[^[:space:]]*(' | grep -v vendorcode | sed "s,^#define $[^[:space:]]*if[^[:space:]]*$(.*$, - '\1',I" | grep -v IFIX | LC_ALL=C sort -u +IfMacros: + - 'IF_BIT_0' + - 'IF_BIT_0_CHECK' + - 'IF_CHANNEL_POPULATED' + - 'IF_DIMM_POPULATED' + - 'IF_RANK_POPULATED' + - 'IfBit0' + - 'if_EXT_COLORS' + - 'if_USE_SCROLL_HINTS' + - 'if_WIDEC' +IncludeBlocks: Preserve +IncludeIsMainSourceRegex: '' +IndentAccessModifiers: false +IndentCaseBlocks: false +IndentCaseLabels: false +IndentExternBlock: AfterExternBlock +IndentGotoLabels: false +IndentPPDirectives: None +IndentRequiresClause: true +IndentWidth: 8 +IndentWrappedFunctionNames: false +InsertBraces: false +InsertNewlineAtEOF: true +InsertTrailingCommas: None +IntegerLiteralSeparator: + Binary: 0 + BinaryMinDigits: 0 + Decimal: 0 + DecimalMinDigits: 0 + Hex: 0 + HexMinDigits: 0 +JavaScriptQuotes: Leave +JavaScriptWrapImports: true +KeepEmptyLinesAtTheStartOfBlocks: false +LambdaBodyIndentation: Signature +LineEnding: LF +MacroBlockBegin: '' +MacroBlockEnd: '' +MaxEmptyLinesToKeep: 1 +NamespaceIndentation: None +ObjCBinPackProtocolList: Auto +ObjCBlockIndentWidth: 8 +ObjCBreakBeforeNestedBlockParam: true +ObjCSpaceAfterProperty: true +ObjCSpaceBeforeProtocolList: true +PackConstructorInitializers: BinPack +PenaltyBreakAssignment: 10 +PenaltyBreakBeforeFirstCallParameter: 30 +PenaltyBreakComment: 10 +PenaltyBreakFirstLessLess: 0 +PenaltyBreakOpenParenthesis: 0 +PenaltyBreakString: 10 +PenaltyBreakTemplateDeclaration: 10 +PenaltyExcessCharacter: 100 +PenaltyIndentedWhitespace: 0 +PenaltyReturnTypeOnItsOwnLine: 60 +PointerAlignment: Right +PPIndentWidth: -1 +QualifierAlignment: Left +ReferenceAlignment: Pointer +ReflowComments: false +RemoveBracesLLVM: false +RemoveSemicolon: false +RequiresClausePosition: OwnLine +RequiresExpressionIndentation: OuterScope +SeparateDefinitionBlocks: Leave +ShortNamespaceLines: 1 +SortIncludes: Never +SortJavaStaticImport: Before +SortUsingDeclarations: Never +SpaceAfterCStyleCast: false +SpaceAfterLogicalNot: false +SpaceAfterTemplateKeyword: true +SpaceAroundPointerQualifiers: Default +SpaceBeforeAssignmentOperators: true +SpaceBeforeCaseColon: false +SpaceBeforeCpp11BracedList: false +SpaceBeforeCtorInitializerColon: true +SpaceBeforeInheritanceColon: true +SpaceBeforeParens: ControlStatementsExceptControlMacros +SpaceBeforeParensOptions: + AfterControlStatements: true + AfterForeachMacros: false + AfterFunctionDefinitionName: false + AfterFunctionDeclarationName: false + AfterIfMacros: false + AfterOverloadedOperator: false + AfterRequiresInClause: false + AfterRequiresInExpression: false + BeforeNonEmptyParentheses: false +SpaceBeforeRangeBasedForLoopColon: true +SpaceBeforeSquareBrackets: false +SpaceInEmptyBlock: false +SpaceInEmptyParentheses: false +SpacesBeforeTrailingComments: 1 +SpacesInAngles: Never +SpacesInConditionalStatement: false +SpacesInContainerLiterals: false +SpacesInCStyleCastParentheses: false +SpacesInLineCommentPrefix: + Minimum: 1 + Maximum: 1 +SpacesInParentheses: false +SpacesInSquareBrackets: false +Standard: c++17 +TabWidth: 8 +UseTab: Always +... + -- To view, visit https://review.coreboot.org/c/coreboot/+/78832?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: I70341d77e167c145f447594b6b0bef628cea83c6 Gerrit-Change-Number: 78832 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: vboot: Add catchall recovery reason for unspecified phase 4 errors
by Julius Werner (Code Review) 31 Oct '23

31 Oct '23

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78821?usp=email ) Change subject: vboot: Add catchall recovery reason for unspecified phase 4 errors ...................................................................... vboot: Add catchall recovery reason for unspecified phase 4 errors The code for "phase 4" of firmware verification currently only sets a recovery reason when there's an actual hash mismatch detected in vb2api_check_hash_get_digest(). This is the most likely way how this section of code can fail but not the only one. If any other unexpected issue occurs, we should still set a recovery reason rather than just reboot and risk an infinite boot loop. This patch adds a catchall recovery reason for any error code that falls out of this block of code. If a more specific recovery reason had already been set beforehand, we'll continue to use that -- if not, we'll set VB2_RECOVERY_FW_GET_FW_BODY. Change-Id: If00f8f8a5d17aa113e0325aad58d367f244aca49 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78821 Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/security/vboot/vboot_logic.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c index 11983b9..93a188c 100644 --- a/src/security/vboot/vboot_logic.c +++ b/src/security/vboot/vboot_logic.c @@ -374,7 +374,7 @@ } if (rv) - vboot_save_and_reboot(ctx, rv); + vboot_fail_and_reboot(ctx, VB2_RECOVERY_FW_GET_FW_BODY, rv); vboot_save_data(ctx); /* Only extend PCRs once on boot. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/78821?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: If00f8f8a5d17aa113e0325aad58d367f244aca49 Gerrit-Change-Number: 78821 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: Update vboot submodule to upstream main
by Julius Werner (Code Review) 31 Oct '23

31 Oct '23

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78820?usp=email ) Change subject: Update vboot submodule to upstream main ...................................................................... Update vboot submodule to upstream main Updating from commit id 24cb127a: 2023-08-21 Nicholas Bishop sign_uefi_unittest.py: Fix long-line lint to commit id f2b01bf0: 2023-10-27 Julius Werner firmware: Undeprecate VB2_RECOVERY_FW_GET_FW_BODY This brings in 47 new commits: f2b01bf0 firmware: Undeprecate VB2_RECOVERY_FW_GET_FW_BODY ef6d02df futility/vb2_helper: Add missing newline for error messages 886d13d7 PRESUBMIT: switch to cros format ac2e1a75 host/lib: Decouple openssl headers from HOSTLIB 86ec05f7 futility: updater: Add help info for --quirks 2850244e futility: updater: Abort if the unlock_csme_* is used on a locked device f1b5c88d devkeys: delete old unused firmware_bmpfv.bin 4444c5fe crossystem: Fix tpm_fwver for fwid < 12935 98ef339f 2lib: Prevent overwriting the value of fw_vboot2 c7517eb4 make_dev_ssd: support ChromeOS Kdump 8e3462cc tlcl: Increase the TPM_BUFFER_SIZE 740a2966 vboot_reference: Drop 'host' usage for 'internal' in flashrom.h 57877a44 vboot: Remove comments about physical dev switch 3401d16c 2lib: Fix typos, comments and formats fdf52d45 scripts/: Drop deprecated {g,s}et_gbb_flags.sh scripts bf76e9ee 2lib: Output the correct kernel_version 1ac4663e make_dev_firmware.sh: update pattern for matching wp status c57ab9f7 2lib: Add recovery reason VB2_RECOVERY_WIDEVINE_PREPARE e094ba31 tlcl: Reduce the variants of TPM2B b047600d sign_official_build: support key config for pkcs11 f8712b73 vboot: support signing with pkcs11 private key 17fe786f strip_boot_from_image.sh: sfill fast 6c856cd3 futility/updater: Fix EC software write protection logic 1dc5a421 futility: update: Deprecate --unlock_me by --quirk unlock_csme_nissa f0d88587 futility: update: Refactor the 'unlock ME' quirk(s) 81429ee9 futility: update: Do not update RO when the AP RO is locked a3beb737 futility: update: Revise the ordering or quirks 2c1844fa futility: update: Remove unused quirk 'unlock_wilco_me_for_update' 75530d32 tests/futility: Test with new signer_config.csv based firmware updater cba649fa 2lib: Expose 2hmac ab015448 2lib: Refactor hmac to vb2_hmac_calculate 3545f8b4 Revert "sign_uefi: Remove exception catching" 55f625a9 dump_fmap: Add offset and size to flash_ec format output a27ee336 keygeneration: add shellcheck source statements to help linting 055f9aa2 keygeneration: replace_recovery_key.sh: make minios key optional 6cb8ab60 scripts: delete unused values kernel command line 1f76c38b vboot: Drop phone recovery support ccf6b037 scripts: Legacy fix for set_gbb_flags.sh 8f03069e futility: Add basic README.md 88963df8 utility: Query platform wp status with futility 6c3817d2 utility: Drop cros_alias technical debt in dev_debug_vboot df85f512 scripts: Drop cros_alias technical debt in make_dev_firmware.sh 7395cd68 futility/updater_utils.c: Match on EC path to prepare for split 52518415 crossystem: Recover corrupted RW_NVRAM on flash writes 81f9ddaf futility/cmd_gbb_utility.md: Add basic GBB subcmd doc c4995268 futility/: Fix define confusion 69dab5a6 crossystem: Avoid writing duplicate entries to RW_NVRAM 6c37b520 Revert "crossystem: stop supporting legacy chromeos_acpi driver" Change-Id: Ic7ecd1755d26df349b8abf1c5a77c806facfe1d8 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78820 Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Reviewed-by: Eric Lai <ericllai(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> --- M 3rdparty/vboot 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Yu-Ping Wu: Looks good to me, approved Eric Lai: Looks good to me, approved Felix Singer: Looks good to me, approved build bot (Jenkins): Verified diff --git a/3rdparty/vboot b/3rdparty/vboot index 24cb127..f2b01bf 160000 --- a/3rdparty/vboot +++ b/3rdparty/vboot @@ -1 +1 @@ -Subproject commit 24cb127a5e83a713131ab75cc39b11336019443c +Subproject commit f2b01bf08a813fb3035102c7c948b78fa191e544 -- To view, visit https://review.coreboot.org/c/coreboot/+/78820?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: Ic7ecd1755d26df349b8abf1c5a77c806facfe1d8 Gerrit-Change-Number: 78820 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: arch/arm64/arch_timer: Fix possible overflow in multiplication
by Julius Werner (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Paul Menzel, Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78800?usp=email ) Change subject: arch/arm64/arch_timer: Fix possible overflow in multiplication ...................................................................... Patch Set 2: (3 comments) File src/arch/arm64/arch_timer.c: https://review.coreboot.org/c/coreboot/+/78800/comment/eea52878_ae8cb645 : PS2, Line 16: assert(tfreq > 0) Missing semicolon (also, this is probably not necessary, you'll get a pretty obvious division-by-zero exception when this is not initialized anyway). https://review.coreboot.org/c/coreboot/+/78800/comment/088f888b_eaa045be : PS2, Line 18: div = gcd(mult, tfreq); nit: depending on the GCD implementation you'll probably want to write it as `gcd(tfreq, mult)` to help it terminate a little faster (in the common case where tfreq is a multiple of mult). https://review.coreboot.org/c/coreboot/+/78800/comment/107e6c51_75e29c0f : PS2, Line 11: static uint32_t tfreq, mult; : uint32_t div; : : if (tfreq == 0) { : tfreq = raw_read_cntfrq_el0(); : assert(tfreq > 0) : mult = 1000000; : div = gcd(mult, tfreq); : tfreq /= div; : mult /= div; : } : : long usecs = (tvalue * mult) / tfreq; > Nice, but why not just cast `tvalue`, and let the compiler figure out the rest? Is that possible? Not really sure what you want to cast it to? It's already 64-bit. The concern here is that even the 64-bit multiplication could overflow since the initialization value of CNTPCT is unknown. -- To view, visit https://review.coreboot.org/c/coreboot/+/78800?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: I366667de05392913150414f0fa9058725be71c52 Gerrit-Change-Number: 78800 Gerrit-PatchSet: 2 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Tue, 31 Oct 2023 22:25:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: lib: Add GCD function
by Julius Werner (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78798?usp=email ) Change subject: lib: Add GCD function ...................................................................... Patch Set 1: (2 comments) File src/lib/gcd.c: https://review.coreboot.org/c/coreboot/+/78798/comment/1b8583d5_6eb6b62e : PS1, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */ Since we need to solve the same problem in libpayload, please put this in commonlib/bsd, and use a GCD implementation that is BSD-licensed. (In the majority of cases, at least for the timer problem, we'll just be trying to find the GCD between 1MHz and a multiple of 1MHz. So we probably don't need the most sophisticated GCD algorithm in the world, the simple modulus one should be good enough, it'll only take one iteration.) https://review.coreboot.org/c/coreboot/+/78798/comment/8fb939cc_75e170da : PS1, Line 20: *a ^= *b; This hasn't been a good swap algorithm in 20+ years (see https://en.wikipedia.org/wiki/Register_renaming). Just use a temp variable, and I don't think this is worth factoring out into a separate function for a single use. Oh, actually, looks like we already have a SWAP() in commonlib/helpers.h (part of <stdlib.h>) that you can use. -- To view, visit https://review.coreboot.org/c/coreboot/+/78798?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: I21819cda4299b3809b8ca7a95cbdc6a87e4b3481 Gerrit-Change-Number: 78798 Gerrit-PatchSet: 1 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Tue, 31 Oct 2023 22:18:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/guybrush: Set PS2K_IRQ to level/low
by Mark Hasemeyer (Code Review) 31 Oct '23

31 Oct '23

1 0

[S] Change in coreboot[main]: mb/google/guybrush: Set PS2K_IRQ to level/low
by Mark Hasemeyer (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Jason Nien, Karthik Ramasubramanian, Martin L Roth, Paul Menzel. Mark Hasemeyer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78137?usp=email ) Change subject: mb/google/guybrush: Set PS2K_IRQ to level/low ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78137/comment/327dfaa9_ae990c85 : PS3, Line 12: From https://review.coreboot.org/c/coreboot/+/54924: > git operates with the hashes (74bce48f1d445053b217913df79a77dfe286e893), so I’d use that. Done https://review.coreboot.org/c/coreboot/+/78137/comment/90631dcc_bd8cb7f2 : PS3, Line 13: > The default state for the IRQ lines when the eSPI controller comes : > out of reset is high. This is because the IRQ lines are shared with : > the other IRQ sources using AND gates. This means that in order to : > not cause any spurious interrupts or miss any interrupts, the : > IO-APIC must use a low polarity trigger. > Remove the trailing two spaces? Did you mean the leading spaces? I removed them. -- To view, visit https://review.coreboot.org/c/coreboot/+/78137?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: I7d093d94a666263684645ef724e945069c68c806 Gerrit-Change-Number: 78137 Gerrit-PatchSet: 4 Gerrit-Owner: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 31 Oct 2023 22:16:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/guybrush: Set PS2K_IRQ to level/low
by Mark Hasemeyer (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Jason Nien, Karthik Ramasubramanian, Mark Hasemeyer, Martin L Roth. Hello Jason Nien, Karthik Ramasubramanian, Martin L Roth, Martin Roth, Raul Rangel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78137?usp=email to look at the new patch set (#4). Change subject: mb/google/guybrush: Set PS2K_IRQ to level/low ...................................................................... mb/google/guybrush: Set PS2K_IRQ to level/low On guybrush, keyboard presses are signaled by the EC via eSPI virtual wire. The interrupt is shared with others and should be active low. From 74bce48f1d4 ("mb/google/{zork,guybrush,skyrim},soc/amd/espi: Fix vw_irq_polarity"): > The default state for the IRQ lines when the eSPI controller comes > out of reset is high. This is because the IRQ lines are shared with > the other IRQ sources using AND gates. This means that in order to > not cause any spurious interrupts or miss any interrupts, the > IO-APIC must use a low polarity trigger. Setting `vw_irq_polarity` in the device tree provides an option to invert interrupts from the eSPI controller, but the register is initialized from verstage which is baked into RO. As a workaround, the necessary interrupts on the EC have been reconfigured to be active low, and we can modify the IO-APIC accordingly. EC related CL here: https://crrev.com/c/4891663 BUG=b:218874489 TEST=-`emerge-guybrush chromeos-ec coreboot chromeos-bootimage` -Flash new RW fw and verify keyboard is functional -`suspend_stress_test -c 1` and verify i8042 irq is removed as a wake source -`echo mem > /sys/power/state`. Press key and verify system wake from i8042. Cq-Depend: chromium:4891663 Change-Id: I7d093d94a666263684645ef724e945069c68c806 Signed-off-by: Mark Hasemeyer <markhas(a)google.com> --- M src/mainboard/google/guybrush/variants/baseboard/devicetree.cb M src/mainboard/google/guybrush/variants/baseboard/include/baseboard/ec.h 2 files changed, 5 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/78137/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/78137?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: I7d093d94a666263684645ef724e945069c68c806 Gerrit-Change-Number: 78137 Gerrit-PatchSet: 4 Gerrit-Owner: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: Documentation: add Nitrokey to distributions
by Markus Meissner (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Felix Singer, Michał Żygowski, Patrick Georgi, Paul Menzel. Markus Meissner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78299?usp=email ) Change subject: Documentation: add Nitrokey to distributions ...................................................................... Patch Set 5: (1 comment) File Documentation/distributions.md: PS4: > okok, will do done -> restored original order -- To view, visit https://review.coreboot.org/c/coreboot/+/78299?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: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Gerrit-Change-Number: 78299 Gerrit-PatchSet: 5 Gerrit-Owner: Markus Meissner Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 31 Oct 2023 21:44:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Markus Meissner Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Documentation: add Nitrokey to distributions
by Markus Meissner (Code Review) 31 Oct '23

31 Oct '23

Attention is currently required from: Felix Singer, Michał Żygowski, Patrick Georgi, Paul Menzel. Hello Michał Żygowski, Patrick Georgi, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78299?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Documentation: add Nitrokey to distributions ...................................................................... Documentation: add Nitrokey to distributions Nitrokey offers various products, which have coreboot pre-installed. Change-Id: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Signed-off-by: Markus Meissner <coder(a)safemailbox.de> --- M Documentation/distributions.md 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/78299/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/78299?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: I3ef25e0e1cb97eda5fd457bdb650f3ee3f00210a Gerrit-Change-Number: 78299 Gerrit-PatchSet: 5 Gerrit-Owner: Markus Meissner Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

← Newer
1
...
4
5
6
7
8
9
10
...
336
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2023