coreboot-gerrit March 2019

coreboot-gerrit@coreboot.org

1 participants
1542 discussions

Change in ...coreboot[master]: drivers/i2c/nct7802y: Add new hardware-monitoring IC
by Nico Huber (Code Review) 14 Mar '19

14 Mar '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29475 ) Change subject: drivers/i2c/nct7802y: Add new hardware-monitoring IC ...................................................................... Uploaded patch set 6. (6 comments) https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h File src/drivers/i2c/nct7802y/nct7802y.h: https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h… PS1, Line 39: << 0 > Just to align things and make it more visible that it's a bit mask. Done https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h… PS1, Line 55: tab > maybe rename tab to fan, so it's clearer that that's the fan id. […] Done https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y.h… PS1, Line 93: unset_mask > clear_mask? Done https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_f… File src/drivers/i2c/nct7802y/nct7802y_fan.c: https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_f… PS1, Line 32: 3 > use NCT7802Y_FAN_CNT here Done https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_f… PS1, Line 68: div > for the case smart.mode == SMART_FAN_DUTY, div = 1 is likely wrong (comment in chip. […] Ack, Smart Fan documentation mentions a 255 maximum. https://review.coreboot.org/#/c/29475/1/src/drivers/i2c/nct7802y/nct7802y_f… PS1, Line 82: return; > print a warning, when config is 0? Done Check moved into nct7802y_init(). -- To view, visit https://review.coreboot.org/c/coreboot/+/29475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I35ea79e12941804e398c6304a08170a776f4ca76 Gerrit-Change-Number: 29475 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 13 Mar 2019 18:51:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: drivers/i2c/nct7802y: Add new hardware-monitoring IC
by Nico Huber (Code Review) 14 Mar '19

14 Mar '19

Hello Felix Held, Paul Menzel, Thomas Heijligen, build bot (Jenkins), Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29475 to look at the new patch set (#6). Change subject: drivers/i2c/nct7802y: Add new hardware-monitoring IC ...................................................................... drivers/i2c/nct7802y: Add new hardware-monitoring IC Just another hardware-monitoring chip. Only limited fan control and PECI configuration is implemented. Change-Id: I35ea79e12941804e398c6304a08170a776f4ca76 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- A src/drivers/i2c/nct7802y/Kconfig A src/drivers/i2c/nct7802y/Makefile.inc A src/drivers/i2c/nct7802y/chip.h A src/drivers/i2c/nct7802y/nct7802y.c A src/drivers/i2c/nct7802y/nct7802y.h A src/drivers/i2c/nct7802y/nct7802y_fan.c A src/drivers/i2c/nct7802y/nct7802y_peci.c 7 files changed, 454 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/29475/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/29475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I35ea79e12941804e398c6304a08170a776f4ca76 Gerrit-Change-Number: 29475 Gerrit-PatchSet: 6 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: mb/google/hatch: Provide DRAM part number from EEPROM
by Furquan Shaikh (Code Review) 14 Mar '19

14 Mar '19

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31851 Change subject: mb/google/hatch: Provide DRAM part number from EEPROM ...................................................................... mb/google/hatch: Provide DRAM part number from EEPROM This change reads DRAM part number from EEPROM if available and returns it using the SoC callback (mainboard_get_dram_part_number). BUG=b:127609572 TEST=Verify that DRAM part number from EEPROM is added to DMI table 17 (dmidecode -t 17). Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/mainboard/google/hatch/romstage.c 1 file changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/31851/1 diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c index 401f41f..9f1aabd 100644 --- a/src/mainboard/google/hatch/romstage.c +++ b/src/mainboard/google/hatch/romstage.c @@ -14,6 +14,9 @@ */ #include <baseboard/variants.h> +#include <console/console.h> +#include <ec/google/chromeec/ec.h> +#include <memory_info.h> #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h> @@ -27,3 +30,16 @@ cannonlake_memcfg_init(&memupd->FspmConfig, variant_memory_params(), &spd); } + +void mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE]; + + if (google_chromeec_cbi_get_dram_part_num(&part_num_store[0], + sizeof(part_num_store)) < 0) { + printk(BIOS_ERR, "No DRAM part number in CBI!\n"); + } else { + *part_num = &part_num_store[0]; + *len = sizeof(part_num_store); + } +} -- To view, visit https://review.coreboot.org/c/coreboot/+/31851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ade6999828b6d67aa78d04199138f195a97ba8c Gerrit-Change-Number: 31851 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

2 10

Change in ...coreboot[master]: soc/intel/cannonlake: Allow mainboard to override DRAM part number
by Furquan Shaikh (Code Review) 14 Mar '19

14 Mar '19

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31850 Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ...................................................................... soc/intel/cannonlake: Allow mainboard to override DRAM part number In order to support mainboards that do not store DRAM part number in the traditional way i.e. within the CBFS SPD for soldered memory, this change provides a runtime callback to allow mainboards to provide DRAM part number from a custom location e.g. external EEPROM on hatch. For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing. BUG=b:127609572 Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 20 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31850/1 diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index a58ace5..643105a 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -20,6 +20,9 @@ #include <fsp/api.h> void mainboard_memory_init_params(FSPM_UPD *mupd); + +/* Provide a callback to allow mainboard to override the DRAM part number. */ +void mainboard_get_dram_part_num(const char **part_num, size_t *len); void systemagent_early_init(void); /* Board type */ diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index 6abeb3d..ebbd213 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -37,6 +37,12 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ } +void __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + /* Default weak implementation, no need to override part number. */ + return; +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -50,6 +56,8 @@ const MEMORY_INFO_DATA_HOB *memory_info_hob; const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; + const char *dram_part_num; + size_t dram_part_num_len; /* Locate the memory info HOB, presence validated by raminit */ memory_info_hob = fsp_find_extension_hob_by_guid( @@ -86,6 +94,13 @@ if (src_dimm->Status != DIMM_PRESENT) continue; + dram_part_num_len = sizeof(src_dimm->ModulePartNum); + dram_part_num = (const char *)&src_dimm->ModulePartNum[0]; + + /* Allow mainboard to override DRAM part number. */ + mainboard_get_dram_part_num(&dram_part_num, + &dram_part_num_len); + /* Populate the DIMM information */ dimm_info_fill(dest_dimm, src_dimm->DimmCapacity, @@ -94,8 +109,8 @@ src_dimm->RankInDimm, channel_info->ChannelId, src_dimm->DimmId, - (const char *)src_dimm->ModulePartNum, - sizeof(src_dimm->ModulePartNum), + dram_part_num, + dram_part_num_len, memory_info_hob->DataWidth); index++; } -- To view, visit https://review.coreboot.org/c/coreboot/+/31850 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Gerrit-Change-Number: 31850 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

3 6

Change in ...coreboot[master]: device/root_device: Scan bridges on LPC bus too
by Nico Huber (Code Review) 14 Mar '19

14 Mar '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29474 ) Change subject: device/root_device: Scan bridges on LPC bus too ...................................................................... Patch Set 5: Kyösti, IIRC we discussed this before (it's been a while, look at the author date). I think there were some doubts about changing bus numbers (that some mainboard code might assume to be steady). Any idea what I could test / look at? -- To view, visit https://review.coreboot.org/c/coreboot/+/29474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0eb005e41b9168fffc344ee8e666d43b605a30ba Gerrit-Change-Number: 29474 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 13 Mar 2019 15:39:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: device/root_device: Scan bridges on LPC bus too
by Nico Huber (Code Review) 14 Mar '19

14 Mar '19

Hello Kyösti Mälkki, Aaron Durbin, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29474 to look at the new patch set (#5). Change subject: device/root_device: Scan bridges on LPC bus too ...................................................................... device/root_device: Scan bridges on LPC bus too There may be more buses behind LPC. For instance I2C on a super-i/o. Change-Id: I0eb005e41b9168fffc344ee8e666d43b605a30ba Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M src/device/root_device.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/29474/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/29474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0eb005e41b9168fffc344ee8e666d43b605a30ba Gerrit-Change-Number: 29474 Gerrit-PatchSet: 5 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in ...coreboot[master]: arch/x86: Fix PCI IO config accessor
by Nico Huber (Code Review) 13 Mar '19

13 Mar '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31847 ) Change subject: arch/x86: Fix PCI IO config accessor ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31847 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id2888e07fc0f2b182b4633a747c1786e5c560678 Gerrit-Change-Number: 31847 Gerrit-PatchSet: 3 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 13 Mar 2019 12:02:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: lib/ramtest: Use Kconfig switch CONFIG_RAMBASE for RAM test
by Kyösti Mälkki (Code Review) 13 Mar '19

13 Mar '19

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29106 ) Change subject: lib/ramtest: Use Kconfig switch CONFIG_RAMBASE for RAM test ...................................................................... Patch Set 2: > Patch Set 2: > > With the few remaining quick_ram_check() calls, I think checking memory near CBMEM / IMD root would be good option. > > Sounds like a good idea as the first thing placed in RAM is cbmem right after memory init. Except ith FSP 1.0, by the time we return from raminit, CAR is already torn down and our stack is in RAM. So yeah... can't test RAM before utilising it so I quess it will die silently with postcode (unless there is serial debug from FSP). -- To view, visit https://review.coreboot.org/c/coreboot/+/29106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc57e684a677ac64225ea54cb43e6fbb68930354 Gerrit-Change-Number: 29106 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Wed, 13 Mar 2019 12:01:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: arch/x86: Optimise PCI IO config accessor
by Nico Huber (Code Review) 13 Mar '19

13 Mar '19

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31754 ) Change subject: arch/x86: Optimise PCI IO config accessor ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31754 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6e729fe800c92b1ce4994ad2b4203072fa75a958 Gerrit-Change-Number: 31754 Gerrit-PatchSet: 5 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Wed, 13 Mar 2019 12:01:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: lib/ramtest: Use Kconfig switch CONFIG_RAMBASE for RAM test
by Patrick Rudolph (Code Review) 13 Mar '19

13 Mar '19

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29106 ) Change subject: lib/ramtest: Use Kconfig switch CONFIG_RAMBASE for RAM test ...................................................................... Patch Set 2: > Patch Set 2: > > > Patch Set 2: > > > > > Patch Set 2: > > > > > > That is an arbitrary address that is "tested" here. For all the platforms with RELOCATABLE_RAMSTAGE=y, CONFIG_RAMBASE is no longer where ramstage will be loaded as it's dynamically located somewhere high inside CBMEM. > > > > > > In my opinion testing some address near CBMEM / IMD root (or even cbmem_top()) would be more useful, as that will be utilised before ramstage region. > > > > Thank you for the clarification Kyösti, then I was wrong here. Should we still follow the CBMEM approach? Otherwise I can abandon this patch. > > Sorry for the noise. > > With the few remaining quick_ram_check() calls, I think checking memory near CBMEM / IMD root would be good option. Sounds like a good idea as the first thing placed in RAM is cbmem right after memory init. -- To view, visit https://review.coreboot.org/c/coreboot/+/29106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc57e684a677ac64225ea54cb43e6fbb68930354 Gerrit-Change-Number: 29106 Gerrit-PatchSet: 2 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Wed, 13 Mar 2019 10:58:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2019