coreboot-gerrit February 2021

coreboot-gerrit@coreboot.org

1 participants
7189 discussions

Change in coreboot[master]: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates
by build bot (Jenkins) (Code Review) 05 Feb '21

05 Feb '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49766 ) Change subject: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates ...................................................................... Patch Set 12: (8 comments) File src/soc/intel/tigerlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/49766/comment/9ed34929_4f2e2671 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) that open brace { should be on the previous line https://review.coreboot.org/c/coreboot/+/49766/comment/fe0585c0_3f039559 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) spaces required around that '==' (ctx:WxV) https://review.coreboot.org/c/coreboot/+/49766/comment/7cc273d9_65b0e220 PS12, Line 60: if((cpu_id == CPUID_TIGERLAKE_A0) || (cpu_id ==CPUID_TIGERLAKE_B0)) space required before the open parenthesis '(' https://review.coreboot.org/c/coreboot/+/49766/comment/ac44255d_f99032fe PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) line over 96 characters https://review.coreboot.org/c/coreboot/+/49766/comment/39902f84_6cd5777a PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) that open brace { should be on the previous line https://review.coreboot.org/c/coreboot/+/49766/comment/69281857_7453ca18 PS12, Line 62: if((mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_2_2) || (mchid == PCI_DEVICE_ID_INTEL_TGL_ID_U_4_2)) space required before the open parenthesis '(' https://review.coreboot.org/c/coreboot/+/49766/comment/3dc47cdb_8ccd0719 PS12, Line 89: /* If external phy gating is not implemented, S0i3.3/S0i3.4/S0i2.2 are not recommended. */ line over 96 characters https://review.coreboot.org/c/coreboot/+/49766/comment/f18a94c7_30823ab3 PS12, Line 94: if (is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_CNVI_BT)) || is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_CNVI_WIFI)) || is_dev_enabled(pcidev_path_on_root(PCH_DEVFN_ISH))) line over 96 characters -- To view, visit https://review.coreboot.org/c/coreboot/+/49766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 Gerrit-Change-Number: 49766 Gerrit-PatchSet: 12 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 05 Feb 2021 00:00:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates
by Shreesh Chhabbi (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Shreesh Chhabbi. Hello build bot (Jenkins), Furquan Shaikh, Ravishankar Sarawadi, Duncan Laurie, Alex Levin, Duncan Laurie, Sukumar Ghorai, Raj Astekar, Patrick Rudolph, Shreesh Chhabbi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49766 to look at the new patch set (#12). Change subject: soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates ...................................................................... soc/intel/tgl: Disable S0i3.2 & S0i3.3 substates S0i3.2 and S0i3.3 are applicable only if wake on voice is disabled. As per Platform Design Guide, S0i3.2 and S0i3.3 substates need to be disabled for Tigerlake. Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 72 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/49766/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/49766 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f2ac8b72d0c9b05bc02c092188d0c742cc83af9 Gerrit-Change-Number: 49766 Gerrit-PatchSet: 12 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Alex Levin <levinale(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Subrata Banik, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50241 to look at the new patch set (#4). Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, we're running into trouble. If the expected size is smaller than the UPD size the FSP provides, call die(), since the target buffer isn't large enough so only the beginning of the UPD defaults from the FSP will get copied into the buffer. We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d --- M src/drivers/intel/fsp2_0/memory_init.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/50241/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/50241/comment/71f4349f_a52a88d1 PS1, Line 242: != > After looking at the code again, yes you're right. […] i split that into the two cases and added a comment explaining the two cases and only call die() in the case where we're sure that things will be broken -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Feb 2021 23:39:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marshall Dawson <marshalldawson3rd(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by build bot (Jenkins) (Code Review) 05 Feb '21

05 Feb '21

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Subrata Banik, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50241 to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, call die(). We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d --- M src/drivers/intel/fsp2_0/memory_init.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/50241/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by build bot (Jenkins) (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50241 ) Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... Patch Set 2: (10 comments) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/50241/comment/93a85176_06498264 PS2, Line 242: /* trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/dc131924_1b8e05da PS2, Line 244: * use a static buffer for the FSP-M UPDs which is sizeof(FSPM_UPD) trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/fa3cf0d2_61c9a60d PS2, Line 245: * byes long, since that is the value known at compile time. If trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/366f8d64_0eac1687 PS2, Line 246: * hdr->cfg_region_size is bigger than that, not all UPD defaults will trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/f69c8675_ff3f3156 PS2, Line 247: * be copied, so it'll contain random data at the end, so we just call trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/7d8f1bf9_c2f23903 PS2, Line 248: * die() in that case. If hdr->cfg_region_size is smaller than that, trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/c90927b6_675785a0 PS2, Line 249: * there's a mismatch between the FSP and the header, but since it trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/525420f3_b7677027 PS2, Line 250: * will copy the full UPD defaults to the buffer, we try to continue trailing whitespace https://review.coreboot.org/c/coreboot/+/50241/comment/abc55497_386b351a PS2, Line 251: * and hope that there wasn't any incompatible change in the UPDs. 'wasn' may be misspelled - perhaps 'was'? https://review.coreboot.org/c/coreboot/+/50241/comment/c4de3840_55b8e7a4 PS2, Line 252: */ trailing whitespace -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 04 Feb 2021 23:36:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
by Felix Held (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Furquan Shaikh, Meera Ravindranath, Andrey Petrov, Patrick Rudolph, Felix Held. Hello Justin Frodsham, build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Meera Ravindranath, Subrata Banik, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50241 to look at the new patch set (#2). Change subject: drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size ...................................................................... drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, call die(). We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d --- M src/drivers/intel/fsp2_0/memory_init.c 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/50241/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/50241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Gerrit-Change-Number: 50241 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Justin Frodsham <justin.frodsham(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: {src}: Assign values before if statement
by Julius Werner (Code Review) 05 Feb '21

05 Feb '21

Attention is currently required from: Frans Hendriks, Kyösti Mälkki, Alexander Couzens, Aaron Durbin, Patrick Rudolph. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50040 ) Change subject: {src}: Assign values before if statement ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > I was not aware of history of checkpatch. […] I mean, are you implying that the current style was inconsistent? I would say this is currently allowed and code authors can write code that way at their discretion, what's not consistent about that? There's no single situation where you'd _have_ to write something like this, just like you don't _have_ to use long strings or lines that require breaking or any of the dozen other things we have code style rules for. Like I mentioned there is also a while() version of this which is used more often, and I think disallowing one while allowing the other would be the bigger inconsistency. -- To view, visit https://review.coreboot.org/c/coreboot/+/50040 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idd346d31f1da71271ad1545e5645e57530c7b374 Gerrit-Change-Number: 50040 Gerrit-PatchSet: 3 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 04 Feb 2021 23:21:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Frans Hendriks <fhendriks(a)eltan.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboards: Remove default CHROMEOS=y
by Tim Wawrzynczak (Code Review) 04 Feb '21

04 Feb '21

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50118 ) Change subject: mainboards: Remove default CHROMEOS=y ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > We should prepare the chorme ToT CL for this when this CL UPSTREAM to our tree. Everything should be OK, I went through and checked all of the boards to ensure we already select CONFIG_CHROMEOS in the config.${BOARD} files. -- To view, visit https://review.coreboot.org/c/coreboot/+/50118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I16fcf62a23dae1b21c77cee275c867f9c1de893b Gerrit-Change-Number: 50118 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Thu, 04 Feb 2021 22:56:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2021