coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: mb/google/brya: Ensure descriptor is writable before writing
by Sridhar Siricilla (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, EricR Lai. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56679 ) Change subject: mb/google/brya: Ensure descriptor is writable before writing ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I09613d569ffc92fc3b7334bc30628c4f88955250 Gerrit-Change-Number: 56679 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 20:13:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: create dynamic power limits mechanism for thermal
by Tim Wawrzynczak (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Furquan Shaikh, Sumeet R Pawnikar, Zhuohao Lee, Aaron Durbin, Karthik Ramasubramanian. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56515 ) Change subject: mb/google/brya: create dynamic power limits mechanism for thermal ...................................................................... Patch Set 5: (1 comment) File src/mainboard/google/brya/variants/brya0/overridetree.cb: https://review.coreboot.org/c/coreboot/+/56515/comment/cf5c93a8_7d900f0d PS1, Line 30: field THERMAL 15 17 : option POWER_LIMITS_282 0 : option POWER_LIMITS_482 1 : option POWER_LIMITS_682 2 : end > Sorry, let me reopen this thread for the discussion. […] This is just for Brya0, each mainboard can set their own individual policies. -- To view, visit https://review.coreboot.org/c/coreboot/+/56515 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86619516adeec13642f02ba7faf9fc4945ad774e Gerrit-Change-Number: 56515 Gerrit-PatchSet: 5 Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 19:58:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Comment-In-Reply-To: Zhuohao Lee <zhuohao(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Update descriptor region for board ID 2
by Tim Wawrzynczak (Code Review) 29 Jul '21

29 Jul '21

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56681 ) Change subject: mb/google/brya: Update descriptor region for board ID 2 ...................................................................... mb/google/brya: Update descriptor region for board ID 2 brya0 boards with board ID 2 can support a 50MHz SPI flash speed, therefore modify the descriptor on the first boot to change the fast read, write, erase, and status read speeds to 50 MHz. BUG=b:188577893 Change-Id: Ibc4cca72f72eceededf2790f28124e46a02d12dc Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/brya/variants/brya0/Makefile.inc A src/mainboard/google/brya/variants/brya0/bootblock.c 2 files changed, 46 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/56681/1 diff --git a/src/mainboard/google/brya/variants/brya0/Makefile.inc b/src/mainboard/google/brya/variants/brya0/Makefile.inc index cd8eb69..319ad98 100644 --- a/src/mainboard/google/brya/variants/brya0/Makefile.inc +++ b/src/mainboard/google/brya/variants/brya0/Makefile.inc @@ -1,4 +1,5 @@ bootblock-y += gpio.c +bootblock-y += bootblock.c ramstage-$(CONFIG_FW_CONFIG) += fw_config.c ramstage-$(CONFIG_FW_CONFIG) += variant.c ramstage-y += gpio.c diff --git a/src/mainboard/google/brya/variants/brya0/bootblock.c b/src/mainboard/google/brya/variants/brya0/bootblock.c new file mode 100644 index 0000000..6a7228e --- /dev/null +++ b/src/mainboard/google/brya/variants/brya0/bootblock.c @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <baseboard/variants.h> +#include <boardid.h> +#include <types.h> + +static bool update_spi_freq(uint8_t *buf) +{ + struct flcomp { + uint32_t dontcare0 : 20; + uint32_t fast_read : 1; + uint32_t fast_read_freq : 3; + uint32_t write_erase_freq : 3; + uint32_t read_status_freq : 3; + uint32_t dontcare1 : 2; + } *flcomp = (struct flcomp *)(buf + 0x30); + enum { + FREQ_100MHZ = 0x00, + FREQ_50MHZ = 0x01, + FREQ_33MHZ = 0x03, + FREQ_25MHZ = 0x04, + FREQ_14MHZ = 0x06, + }; + + /* Only applicable to board ID 2 */ + if (board_id() != 2) + return false; + + if (flcomp->fast_read_freq == FREQ_50MHZ && + flcomp->write_erase_freq == FREQ_50MHZ && + flcomp->read_status_freq == FREQ_50MHZ && + flcomp->fast_read == 0x01) + return false; + + flcomp->fast_read_freq = FREQ_50MHZ; + flcomp->write_erase_freq = FREQ_50MHZ; + flcomp->read_status_freq = FREQ_50MHZ; + flcomp->fast_read = 0x01; + return true; +} + +bool variant_update_descriptor(uint8_t *desc) +{ + return update_spi_freq(desc); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/56681 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc4cca72f72eceededf2790f28124e46a02d12dc Gerrit-Change-Number: 56681 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brya: Add overrideable variant_update_descriptor
by Tim Wawrzynczak (Code Review) 29 Jul '21

29 Jul '21

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56680 ) Change subject: mb/google/brya: Add overrideable variant_update_descriptor ...................................................................... mb/google/brya: Add overrideable variant_update_descriptor Some variants may want to uniquely modify the descriptor region based on their own criteria. Therefore add a weak function that variants can use for this. BUG=b:188577893 Change-Id: I1ac3b639b77855695aabac79a069b621554df707 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/brya/bootblock.c M src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h 2 files changed, 9 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/56680/1 diff --git a/src/mainboard/google/brya/bootblock.c b/src/mainboard/google/brya/bootblock.c index 4b655c0..23dfd01 100644 --- a/src/mainboard/google/brya/bootblock.c +++ b/src/mainboard/google/brya/bootblock.c @@ -20,6 +20,11 @@ #define FLASH_VAL_SIGN 0xFF0A55A #define FLMSTR_WR_SHIFT_V2 20 +__weak bool variant_update_descriptor(uint8_t *desc) +{ + return false; +} + /* It checks whether host(Flash Master 1) has write access to the Descriptor Region or not */ static int is_descriptor_writeable(uint8_t *desc) { @@ -80,7 +85,7 @@ if (!read_descriptor(&desc_rdev, si_desc_buf) || !is_descriptor_writeable(si_desc_buf)) return; - if (!update_for_a0(si_desc_buf)) + if (!update_for_a0(si_desc_buf) && !variant_update_descriptor(si_desc_buf)) return; if (!write_descriptor(&desc_rdev, si_desc_buf)) { diff --git a/src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h index c938de8..fa58164 100644 --- a/src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h @@ -22,4 +22,7 @@ bool variant_is_half_populated(void); void variant_update_soc_chip_config(struct soc_intel_alderlake_config *config); +/* Optional function for variants to update the descriptor when it is writable */ +bool variant_update_descriptor(uint8_t *desc); + #endif /*__BASEBOARD_VARIANTS_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/56680 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1ac3b639b77855695aabac79a069b621554df707 Gerrit-Change-Number: 56680 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: mb/google/brya: Ensure descriptor is writable before writing
by Tim Wawrzynczak (Code Review) 29 Jul '21

29 Jul '21

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56679 ) Change subject: mb/google/brya: Ensure descriptor is writable before writing ...................................................................... mb/google/brya: Ensure descriptor is writable before writing Before updating the descriptor region in bootblock, ensure that the region is writeable to avoid infinite boot loops. BUG=b:188577893 Change-Id: I09613d569ffc92fc3b7334bc30628c4f88955250 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/brya/bootblock.c 1 file changed, 74 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/56679/1 diff --git a/src/mainboard/google/brya/bootblock.c b/src/mainboard/google/brya/bootblock.c index ac0dc32..4b655c0 100644 --- a/src/mainboard/google/brya/bootblock.c +++ b/src/mainboard/google/brya/bootblock.c @@ -1,57 +1,97 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include <arch/mmio.h> #include <baseboard/variants.h> #include <bootblock_common.h> +#include <cf9_reset.h> #include <console/console.h> -#include <fmap.h> #include <commonlib/region.h> #include <console/console.h> #include <cpu/intel/cpu_ids.h> +#include <fmap.h> #include <intelblocks/pmclib.h> -#include <cf9_reset.h> +#include <types.h> #define SI_DESC_REGION "SI_DESC" #define SI_DESC_REGION_SZ 4096 #define PMC_DESC_7_BYTE3 0xc32 +#define FLASH_SIGN_OFFSET 0x10 +#define FLMSTR1 0x80 +#define FLASH_VAL_SIGN 0xFF0A55A +#define FLMSTR_WR_SHIFT_V2 20 -/* It updates PMC Descriptor in the Descriptor Region */ -static void configure_pmc_descriptor(void) +/* It checks whether host(Flash Master 1) has write access to the Descriptor Region or not */ +static int is_descriptor_writeable(uint8_t *desc) +{ + /* Check flash has valid signature */ + if (read32((void *)(desc + FLASH_SIGN_OFFSET)) != FLASH_VAL_SIGN) { + printk(BIOS_DEBUG, "Flash Descriptor is not valid\n"); + return 0; + } + + /* Check host has write access to the Descriptor Region */ + if (!((read32((void *)(desc + FLMSTR1)) >> FLMSTR_WR_SHIFT_V2) & BIT(0))) { + printk(BIOS_DEBUG, "Host doesn't have write access to Descriptor Region\n"); + return 0; + } + + return 1; +} + +static bool read_descriptor(struct region_device *rdev, uint8_t *buf) +{ + if (fmap_locate_area_as_rdev_rw(SI_DESC_REGION, rdev) < 0) { + printk(BIOS_ERR, "Failed to locate %s in the FMAP\n", SI_DESC_REGION); + return false; + } + + if (rdev_readat(rdev, buf, 0, SI_DESC_REGION_SZ) != SI_DESC_REGION_SZ) { + printk(BIOS_ERR, "Failed to read Descriptor Region from SPI Flash\n"); + return false; + } + + return true; +} + +static bool update_for_a0(uint8_t *desc) +{ + if (cpu_get_cpuid() != CPUID_ALDERLAKE_A0 || desc[PMC_DESC_7_BYTE3] != 0x40) { + printk(BIOS_DEBUG, "Update for A0 Si is not required!\n"); + return false; + } + + desc[PMC_DESC_7_BYTE3] = 0x44; + return true; +} + +static bool write_descriptor(struct region_device *rdev, uint8_t *desc) +{ + if (rdev_eraseat(rdev, 0, SI_DESC_REGION_SZ) != SI_DESC_REGION_SZ) + return false; + + return rdev_writeat(rdev, desc, 0, SI_DESC_REGION_SZ) == SI_DESC_REGION_SZ; +} + +static void configure_descriptor(void) { uint8_t si_desc_buf[SI_DESC_REGION_SZ]; struct region_device desc_rdev; - if (fmap_locate_area_as_rdev_rw(SI_DESC_REGION, &desc_rdev) < 0) { - printk(BIOS_ERR, "Failed to locate %s in the FMAP\n", SI_DESC_REGION); + if (!read_descriptor(&desc_rdev, si_desc_buf) || !is_descriptor_writeable(si_desc_buf)) + return; + + if (!update_for_a0(si_desc_buf)) + return; + + if (!write_descriptor(&desc_rdev, si_desc_buf)) { + printk(BIOS_INFO, "Failed to update descriptor region!\n"); return; } - if (rdev_readat(&desc_rdev, si_desc_buf, 0, SI_DESC_REGION_SZ) != SI_DESC_REGION_SZ) { - printk(BIOS_ERR, "Failed to read Descriptor Region from SPI Flash\n"); - return; - } - - if (si_desc_buf[PMC_DESC_7_BYTE3] == 0x40) { - si_desc_buf[PMC_DESC_7_BYTE3] = 0x44; - - if (rdev_eraseat(&desc_rdev, 0, SI_DESC_REGION_SZ) != SI_DESC_REGION_SZ) { - printk(BIOS_ERR, "Failed to erase Descriptor Region area\n"); - return; - } - - if (rdev_writeat(&desc_rdev, si_desc_buf, 0, SI_DESC_REGION_SZ) - != SI_DESC_REGION_SZ) { - printk(BIOS_ERR, "Failed to update Descriptor Region\n"); - return; - } - - printk(BIOS_DEBUG, "Update of PMC Descriptor successful, trigger GLOBAL RESET\n"); - - pmc_global_reset_enable(1); - do_full_reset(); - die("Failed to trigger GLOBAL RESET\n"); - } - - printk(BIOS_DEBUG, "Update of PMC Descriptor is not required!\n"); + printk(BIOS_DEBUG, "Update of Descriptor successful, trigger GLOBAL RESET\n"); + pmc_global_reset_enable(true); + do_full_reset(); + die("Failed to trigger GLOBAL RESET\n"); } void bootblock_mainboard_early_init(void) @@ -64,6 +104,5 @@ void bootblock_mainboard_init(void) { - if (cpu_get_cpuid() == CPUID_ALDERLAKE_A0) - configure_pmc_descriptor(); + configure_descriptor(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56679 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I09613d569ffc92fc3b7334bc30628c4f88955250 Gerrit-Change-Number: 56679 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: cpu/x86: Rename X86_AMD_INIT_SIPI to X86_INIT_SIPI
by Furquan Shaikh (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Tim Wawrzynczak, Subrata Banik, Felix Held. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56651 ) Change subject: cpu/x86: Rename X86_AMD_INIT_SIPI to X86_INIT_SIPI ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This is a finding by us and we had initiated a discussion to capture the details at validation side, […] Subrata there are two changes that happen under X86_AMD_INIT_SIPI/X86_INIT_SIPI: 1. Remove the 10msec delay after sending INIT 2. Second SIPI is skipped Both of these are not documented anywhere. When you say that "this is a finding by us" --> Finding based on? And has this been confirmed with the h/w architecture team. If so, then it should be updated as an addendum to the MP spec or added to BWG to make it clear what platforms really support this. Also, I see the linux kernel still following the same specification: https://elixir.bootlin.com/linux/latest/source/arch/x86/kernel/smpboot.c#L7… -- To view, visit https://review.coreboot.org/c/coreboot/+/56651 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7a4e6a8b1edc6e8ba43597259bd8b2de697e4e62 Gerrit-Change-Number: 56651 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 28 Jul 2021 19:26:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Don't suppress SPD Module Type value
by Furquan Shaikh (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Paul Menzel, Subrata Banik, Angel Pons, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56650 ) Change subject: soc/intel/common: Don't suppress SPD Module Type value ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/smbios.c: https://review.coreboot.org/c/coreboot/+/56650/comment/6872d51b_4c50768f PS3, Line 18: /* Translate to DDR2 module type field that SMBIOS code expects. */ > Yes i don't know that answer either and IMHO we should let platform choose if they want to go through this conversion or pass the module type directly. But that is not really a platform decision. We have module type that is provided by SPD which the platform gets either from CBFS SPD or EEPROM SPD. That module type is specific to the ddr type. SMBIOS has its own definitions for memory form factor. So, what we need is conversion of module type for a specific ddr type into smbios form factor. This can be handled by the platform, but I don't think that is required. It will help keep things simpler so that platforms don't have to worry about the conversion. > Its possible to add the type conversion how DDR3 had done so far for other memory type as well like DDR4/LP4/5 but does that make sense is my question? No, we shouldn't be converting other memory types to DDR2 type. We should be converting all memory types into smbios type directly. i.e. in `create_smbios_type17_for_dimm()` we should do something like this: ``` if (dimm->ddr_type == MEMORY_TYPE_DDR2) t->form_factor = convert_ddr2_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_DDR3) t->form_factor = convert_ddr3_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_DDR4) t->form_factor = convert_ddr4_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_LPDDR3) t->form_factor = convert_lp3_module_type(dimm->mod_type); else ... ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/56650 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia3f38c24efa6a8685639bb607926e2fd9c702ff6 Gerrit-Change-Number: 56650 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 28 Jul 2021 19:19:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Don't suppress SPD Module Type value
by Subrata Banik (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Furquan Shaikh, Paul Menzel, Angel Pons, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56650 ) Change subject: soc/intel/common: Don't suppress SPD Module Type value ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/smbios.c: https://review.coreboot.org/c/coreboot/+/56650/comment/a09e0640_e1cf98bd PS3, Line 18: /* Translate to DDR2 module type field that SMBIOS code expects. */ > Subrata, I think we need to address the root problem here. Why does the SMBIOS code expect module type in DDR2 format? Yes i don't know that answer either and IMHO we should let platform choose if they want to go through this conversion or pass the module type directly. I have tested DDR4/5/LP4/5 by passing direct memory type with those few CLs and no DDR2 conversion, I didn't see any issue, so I don't understand why we always need to convert any latest SPD spec into older DDR2 type and feed into SMBIOS function. Can we ignore that conversion. Its possible to add the type conversion how DDR3 had done so far for other memory type as well like DDR4/LP4/5 but does that make sense is my question? -- To view, visit https://review.coreboot.org/c/coreboot/+/56650 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia3f38c24efa6a8685639bb607926e2fd9c702ff6 Gerrit-Change-Number: 56650 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 28 Jul 2021 18:49:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: guybrush: Document USB mapping in devicetree
by Rob Barnes (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Martin Roth. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56667 ) Change subject: guybrush: Document USB mapping in devicetree ...................................................................... Patch Set 3: (2 comments) File src/mainboard/google/guybrush/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/56667/comment/b30a92a2_c87b8932 PS1, Line 167: }, > Does USB3 ports not support USB A receptacle? Also same for WWAN, some WWAN use USB3 to support high […] You are right, I updated Usb3PhyPort with A0 and A1. File src/mainboard/google/guybrush/variants/baseboard/devicetree.cb: https://review.coreboot.org/c/coreboot/+/56667/comment/1704070b_7c4c6253 PS2, Line 222: register "desc" = ""Right Type-C Port"" Should this actually be Left Type-C port / C0? -- To view, visit https://review.coreboot.org/c/coreboot/+/56667 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14cbb6af021bb27c89aa82456722f21aa09617be Gerrit-Change-Number: 56667 Gerrit-PatchSet: 3 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 18:39:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: guybrush: Document USB mapping in devicetree
by Karthik Ramasubramanian (Code Review) 29 Jul '21

29 Jul '21

Attention is currently required from: Martin Roth, Rob Barnes. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56667 ) Change subject: guybrush: Document USB mapping in devicetree ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56667 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14cbb6af021bb27c89aa82456722f21aa09617be Gerrit-Change-Number: 56667 Gerrit-PatchSet: 3 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Comment-Date: Wed, 28 Jul 2021 18:39:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021