coreboot-gerrit June 2020

coreboot-gerrit@coreboot.org

1 participants
2826 discussions

Change in coreboot[master]: drivers/intel/fsp2_0: Add FSP 2.2 specific support
by Sridhar Siricilla (Code Review) 02 Jun '20

02 Jun '20

Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 ) Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support ...................................................................... Patch Set 3: (4 comments) https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/i… File src/drivers/intel/fsp2_0/include/fsp/util.h: https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/i… PS3, Line 31: number_of_phase nit; no_of_phases? or number_of_phases? https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/s… File src/drivers/intel/fsp2_0/silicon_init.c: https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/s… PS3, Line 27: >= FSP specification : * 2.2 nit: FSP specification >= 2.2 https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/s… PS3, Line 41: fsp_handle_reset(status); nit: Handling reset based on FSP status is an error scenario? https://review.coreboot.org/c/coreboot/+/41728/3/src/drivers/intel/fsp2_0/s… PS3, Line 140: post_code(POST_FSP_MULTI_PHASE_SI_INIT_ENTRY); nit: Should this be out of time measurement block? -- To view, visit https://review.coreboot.org/c/coreboot/+/41728 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If7177a267f3a9b4cbb60a639f1c737b9a3341913 Gerrit-Change-Number: 41728 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(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: Tue, 02 Jun 2020 07:22:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [NOTFORMERGE] cpu/x86/mtrr.h: ALIGN() fix
by Kyösti Mälkki (Code Review) 02 Jun '20

02 Jun '20

Hello Nico Huber, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/41819 to review the following change. Change subject: [NOTFORMERGE] cpu/x86/mtrr.h: ALIGN() fix ...................................................................... [NOTFORMERGE] cpu/x86/mtrr.h: ALIGN() fix Duplicate of CB:41785 Change-Id: Ic9023eceaf86143cd64f89489ba796b0983d39e9 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/include/cpu/x86/mtrr.h 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/41819/1 diff --git a/src/include/cpu/x86/mtrr.h b/src/include/cpu/x86/mtrr.h index 50148ff..301ea30 100644 --- a/src/include/cpu/x86/mtrr.h +++ b/src/include/cpu/x86/mtrr.h @@ -1,7 +1,6 @@ #ifndef CPU_X86_MTRR_H #define CPU_X86_MTRR_H -#include <commonlib/helpers.h> #ifndef __ASSEMBLER__ #include <cpu/x86/msr.h> #include <arch/cpu.h> @@ -187,8 +186,8 @@ /* Last but not least, most (if not all) chipsets have MMIO between 0xfe000000 and 0xff000000, so limit to 16MiB. */ -#if CAR_CACHE_ROM_SIZE >= 16 * MiB -# define CACHE_ROM_SIZE (16 * MiB) +#if CAR_CACHE_ROM_SIZE >= 16 << 20 +# define CACHE_ROM_SIZE (16 << 20) #else # define CACHE_ROM_SIZE CAR_CACHE_ROM_SIZE #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/41819 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic9023eceaf86143cd64f89489ba796b0983d39e9 Gerrit-Change-Number: 41819 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

1 1

Change in coreboot[master]: mb/google/dedede: config spk_en gpio to low by default
by Usha P (Code Review) 02 Jun '20

02 Jun '20

Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41853 ) Change subject: mb/google/dedede: config spk_en gpio to low by default ...................................................................... Patch Set 3: (5 comments) https://review.coreboot.org/c/coreboot/+/41853/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/41853/2//COMMIT_MSG@9 PS2, Line 9: gpio > speaker enable gpio Done https://review.coreboot.org/c/coreboot/+/41853/2//COMMIT_MSG@10 PS2, Line 10: so defaults it to 0 > configure it low? Done https://review.coreboot.org/c/coreboot/+/41853/2//COMMIT_MSG@12 PS2, Line 12: BRANCH=None > not needed Done https://review.coreboot.org/c/coreboot/+/41853/2//COMMIT_MSG@14 PS2, Line 14: tested > tested speaker playback works fine? Yes, speaker playback works. Updated the same. https://review.coreboot.org/c/coreboot/+/41853/2/src/mainboard/google/deded… File src/mainboard/google/dedede/variants/baseboard/gpio.c: https://review.coreboot.org/c/coreboot/+/41853/2/src/mainboard/google/deded… PS2, Line 178: GPP_D17 > This is exposed in ACPI tables only for waddledoo. […] Hi Furquan, for waddledee audio support is not enabled yet. Also since the Audio pin for waddledee is same as waddledoo (GPP_D17) it would be taken care. -- To view, visit https://review.coreboot.org/c/coreboot/+/41853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I070679457b06cb82633c1197b893a5d89c8b2cf0 Gerrit-Change-Number: 41853 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Divagar Mohandass <divagar.mohandass(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jun 2020 06:52:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede: config spk_en gpio to low by default
by Usha P (Code Review) 02 Jun '20

02 Jun '20

Hello Furquan Shaikh, Maulik V Vaghela, Tim Wawrzynczak, Aamir Bohra, Divagar Mohandass, Karthik Ramasubramanian, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/41853 to look at the new patch set (#3). Change subject: mb/google/dedede: config spk_en gpio to low by default ...................................................................... mb/google/dedede: config spk_en gpio to low by default The max98357a_platform_driver will turn on/off the speaker enable gpio based on use, so configure it low to save power. BUG=None TEST=Built dedede and tested speaker playback working. Change-Id: I070679457b06cb82633c1197b893a5d89c8b2cf0 Signed-off-by: Usha P <usha.p(a)intel.com> --- M src/mainboard/google/dedede/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/41853/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/41853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I070679457b06cb82633c1197b893a5d89c8b2cf0 Gerrit-Change-Number: 41853 Gerrit-PatchSet: 3 Gerrit-Owner: Usha P <usha.p(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Divagar Mohandass <divagar.mohandass(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: lib: Upgrade LZMA Decoder to version 19.0
by build bot (Jenkins) (Code Review) 02 Jun '20

02 Jun '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41963 ) Change subject: lib: Upgrade LZMA Decoder to version 19.0 ...................................................................... Patch Set 5: (77 comments) https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.h File src/lib/lzmadecode.h: https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.h@25 PS5, Line 25: #define RINOK(x) { int __result__ = (x); if (__result__ != 0) return __result__; } Macros with flow control statements should be avoided https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.h@143 PS5, Line 143: finished without end mark */ line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c File src/lib/lzmadecode.c: https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@18 PS5, Line 18: #define NORMALIZE if (range < kTopValue) { range <<= 8; code = (code << 8) | (*buf++); } Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@20 PS5, Line 20: #define IF_BIT_0(p) ttt = *(p); NORMALIZE; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@21 PS5, Line 21: bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) trailing statements should be on next line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@22 PS5, Line 22: #define UPDATE_0(p) range = bound; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@22 PS5, Line 22: #define UPDATE_0(p) range = bound; \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@24 PS5, Line 24: #define UPDATE_1(p) range -= bound; code -= bound; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@24 PS5, Line 24: #define UPDATE_1(p) range -= bound; code -= bound; \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@26 PS5, Line 26: #define GET_BIT2(p, i, A0, A1) IF_BIT_0(p) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@27 PS5, Line 27: {UPDATE_0(p); i = (i + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@28 PS5, Line 28: {UPDATE_1(p); i = (i + i) + 1; A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@32 PS5, Line 32: #define REV_BIT(p, i, A0, A1) IF_BIT_0(p + i) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@33 PS5, Line 33: {UPDATE_0(p + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@34 PS5, Line 34: {UPDATE_1(p + i); A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@36 PS5, Line 36: #define REV_BIT_CONST(p, i, m) REV_BIT(p, i, i += m;, i += m * 2;) space required after that ';' (ctx:VxO) https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@40 PS5, Line 40: {i = 1; do { TREE_GET_BIT(probs, i); } while (i < limit); i -= limit; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@43 PS5, Line 43: {i = 1; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@44 PS5, Line 44: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@45 PS5, Line 45: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@46 PS5, Line 46: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@47 PS5, Line 47: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@48 PS5, Line 48: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@49 PS5, Line 49: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@50 PS5, Line 50: i -= 0x40; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@54 PS5, Line 54: matchByte += matchByte; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@55 PS5, Line 55: bit = offs; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@56 PS5, Line 56: offs &= matchByte; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@57 PS5, Line 57: probLit = prob + (offs + bit + symbol); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@58 PS5, Line 58: GET_BIT2(probLit, symbol, offs ^= bit;, ;) please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@58 PS5, Line 58: GET_BIT2(probLit, symbol, offs ^= bit;, ;) space required after that ';' (ctx:VxO) https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@60 PS5, Line 60: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; \ Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@60 PS5, Line 60: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; \ Macros with flow control statements should be avoided https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@63 PS5, Line 63: #define IF_BIT_0_CHECK(p) ttt = *(p); NORMALIZE_CHECK; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@64 PS5, Line 64: bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) trailing statements should be on next line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@65 PS5, Line 65: #define UPDATE_0_CHECK range = bound; Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@65 PS5, Line 65: #define UPDATE_0_CHECK range = bound; macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@66 PS5, Line 66: #define UPDATE_1_CHECK range -= bound; code -= bound; Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@66 PS5, Line 66: #define UPDATE_1_CHECK range -= bound; code -= bound; macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@67 PS5, Line 67: #define GET_BIT2_CHECK(p, i, A0, A1) IF_BIT_0_CHECK(p) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@68 PS5, Line 68: {UPDATE_0_CHECK; i = (i + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@69 PS5, Line 69: {UPDATE_1_CHECK; i = (i + i) + 1; A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@72 PS5, Line 72: {i = 1; do { GET_BIT_CHECK(probs + i, i) } while (i < limit); i -= limit; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@75 PS5, Line 75: #define REV_BIT_CHECK(p, i, m) IF_BIT_0_CHECK(p + i) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@76 PS5, Line 76: {UPDATE_0_CHECK; i += m; m += m; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@77 PS5, Line 77: {UPDATE_1_CHECK; m += m; i += m; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@92 PS5, Line 92: unsigned int pbMask = ((unsigned int)1 << (p->prop.pb)) - 1; Unnecessary typecast of c90 int constant https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@94 PS5, Line 94: unsigned int lpMask = ((unsigned int)0x100 << p->prop.lp) - ((unsigned int)0x100 >> lc); Unnecessary typecast of c90 int constant https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@122 PS5, Line 122: dic[(dicPos == 0 ? dicBufSize : dicPos) - 1]) & lpMask) << lc); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@180 PS5, Line 180: dic[dicPos] = dic[dicPos - rep0 + (dicPos < rep0 ? dicBufSize : 0)]; line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@202 PS5, Line 202: } else { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@232 PS5, Line 232: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@240 PS5, Line 240: TREE_DECODE(probLen, (1 << kLenNumHighBits), len); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@249 PS5, Line 249: len : kNumLenToPosStates - 1) << kNumPosSlotBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@253 PS5, Line 253: unsigned int numDirectBits = (unsigned int)(((distance >> 1) - 1)); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@261 PS5, Line 261: do { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@263 PS5, Line 263: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@268 PS5, Line 268: do { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@274 PS5, Line 274: t = (0 - ((UInt32)code >> 31)); /* (UInt32)((Int32)code >> 31) */ line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@275 PS5, Line 275: distance = (distance << 1) + (t + 1); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@286 PS5, Line 286: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@297 PS5, Line 297: if (distance == (UInt32)0xFFFFFFFF) { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@310 PS5, Line 310: if (distance >= (checkDicSize == 0 ? processedPos : checkDicSize)) { line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@344 PS5, Line 344: if (++pos == dicBufSize) Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@458 PS5, Line 458: ((((p->processedPos) & ((1 << (p->prop.lp)) - 1)) << p->prop.lc) + line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@460 PS5, Line 460: p->dicBufSize : p->dicPos) - 1] >> (8 - p->prop.lc)))); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@464 PS5, Line 464: do { GET_BIT_CHECK(prob + symbol, symbol) } while (symbol < 0x100); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@477 PS5, Line 477: GET_BIT2_CHECK(probLit, symbol, offs ^= bit; , ;) space prohibited before that ',' (ctx:WxW) https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@543 PS5, Line 543: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@560 PS5, Line 560: ((len < kNumLenToPosStates - 1 ? len : kNumLenToPosStates - 1) << line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@569 PS5, Line 569: prob = probs + SpecPos + ((2 | (posSlot & 1)) << numDirectBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@575 PS5, Line 575: code -= range & (((code - range) >> 31) - 1); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@577 PS5, Line 577: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@586 PS5, Line 586: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@658 PS5, Line 658: while (p->remainLen != kMatchSpecLenStart) { suspect code indent for conditional statements (8, 24) https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@709 PS5, Line 709: int dummyRes = LzmaDec_TryDummy(p, p->tempBuf, (SizeT)rem); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/5/src/lib/lzmadecode.c@752 PS5, Line 752: else else is not generally useful after a break or return -- To view, visit https://review.coreboot.org/c/coreboot/+/41963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 Gerrit-Change-Number: 41963 Gerrit-PatchSet: 5 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jun 2020 04:56:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/volteer: Enable TCSS DMA0 and DMA1 for Volteer
by John Zhao (Code Review) 02 Jun '20

02 Jun '20

John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41387 ) Change subject: mb/google/volteer: Enable TCSS DMA0 and DMA1 for Volteer ...................................................................... mb/google/volteer: Enable TCSS DMA0 and DMA1 for Volteer This explicitly enables both of TCSS DMA0 and DMA1 controllers from Volteer devicetree.cb setting. BUG=:b:146624360 TEST=Built and booted on Volteer. Signed-off-by: John Zhao <john.zhao(a)intel.com> Change-Id: I05cc9e3964d8037d433fca443be6e8d5b444bbce --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/41387/1 diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index b689663..70f2968 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -125,8 +125,13 @@ # TCSS USB3 register "TcssXhciEn" = "1" + register "TcssXdciEn" = "0" register "TcssAuxOri" = "0" + # TCSS DMA + register "TcssDma0En" = "1" + register "TcssDma1En" = "1" + # DP port register "DdiPortAConfig" = "1" # eDP register "DdiPortBConfig" = "0" @@ -216,8 +221,8 @@ device pci 0a.0 off end # Crash-log SRAM 0x9A0D device pci 0d.0 on end # USB xHCI 0x9A13 device pci 0d.1 off end # USB xDCI (OTG) 0x9A15 - device pci 0d.2 off end # TBT DMA0 0x9A1B - device pci 0d.3 off end # TBT DMA1 0x9A1D + device pci 0d.2 on end # TBT DMA0 0x9A1B + device pci 0d.3 on end # TBT DMA1 0x9A1D device pci 0e.0 off end # VMD 0x9A0B # From PCH EDS(576591) -- To view, visit https://review.coreboot.org/c/coreboot/+/41387 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I05cc9e3964d8037d433fca443be6e8d5b444bbce Gerrit-Change-Number: 41387 Gerrit-PatchSet: 1 Gerrit-Owner: John Zhao <john.zhao(a)intel.com> Gerrit-MessageType: newchange

6 23

Change in coreboot[master]: lib: Upgrade LZMA Decoder to version 19.0
by Harshit Sharma (Code Review) 02 Jun '20

02 Jun '20

Hello build bot (Jenkins), Werner Zeh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/41963 to look at the new patch set (#5). Change subject: lib: Upgrade LZMA Decoder to version 19.0 ...................................................................... lib: Upgrade LZMA Decoder to version 19.0 This patch upgrades LZMA Decoder library to version 19.0. It copies the decompression code unmodified from LZMA SDK 19.0 at https://www.7-zip.org/sdk.html. However, it only includes those functions which are necessary for single-call decompression and leaves out the rest. LZMA interface has now changed a lot from the version 4.42. Earlier, there was a separate test application LzmaTest.c on which our lzmadecode wrapper was based. But now there is a dedicated library function LzmaDecode() for single-call decompressing. To support this newer interface, this patch also updates the lzmadecode wrapper ulzman(). Fixes: https://ticket.coreboot.org/issues/194 Signed-off-by: Harshit Sharma <harshitsharmajs(a)gmail.com> Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 --- M src/lib/lzma.c M src/lib/lzmadecode.c M src/lib/lzmadecode.h 3 files changed, 861 insertions(+), 430 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/41963/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/41963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 Gerrit-Change-Number: 41963 Gerrit-PatchSet: 5 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: lib: Upgrade LZMA Decoder to version 19.0
by build bot (Jenkins) (Code Review) 02 Jun '20

02 Jun '20

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41963 ) Change subject: lib: Upgrade LZMA Decoder to version 19.0 ...................................................................... Patch Set 4: (83 comments) https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.h File src/lib/lzmadecode.h: https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.h@25 PS4, Line 25: #define RINOK(x) { int __result__ = (x); if (__result__ != 0) return __result__; } Macros with flow control statements should be avoided https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.h@142 PS4, Line 142: LZMA_STATUS_MAYBE_FINISHED_WITHOUT_MARK /* there is probability that stream was trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.h@143 PS4, Line 143: finished without end mark */ line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c File src/lib/lzmadecode.c: https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@18 PS4, Line 18: #define NORMALIZE if (range < kTopValue) { range <<= 8; code = (code << 8) | (*buf++); } Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@20 PS4, Line 20: #define IF_BIT_0(p) ttt = *(p); NORMALIZE; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@21 PS4, Line 21: bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) trailing statements should be on next line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@22 PS4, Line 22: #define UPDATE_0(p) range = bound; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@22 PS4, Line 22: #define UPDATE_0(p) range = bound; \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@24 PS4, Line 24: #define UPDATE_1(p) range -= bound; code -= bound; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@24 PS4, Line 24: #define UPDATE_1(p) range -= bound; code -= bound; \ macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@26 PS4, Line 26: #define GET_BIT2(p, i, A0, A1) IF_BIT_0(p) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@27 PS4, Line 27: {UPDATE_0(p); i = (i + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@28 PS4, Line 28: {UPDATE_1(p); i = (i + i) + 1; A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@32 PS4, Line 32: #define REV_BIT(p, i, A0, A1) IF_BIT_0(p + i) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@33 PS4, Line 33: {UPDATE_0(p + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@34 PS4, Line 34: {UPDATE_1(p + i); A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@36 PS4, Line 36: #define REV_BIT_CONST(p, i, m) REV_BIT(p, i, i += m;, i += m * 2;) space required after that ';' (ctx:VxO) https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@40 PS4, Line 40: {i = 1; do { TREE_GET_BIT(probs, i); } while (i < limit); i -= limit; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@43 PS4, Line 43: {i = 1; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@44 PS4, Line 44: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@45 PS4, Line 45: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@46 PS4, Line 46: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@47 PS4, Line 47: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@48 PS4, Line 48: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@49 PS4, Line 49: TREE_GET_BIT(probs, i); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@50 PS4, Line 50: i -= 0x40; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@54 PS4, Line 54: matchByte += matchByte; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@55 PS4, Line 55: bit = offs; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@56 PS4, Line 56: offs &= matchByte; \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@57 PS4, Line 57: probLit = prob + (offs + bit + symbol); \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@58 PS4, Line 58: GET_BIT2(probLit, symbol, offs ^= bit;, ;) please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@58 PS4, Line 58: GET_BIT2(probLit, symbol, offs ^= bit;, ;) space required after that ';' (ctx:VxO) https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@60 PS4, Line 60: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; \ Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@60 PS4, Line 60: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; \ Macros with flow control statements should be avoided https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@63 PS4, Line 63: #define IF_BIT_0_CHECK(p) ttt = *(p); NORMALIZE_CHECK; \ Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@64 PS4, Line 64: bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) trailing statements should be on next line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@65 PS4, Line 65: #define UPDATE_0_CHECK range = bound; Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@65 PS4, Line 65: #define UPDATE_0_CHECK range = bound; macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@66 PS4, Line 66: #define UPDATE_1_CHECK range -= bound; code -= bound; Macros with multiple statements should be enclosed in a do - while loop https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@66 PS4, Line 66: #define UPDATE_1_CHECK range -= bound; code -= bound; macros should not use a trailing semicolon https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@67 PS4, Line 67: #define GET_BIT2_CHECK(p, i, A0, A1) IF_BIT_0_CHECK(p) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@68 PS4, Line 68: {UPDATE_0_CHECK; i = (i + i); A0; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@69 PS4, Line 69: {UPDATE_1_CHECK; i = (i + i) + 1; A1; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@72 PS4, Line 72: {i = 1; do { GET_BIT_CHECK(probs + i, i) } while (i < limit); i -= limit; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@75 PS4, Line 75: #define REV_BIT_CHECK(p, i, m) IF_BIT_0_CHECK(p + i) \ Macros with complex values should be enclosed in parentheses https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@76 PS4, Line 76: {UPDATE_0_CHECK; i += m; m += m; } else \ please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@77 PS4, Line 77: {UPDATE_1_CHECK; m += m; i += m; } please, no spaces at the start of a line https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@92 PS4, Line 92: unsigned int pbMask = ((unsigned int)1 << (p->prop.pb)) - 1; Unnecessary typecast of c90 int constant https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@94 PS4, Line 94: unsigned int lpMask = ((unsigned int)0x100 << p->prop.lp) - ((unsigned int)0x100 >> lc); Unnecessary typecast of c90 int constant https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@122 PS4, Line 122: dic[(dicPos == 0 ? dicBufSize : dicPos) - 1]) & lpMask) << lc); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@137 PS4, Line 137: unsigned int matchByte = dic[dicPos - rep0 + trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@180 PS4, Line 180: dic[dicPos] = dic[dicPos - rep0 + trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@203 PS4, Line 203: } else { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@233 PS4, Line 233: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@241 PS4, Line 241: TREE_DECODE(probLen, (1 << kLenNumHighBits), len); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@249 PS4, Line 249: ((len < kNumLenToPosStates ? trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@250 PS4, Line 250: len : kNumLenToPosStates - 1) << kNumPosSlotBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@254 PS4, Line 254: unsigned int numDirectBits = (unsigned int)(((distance >> 1) - 1)); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@262 PS4, Line 262: do { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@264 PS4, Line 264: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@269 PS4, Line 269: do { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@275 PS4, Line 275: t = (0 - ((UInt32)code >> 31)); /* (UInt32)((Int32)code >> 31) */ line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@276 PS4, Line 276: distance = (distance << 1) + (t + 1); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@287 PS4, Line 287: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@298 PS4, Line 298: if (distance == (UInt32)0xFFFFFFFF) { Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@309 PS4, Line 309: state = (state < kNumStates + kNumLitStates) ? trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@311 PS4, Line 311: if (distance >= (checkDicSize == 0 ? processedPos : checkDicSize)) { line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@345 PS4, Line 345: if (++pos == dicBufSize) Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@459 PS4, Line 459: ((((p->processedPos) & ((1 << (p->prop.lp)) - 1)) << p->prop.lc) + line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@460 PS4, Line 460: (p->dic[(p->dicPos == 0 ? trailing whitespace https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@461 PS4, Line 461: p->dicBufSize : p->dicPos) - 1] >> (8 - p->prop.lc)))); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@465 PS4, Line 465: do { GET_BIT_CHECK(prob + symbol, symbol) } while (symbol < 0x100); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@478 PS4, Line 478: GET_BIT2_CHECK(probLit, symbol, offs ^= bit; , ;) space prohibited before that ',' (ctx:WxW) https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@544 PS4, Line 544: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@561 PS4, Line 561: ((len < kNumLenToPosStates - 1 ? len : kNumLenToPosStates - 1) << line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@570 PS4, Line 570: prob = probs + SpecPos + ((2 | (posSlot & 1)) << numDirectBits); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@576 PS4, Line 576: code -= range & (((code - range) >> 31) - 1); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@578 PS4, Line 578: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@587 PS4, Line 587: } while (--numDirectBits); Too many leading tabs - consider code refactoring https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@659 PS4, Line 659: while (p->remainLen != kMatchSpecLenStart) { suspect code indent for conditional statements (8, 24) https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@710 PS4, Line 710: int dummyRes = LzmaDec_TryDummy(p, p->tempBuf, (SizeT)rem); line over 96 characters https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@753 PS4, Line 753: else else is not generally useful after a break or return https://review.coreboot.org/c/coreboot/+/41963/4/src/lib/lzmadecode.c@754 PS4, Line 754: dicSize = data[1] | ((UInt32)data[2] << 8) | ((UInt32)data[3] << 16) | trailing whitespace -- To view, visit https://review.coreboot.org/c/coreboot/+/41963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 Gerrit-Change-Number: 41963 Gerrit-PatchSet: 4 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jun 2020 04:44:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib: Upgrade LZMA Decoder to version 19.0
by Harshit Sharma (Code Review) 02 Jun '20

02 Jun '20

Hello build bot (Jenkins), Werner Zeh, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/41963 to look at the new patch set (#4). Change subject: lib: Upgrade LZMA Decoder to version 19.0 ...................................................................... lib: Upgrade LZMA Decoder to version 19.0 This patch upgrades LZMA Decoder library to version 19.0. It copies the decompression code unmodified from LZMA SDK 19.0 at https://www.7-zip.org/sdk.html. However, it only includes those functions which are necessary for single-call decompression and leaves out the rest. LZMA interface has now changed a lot from the version 4.42. Earlier, there was a separate test application LzmaTest.c on which our lzmadecode wrapper was based. But now there is a dedicated library function LzmaDecode() for single-call decompressing. To support this newer interface, this patch also updates the lzmadecode wrapper ulzman(). Fixes: https://ticket.coreboot.org/issues/194 Signed-off-by: Harshit Sharma <harshitsharmajs(a)gmail.com> Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 --- M src/lib/lzma.c M src/lib/lzmadecode.c M src/lib/lzmadecode.h 3 files changed, 862 insertions(+), 430 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/41963/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/41963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 Gerrit-Change-Number: 41963 Gerrit-PatchSet: 4 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: lib: Upgrade LZMA Decoder to version 19.0
by Harshit Sharma (Code Review) 02 Jun '20

02 Jun '20

Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41963 ) Change subject: lib: Upgrade LZMA Decoder to version 19.0 ...................................................................... Patch Set 3: (130 comments) https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h File src/lib/lzmadecode.h: https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@25 PS2, Line 25: #define RINOK(x) { int __result__ = (x); if (__result__ != 0) return __result__; } > Macros with flow control statements should be avoided Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@38 PS2, Line 38: SRes LzmaProps_Decode(CLzmaProps *p, const Byte *data, unsigned size); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@99 PS2, Line 99: #define LzmaProps_GetNumProbs(p) (NUM_BASE_PROBS + ((UInt32)LZMA_LIT_SIZE << ((p)->lc + (p)->lp))) > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@121 PS2, Line 121: unsigned tempBufSize; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@141 PS2, Line 141: LZMA_STATUS_MAYBE_FINISHED_WITHOUT_MARK /* there is probability that stream was finished without end mark */ > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.h@145 PS2, Line 145: const Byte *src, SizeT *srcLen, ELzmaFinishMode finishMode, ELzmaStatus *status); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c File src/lib/lzmadecode.c: https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@18 PS2, Line 18: #define NORMALIZE if (range < kTopValue) { range <<= 8; code = (code << 8) | (*buf++); } > Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defe […] Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@20 PS2, Line 20: #define IF_BIT_0(p) ttt = *(p); NORMALIZE; bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@20 PS2, Line 20: #define IF_BIT_0(p) ttt = *(p); NORMALIZE; bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@21 PS2, Line 21: #define UPDATE_0(p) range = bound; *(p) = (CLzmaProb)(ttt + ((kBitModelTotal - ttt) >> kNumMoveBits)); > macros should not use a trailing semicolon Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@21 PS2, Line 21: #define UPDATE_0(p) range = bound; *(p) = (CLzmaProb)(ttt + ((kBitModelTotal - ttt) >> kNumMoveBits)); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@21 PS2, Line 21: #define UPDATE_0(p) range = bound; *(p) = (CLzmaProb)(ttt + ((kBitModelTotal - ttt) >> kNumMoveBits)); > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@22 PS2, Line 22: #define UPDATE_1(p) range -= bound; code -= bound; *(p) = (CLzmaProb)(ttt - (ttt >> kNumMoveBits)); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@22 PS2, Line 22: #define UPDATE_1(p) range -= bound; code -= bound; *(p) = (CLzmaProb)(ttt - (ttt >> kNumMoveBits)); > macros should not use a trailing semicolon Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@22 PS2, Line 22: #define UPDATE_1(p) range -= bound; code -= bound; *(p) = (CLzmaProb)(ttt - (ttt >> kNumMoveBits)); > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@23 PS2, Line 23: #define GET_BIT2(p, i, A0, A1) IF_BIT_0(p) \ > Macros with complex values should be enclosed in parentheses Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@24 PS2, Line 24: { UPDATE_0(p); i = (i + i); A0; } else \ > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@25 PS2, Line 25: { UPDATE_1(p); i = (i + i) + 1; A1; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@29 PS2, Line 29: #define REV_BIT(p, i, A0, A1) IF_BIT_0(p + i) \ > Macros with complex values should be enclosed in parentheses Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@30 PS2, Line 30: { UPDATE_0(p + i); A0; } else \ > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@31 PS2, Line 31: { UPDATE_1(p + i); A1; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@33 PS2, Line 33: #define REV_BIT_CONST(p, i, m) REV_BIT(p, i, i += m;, i += m * 2;) > space required after that ';' (ctx:VxO) Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@37 PS2, Line 37: { i = 1; do { TREE_GET_BIT(probs, i); } while (i < limit); i -= limit; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@40 PS2, Line 40: { i = 1; \ > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@42 PS2, Line 42: TREE_GET_BIT(probs, i); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@43 PS2, Line 43: TREE_GET_BIT(probs, i); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@44 PS2, Line 44: TREE_GET_BIT(probs, i); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@45 PS2, Line 45: TREE_GET_BIT(probs, i); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@46 PS2, Line 46: TREE_GET_BIT(probs, i); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@47 PS2, Line 47: i -= 0x40; } > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@51 PS2, Line 51: matchByte += matchByte; \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@52 PS2, Line 52: bit = offs; \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@53 PS2, Line 53: offs &= matchByte; \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@54 PS2, Line 54: probLit = prob + (offs + bit + symbol); \ > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@55 PS2, Line 55: GET_BIT2(probLit, symbol, offs ^= bit;, ;) > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@55 PS2, Line 55: GET_BIT2(probLit, symbol, offs ^= bit;, ;) > space required after that ';' (ctx:VxO) Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@57 PS2, Line 57: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; range <<= 8; code = (code << 8) | (*buf++); } > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@57 PS2, Line 57: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; range <<= 8; code = (code << 8) | (*buf++); } > Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defe […] Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@57 PS2, Line 57: #define NORMALIZE_CHECK if (range < kTopValue) { if (buf >= bufLimit) return DUMMY_ERROR; range <<= 8; code = (code << 8) | (*buf++); } > Macros with flow control statements should be avoided Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@59 PS2, Line 59: #define IF_BIT_0_CHECK(p) ttt = *(p); NORMALIZE_CHECK; bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@59 PS2, Line 59: #define IF_BIT_0_CHECK(p) ttt = *(p); NORMALIZE_CHECK; bound = (range >> kNumBitModelTotalBits) * (UInt32)ttt; if (code < bound) > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@60 PS2, Line 60: #define UPDATE_0_CHECK range = bound; > macros should not use a trailing semicolon Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@60 PS2, Line 60: #define UPDATE_0_CHECK range = bound; > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@61 PS2, Line 61: #define UPDATE_1_CHECK range -= bound; code -= bound; > macros should not use a trailing semicolon Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@61 PS2, Line 61: #define UPDATE_1_CHECK range -= bound; code -= bound; > Macros with multiple statements should be enclosed in a do - while loop Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@62 PS2, Line 62: #define GET_BIT2_CHECK(p, i, A0, A1) IF_BIT_0_CHECK(p) \ > Macros with complex values should be enclosed in parentheses Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@63 PS2, Line 63: { UPDATE_0_CHECK; i = (i + i); A0; } else \ > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@64 PS2, Line 64: { UPDATE_1_CHECK; i = (i + i) + 1; A1; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@67 PS2, Line 67: { i = 1; do { GET_BIT_CHECK(probs + i, i) } while (i < limit); i -= limit; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@70 PS2, Line 70: #define REV_BIT_CHECK(p, i, m) IF_BIT_0_CHECK(p + i) \ > Macros with complex values should be enclosed in parentheses Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@71 PS2, Line 71: { UPDATE_0_CHECK; i += m; m += m; } else \ > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@72 PS2, Line 72: { UPDATE_1_CHECK; m += m; i += m; } > please, no spaces at the start of a line Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@85 PS2, Line 85: unsigned state = (unsigned)p->state; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@85 PS2, Line 85: unsigned state = (unsigned)p->state; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@87 PS2, Line 87: unsigned pbMask = ((unsigned)1 << (p->prop.pb)) - 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@87 PS2, Line 87: unsigned pbMask = ((unsigned)1 << (p->prop.pb)) - 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@88 PS2, Line 88: unsigned lc = p->prop.lc; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@89 PS2, Line 89: unsigned lpMask = ((unsigned)0x100 << p->prop.lp) - ((unsigned)0x100 >> lc); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@89 PS2, Line 89: unsigned lpMask = ((unsigned)0x100 << p->prop.lp) - ((unsigned)0x100 >> lc); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@89 PS2, Line 89: unsigned lpMask = ((unsigned)0x100 << p->prop.lp) - ((unsigned)0x100 >> lc); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@97 PS2, Line 97: unsigned len = 0; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@106 PS2, Line 106: unsigned ttt; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@107 PS2, Line 107: unsigned posState = CALC_POS_STATE(processedPos, pbMask); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@112 PS2, Line 112: unsigned symbol; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@116 PS2, Line 116: prob += (UInt32)3 * ((((processedPos << 8) + dic[(dicPos == 0 ? dicBufSize : dicPos) - 1]) & lpMask) << lc); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@131 PS2, Line 131: unsigned matchByte = dic[dicPos - rep0 + (dicPos < rep0 ? dicBufSize : 0)]; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@131 PS2, Line 131: unsigned matchByte = dic[dicPos - rep0 + (dicPos < rep0 ? dicBufSize : 0)]; > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@132 PS2, Line 132: unsigned offs = 0x100; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@135 PS2, Line 135: unsigned bit; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@173 PS2, Line 173: dic[dicPos] = dic[dicPos - rep0 + (dicPos < rep0 ? dicBufSize : 0)]; > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@195 PS2, Line 195: } else { > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@225 PS2, Line 225: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@233 PS2, Line 233: TREE_DECODE(probLen, (1 << kLenNumHighBits), len); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@241 PS2, Line 241: ((len < kNumLenToPosStates ? len : kNumLenToPosStates - 1) << kNumPosSlotBits); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@244 PS2, Line 244: unsigned posSlot = (unsigned)distance; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@244 PS2, Line 244: unsigned posSlot = (unsigned)distance; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@245 PS2, Line 245: unsigned numDirectBits = (unsigned)(((distance >> 1) - 1)); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@245 PS2, Line 245: unsigned numDirectBits = (unsigned)(((distance >> 1) - 1)); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@245 PS2, Line 245: unsigned numDirectBits = (unsigned)(((distance >> 1) - 1)); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@253 PS2, Line 253: do { > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@255 PS2, Line 255: } while (--numDirectBits); > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@260 PS2, Line 260: do { > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@266 PS2, Line 266: t = (0 - ((UInt32)code >> 31)); /* (UInt32)((Int32)code >> 31) */ > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@267 PS2, Line 267: distance = (distance << 1) + (t + 1); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@278 PS2, Line 278: } while (--numDirectBits); > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@282 PS2, Line 282: unsigned i = 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@286 PS2, Line 286: REV_BIT_LAST (prob, i, 8); > space prohibited between function name and open parenthesis '(' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@289 PS2, Line 289: if (distance == (UInt32)0xFFFFFFFF) { > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@300 PS2, Line 300: state = (state < kNumStates + kNumLitStates) ? kNumLitStates : kNumLitStates + 3; > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@301 PS2, Line 301: if (distance >= (checkDicSize == 0 ? processedPos : checkDicSize)) { > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@311 PS2, Line 311: unsigned curLen; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@318 PS2, Line 318: curLen = ((rem < len) ? (unsigned)rem : len); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@335 PS2, Line 335: if (++pos == dicBufSize) > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@365 PS2, Line 365: unsigned len = (unsigned)p->remainLen; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@365 PS2, Line 365: unsigned len = (unsigned)p->remainLen; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@369 PS2, Line 369: len = (unsigned)(rem); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@388 PS2, Line 388: #define kBadRepCode (kBound0 + (((kRange0 - kBound0) >> kNumBitModelTotalBits) << (kNumBitModelTotalBits - 1))) > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@429 PS2, Line 429: unsigned state = (unsigned)p->state; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@429 PS2, Line 429: unsigned state = (unsigned)p->state; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@435 PS2, Line 435: unsigned ttt; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@436 PS2, Line 436: unsigned posState = CALC_POS_STATE(p->processedPos, (1 << p->prop.pb) - 1); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@448 PS2, Line 448: ((((p->processedPos) & ((1 << (p->prop.lp)) - 1)) << p->prop.lc) + > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@449 PS2, Line 449: (p->dic[(p->dicPos == 0 ? p->dicBufSize : p->dicPos) - 1] >> (8 - p->prop.lc)))); > line over 96 characters Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@452 PS2, Line 452: unsigned symbol = 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@453 PS2, Line 453: do { GET_BIT_CHECK(prob + symbol, symbol) } while (symbol < 0x100); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@455 PS2, Line 455: unsigned matchByte = p->dic[p->dicPos - p->reps[0] + > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@457 PS2, Line 457: unsigned offs = 0x100; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@458 PS2, Line 458: unsigned symbol = 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@460 PS2, Line 460: unsigned bit; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@466 PS2, Line 466: GET_BIT2_CHECK(probLit, symbol, offs ^= bit; , ;) > space prohibited before that ',' (ctx:WxW) Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@471 PS2, Line 471: unsigned len; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@518 PS2, Line 518: unsigned limit, offset; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@532 PS2, Line 532: probLen = prob + LenLow + GET_LEN_STATE + (1 << kLenNumLowBits); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@547 PS2, Line 547: unsigned posSlot; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@549 PS2, Line 549: ((len < kNumLenToPosStates - 1 ? len : kNumLenToPosStates - 1) << > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@553 PS2, Line 553: unsigned numDirectBits = ((posSlot >> 1) - 1); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@564 PS2, Line 564: code -= range & (((code - range) >> 31) - 1); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@566 PS2, Line 566: } while (--numDirectBits); > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@571 PS2, Line 571: unsigned i = 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@572 PS2, Line 572: unsigned m = 1; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@575 PS2, Line 575: } while (--numDirectBits); > Too many leading tabs - consider code refactoring Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@647 PS2, Line 647: while (p->remainLen != kMatchSpecLenStart) { > suspect code indent for conditional statements (8, 24) Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@673 PS2, Line 673: p->tempBufSize = (unsigned)inSize; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@693 PS2, Line 693: unsigned rem = p->tempBufSize, lookAhead = 0; > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@698 PS2, Line 698: int dummyRes = LzmaDec_TryDummy(p, p->tempBuf, (SizeT)rem); > line over 96 characters Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@713 PS2, Line 713: unsigned kkk = (unsigned)(p->buf - p->tempBuf); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@713 PS2, Line 713: unsigned kkk = (unsigned)(p->buf - p->tempBuf); > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@734 PS2, Line 734: SRes LzmaProps_Decode(CLzmaProps *p, const Byte *data, unsigned size) > Prefer 'unsigned int' to bare use of 'unsigned' Done https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@741 PS2, Line 741: else > else is not generally useful after a break or return Ack https://review.coreboot.org/c/coreboot/+/41963/2/src/lib/lzmadecode.c@742 PS2, Line 742: dicSize = data[1] | ((UInt32)data[2] << 8) | ((UInt32)data[3] << 16) | ((UInt32)data[4] << 24); > line over 96 characters Done -- To view, visit https://review.coreboot.org/c/coreboot/+/41963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I31f43e8bdda6536c719879381cad6a921dd94718 Gerrit-Change-Number: 41963 Gerrit-PatchSet: 3 Gerrit-Owner: Harshit Sharma <harshitsharmajs(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 02 Jun 2020 04:41:40 +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

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2020