coreboot-gerrit March 2020

coreboot-gerrit@coreboot.org

1 participants
3326 discussions

Change in coreboot[master]: nb/intel/i945: Program CxODT value for each channel
by HAOUAS Elyes (Code Review) 16 Mar '20

16 Mar '20

HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/18548 ) Change subject: nb/intel/i945: Program CxODT value for each channel ...................................................................... Abandoned let save your time -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 35 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in coreboot[master]: cpu/intel/speedstep: Add Netburst
by HAOUAS Elyes (Code Review) 16 Mar '20

16 Mar '20

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22604 ) Change subject: cpu/intel/speedstep: Add Netburst ...................................................................... Patch Set 77: (24 comments) https://review.coreboot.org/c/coreboot/+/22604/22/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/22/src/cpu/intel/speedstep/s… PS22, Line 45: if (((rdmsr(MSR_EXTENDED_CONFIG).lo >> 27) & 3) == 3) {/*supported > line over 80 characters Ack https://review.coreboot.org/c/coreboot/+/22604/22/src/cpu/intel/speedstep/s… PS22, Line 46: and enabled bits */ > line over 80 characters Ack https://review.coreboot.org/c/coreboot/+/22604/22/src/cpu/intel/speedstep/s… PS22, Line 48: params->slfm = SPEEDSTEP_STATE_FROM_MSR(msr.lo, state_mask); > line over 80 characters Ack https://review.coreboot.org/c/coreboot/+/22604/22/src/cpu/intel/speedstep/s… PS22, Line 80: params->turbo = SPEEDSTEP_STATE_FROM_MSR(msr.hi, state_mask); > line over 80 characters Ack https://review.coreboot.org/c/coreboot/+/22604/42/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/42/src/cpu/intel/speedstep/s… PS42, Line 111: /* Merom */ > The code is correct but it's easy to make errors with switch statements so I'd add a comment that Me […] Ack https://review.coreboot.org/c/coreboot/+/22604/44/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/44/src/cpu/intel/speedstep/s… PS44, Line 42: (c.x86 == 0xf) ? 0 : 1 > just change it to c.x86 == 0x6 or c. […] Ack https://review.coreboot.org/c/coreboot/+/22604/44/src/cpu/intel/speedstep/s… PS44, Line 79: msr_platform_info_supported > the MSR at 0xce is MSR_PLATFORM_INFO Ack https://review.coreboot.org/c/coreboot/+/22604/44/src/cpu/intel/speedstep/s… PS44, Line 81: MSR_FSB_CLOCK_VCC > Please rename MSR_FSB_CLOCK_VCC in this case Ack https://review.coreboot.org/c/coreboot/+/22604/44/src/cpu/intel/speedstep/s… PS44, Line 84: ! > use tab Ack https://review.coreboot.org/c/coreboot/+/22604/46/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/46/src/cpu/intel/speedstep/s… PS46, Line 42: == > != Ack https://review.coreboot.org/c/coreboot/+/22604/54/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/54/src/cpu/intel/speedstep/s… PS54, Line 110: } /* Merom fallthrough */ > do you want me to add "case 0xf" ? (it falls through) Ack https://review.coreboot.org/c/coreboot/+/22604/61/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/61/src/cpu/intel/speedstep/s… PS61, Line 84: !(rdmsr(IA32_MISC_ENABLE).hi & (1 << (38 - 32)))) { > you don't want this on the same indentation level as the code. Ack https://review.coreboot.org/c/coreboot/+/22604/61/src/cpu/intel/speedstep/s… PS61, Line 95: switch (c.x86) { : case 0x6: : switch (c.x86_model) { : case 0xe: : /* Yonah */ : params->min.power = SPEEDSTEP_MIN_POWER_YONAH; : params->max.power = SPEEDSTEP_MAX_POWER_YONAH; : break; : case 0x17: : /* Penryn */ : params->slfm.power = SPEEDSTEP_SLFM_POWER_PENRYN; : params->min.power = SPEEDSTEP_MIN_POWER_PENRYN; : params->max.power = SPEEDSTEP_MAX_POWER_PENRYN; : params->turbo.power = SPEEDSTEP_MAX_POWER_PENRYN; : break; : } /* Merom fallthrough */ : default: : /* Use Merom values by default (as before). */ : params->slfm.power = SPEEDSTEP_SLFM_POWER_MEROM; : params->min.power = SPEEDSTEP_MIN_POWER_MEROM; : params->max.power = SPEEDSTEP_MAX_POWER_MEROM; : params->turbo.power = SPEEDSTEP_MAX_POWER_MEROM; : break; : } > it is likely more readable if you set the default and then override it for select CPU's Ack https://review.coreboot.org/c/coreboot/+/22604/64/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/64/src/cpu/intel/speedstep/s… PS64, Line 84: !(rdmsr(IA32_MISC_ENABLE).hi & (1 << (38 - 32)))) { > Done Ack https://review.coreboot.org/c/coreboot/+/22604/66/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/66/src/cpu/intel/speedstep/s… PS66, Line 102: break; > Hmmm.. I don't like nested 'switch' statements that much. […] Ack https://review.coreboot.org/c/coreboot/+/22604/67/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/67/src/cpu/intel/speedstep/s… PS67, Line 118: } > code indent should use tabs where possible Ack https://review.coreboot.org/c/coreboot/+/22604/67/src/cpu/intel/speedstep/s… PS67, Line 118: } > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/22604/68/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/68/src/cpu/intel/speedstep/s… PS68, Line 118: } > please, no spaces at the start of a line Ack https://review.coreboot.org/c/coreboot/+/22604/68/src/cpu/intel/speedstep/s… PS68, Line 118: } > code indent should use tabs where possible Ack https://review.coreboot.org/c/coreboot/+/22604/69/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/69/src/cpu/intel/speedstep/s… PS69, Line 42: u8 msr_extended_config_supported = c.x86 != 0xf; > const bool Ack https://review.coreboot.org/c/coreboot/+/22604/70/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/70/src/cpu/intel/speedstep/s… PS70, Line 113: params->slfm.power = SPEEDSTEP_SLFM_POWER_MEROM; > Wrong indent. […] Ack https://review.coreboot.org/c/coreboot/+/22604/71/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/71/src/cpu/intel/speedstep/s… PS71, Line 110: default: > Families other than 0x06 will no longer have these set. […] Ack https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… PS77, Line 52: SPEEDSTEP_STATE_FROM_MSR(msr.lo, state_mask); > This now fits in 96 characters. Ack https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… PS77, Line 93: if (c.x86 == 6 && c.x86_model == 0xe) { > Why is the if-else statement better than the switch? Please see CL66 -- To view, visit https://review.coreboot.org/c/coreboot/+/22604 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06e162d3260dedeb3b16583460633507fbcbd52a Gerrit-Change-Number: 22604 Gerrit-PatchSet: 77 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 11:48:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload/drivers/nvram: Add function to write RTC
by Patrick Georgi (Code Review) 16 Mar '20

16 Mar '20

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23585 ) Change subject: libpayload/drivers/nvram: Add function to write RTC ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/23585/7/payloads/libpayload/driver… File payloads/libpayload/drivers/nvram.c: https://review.coreboot.org/c/coreboot/+/23585/7/payloads/libpayload/driver… PS7, Line 185: !(statusB & NVRAM_RTC_FORMAT_BINARY) > Maybe place this check inside a wrapper for nvram_write()? That way, the writes don't have to be dup […] The encoding is more complicated than "always wrap in bcd coding". For example NVRAM_RTC_HOURS sometimes needs bit 7 set, no matter the encoding of the rest (compare lines 192 and 206). -- To view, visit https://review.coreboot.org/c/coreboot/+/23585 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17b4c1ee0dcc649738ac6a7400b087d07213eaf0 Gerrit-Change-Number: 23585 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 11:28:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/inteltool: Makefile: add src/arch to includes
by Patrick Georgi (Code Review) 16 Mar '20

16 Mar '20

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39525 ) Change subject: util/inteltool: Makefile: add src/arch to includes ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile File util/inteltool/Makefile: https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile@48 PS5, Line 48: ARCH = $(subst _64,,$(shell uname -m)) > Well, as an alternative we could copy the macros directly to inteltool Dynamically determining the architecture seems superfluous to me. CPPFLAGS += -I$(top)/src/arch/x86/include should work just as well and not be OS dependent, no? -- To view, visit https://review.coreboot.org/c/coreboot/+/39525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Gerrit-Change-Number: 39525 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 11:25:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/inteltool: Makefile: add src/arch to includes
by Michael Niewöhner (Code Review) 16 Mar '20

16 Mar '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39525 ) Change subject: util/inteltool: Makefile: add src/arch to includes ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile File util/inteltool/Makefile: https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile@48 PS5, Line 48: ARCH = $(subst _64,,$(shell uname -m)) > huh? superfluous? missed the follow-up patches? Well, as an alternative we could copy the macros directly to inteltool -- To view, visit https://review.coreboot.org/c/coreboot/+/39525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Gerrit-Change-Number: 39525 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 11:00:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Comment-In-Reply-To: Michael Niewöhner Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/inteltool: Makefile: add src/arch to includes
by Michael Niewöhner (Code Review) 16 Mar '20

16 Mar '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39525 ) Change subject: util/inteltool: Makefile: add src/arch to includes ...................................................................... Patch Set 6: > Patch Set 5: Code-Review+1 > > > Patch Set 5: > > > > > Patch Set 5: > > > > > > this is to get the read/write functions used in some patches later in this patch train, right? > > > > I guess so > > I'd like to see this in the commit message done -- To view, visit https://review.coreboot.org/c/coreboot/+/39525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Gerrit-Change-Number: 39525 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 10:59:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/inteltool: Makefile: add src/arch to includes
by Michael Niewöhner (Code Review) 16 Mar '20

16 Mar '20

Hello Felix Singer, build bot (Jenkins), Patrick Georgi, Patrick Rudolph, Paul Menzel, Stefan Reinauer, Angel Pons, Mimoja, Aaron Durbin, Nico Huber, Maxim Polyakov, Idwer Vollering, Christian Walter, HAOUAS Elyes, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/39525 to look at the new patch set (#6). Change subject: util/inteltool: Makefile: add src/arch to includes ...................................................................... util/inteltool: Makefile: add src/arch to includes Add src/arch to includes to be able to use the read* macros in the follow-up patches. Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M util/inteltool/Makefile 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/39525/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/39525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Gerrit-Change-Number: 39525 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: util/inteltool: Makefile: add src/arch to includes
by Michael Niewöhner (Code Review) 16 Mar '20

16 Mar '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39525 ) Change subject: util/inteltool: Makefile: add src/arch to includes ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile File util/inteltool/Makefile: https://review.coreboot.org/c/coreboot/+/39525/5/util/inteltool/Makefile@48 PS5, Line 48: ARCH = $(subst _64,,$(shell uname -m)) > That looks both superfluous (do we ever expect _intel_tool to be usable on a non-intel architecture? […] huh? superfluous? missed the follow-up patches? -- To view, visit https://review.coreboot.org/c/coreboot/+/39525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I157178a055a259e40c57f3915671d3b8966fbb96 Gerrit-Change-Number: 39525 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Mimoja <coreboot(a)mimoja.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 10:59:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/intel/speedstep: Add Netburst
by Paul Menzel (Code Review) 16 Mar '20

16 Mar '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22604 ) Change subject: cpu/intel/speedstep: Add Netburst ...................................................................... Patch Set 77: (2 comments) https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… File src/cpu/intel/speedstep/speedstep.c: https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… PS77, Line 52: SPEEDSTEP_STATE_FROM_MSR(msr.lo, state_mask); This now fits in 96 characters. https://review.coreboot.org/c/coreboot/+/22604/77/src/cpu/intel/speedstep/s… PS77, Line 93: if (c.x86 == 6 && c.x86_model == 0xe) { Why is the if-else statement better than the switch? -- To view, visit https://review.coreboot.org/c/coreboot/+/22604 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06e162d3260dedeb3b16583460633507fbcbd52a Gerrit-Change-Number: 22604 Gerrit-PatchSet: 77 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 10:18:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: nb/intel/i945: Program CxODT value for each channel
by HAOUAS Elyes (Code Review) 16 Mar '20

16 Mar '20

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 ) Change subject: nb/intel/i945: Program CxODT value for each channel ...................................................................... Patch Set 35: (1 comment) current change isn't a big deal. it will not break anything and it just fix the values of CxODT per channel. we lost a important time for this patch and maybe we don't need to loos again time for this ... so if you agree, I'll move this to abandoned and save your time. https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… File src/northbridge/intel/i945/raminit.c: https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2450: one > We are talking about `(populated_dimms & 3) == 1 != (populated_dimms & 3) == 2)`, right? […] I'm confused with this : `(populated_dimms & 3) == 1 != (populated_dimms & 3) == 2)` -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 35 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 09:46:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Comment-In-Reply-To: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2020