coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[S] Change in coreboot[master]: intel/i82371eb,speedstep: Use dev_count_cpu()
by Kyösti Mälkki (Code Review) 14 Apr '23

14 Apr '23

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74397 ) Change subject: intel/i82371eb,speedstep: Use dev_count_cpu() ...................................................................... intel/i82371eb,speedstep: Use dev_count_cpu() Change-Id: I8582d401c72ad44137f117315c5c6869654c3e99 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74397 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/intel/speedstep/acpi.c M src/southbridge/intel/i82371eb/acpi_tables.c 2 files changed, 16 insertions(+), 26 deletions(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved diff --git a/src/cpu/intel/speedstep/acpi.c b/src/cpu/intel/speedstep/acpi.c index 839fec4..acca77e 100644 --- a/src/cpu/intel/speedstep/acpi.c +++ b/src/cpu/intel/speedstep/acpi.c @@ -9,18 +9,6 @@ #include <device/device.h> #include <types.h> -static int determine_total_number_of_cores(void) -{ - struct device *cpu; - int count = 0; - for (cpu = all_devices; cpu; cpu = cpu->next) { - if (!is_enabled_cpu(cpu)) - continue; - count++; - } - return count; -} - static void gen_pstate_entries(const sst_table_t *const pstates, const int cpuID, const int cores_per_package, const uint8_t coordination) @@ -78,7 +66,7 @@ void generate_cpu_entries(const struct device *device) { int coreID, cpuID, pcontrol_blk = PMB0_BASE, plen = 6; - int totalcores = determine_total_number_of_cores(); + int totalcores = dev_count_cpu(); int cores_per_package = (cpuid_ebx(1)>>16) & 0xff; int numcpus = totalcores/cores_per_package; /* This assumes that all CPUs share the same diff --git a/src/southbridge/intel/i82371eb/acpi_tables.c b/src/southbridge/intel/i82371eb/acpi_tables.c index c23c2f6..cd002e8 100644 --- a/src/southbridge/intel/i82371eb/acpi_tables.c +++ b/src/southbridge/intel/i82371eb/acpi_tables.c @@ -6,22 +6,10 @@ #include <device/device.h> #include "i82371eb.h" -static int determine_total_number_of_cores(void) -{ - struct device *cpu; - int count = 0; - for (cpu = all_devices; cpu; cpu = cpu->next) { - if (!is_enabled_cpu(cpu)) - continue; - count++; - } - return count; -} - void generate_cpu_entries(const struct device *device) { int cpu, pcontrol_blk=DEFAULT_PMBASE+PCNTRL, plen=6; - int numcpus = determine_total_number_of_cores(); + int numcpus = dev_count_cpu(); printk(BIOS_DEBUG, "Found %d CPU(s).\n", numcpus); /* without the outer scope, further ssdt addition will end up -- To view, visit https://review.coreboot.org/c/coreboot/+/74397 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8582d401c72ad44137f117315c5c6869654c3e99 Gerrit-Change-Number: 74397 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: Mainboard/lenovo/x200: Configure ck505 clockgen
by Paul Menzel (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Stefan Ott, Arthur Heymans, Alexander Couzens. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74418 ) Change subject: Mainboard/lenovo/x200: Configure ck505 clockgen ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/74418/comment/a870a490_a99dba9a PS2, Line 7: Mainboard mb https://review.coreboot.org/c/coreboot/+/74418/comment/7dd7aa3f_0c8f5a6b PS2, Line 11: From #coreboot(a)libera.chat: > Vendor set clockgen to `11 f1 9f df 41 00 25 06 80 00`, while coreboot currently sets it to `01 ff ff fe 00 00 25 06 04 00` https://review.coreboot.org/c/coreboot/+/74418/comment/de70095d_20ed3206 PS2, Line 15: If it does: > Resolves: https://ticket.coreboot.org/issues/478 -- To view, visit https://review.coreboot.org/c/coreboot/+/74418 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec9bbbaed43b74e8782f6963e491dbfa3b8b5247 Gerrit-Change-Number: 74418 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-CC: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 14 Apr 2023 08:37:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/brya/var/omnigul: Adjust I2Cs CLK to be around 400 kHz
by Jamie Chen (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Kyle Lin, Nick Vaccaro, Eric Lai. Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74393 ) Change subject: mb/google/brya/var/omnigul: Adjust I2Cs CLK to be around 400 kHz ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/74393/comment/3736c1c4_0127a16d PS2, Line 13: 293 > 293? why so low? EE reply : the measure value is on the spec. https://review.coreboot.org/c/coreboot/+/74393/comment/ed60125f_a3e07fb6 PS2, Line 15: 294 > same above. EE reply : the measure value is on the spec. -- To view, visit https://review.coreboot.org/c/coreboot/+/74393 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c4fdf0e003318a69b870b487a60accefbc0ffed Gerrit-Change-Number: 74393 Gerrit-PatchSet: 2 Gerrit-Owner: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Fri, 14 Apr 2023 08:36:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/common: Fix acpigen use for processor Device
by Kyösti Mälkki (Code Review) 14 Apr '23

14 Apr '23

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74395 ) Change subject: soc/intel/common: Fix acpigen use for processor Device ...................................................................... soc/intel/common: Fix acpigen use for processor Device Change-Id: Ib4e21732ac31076a1a97a774e03c8466d17c5f29 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74395 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/intel/common/block/acpi/acpi.c 1 file changed, 14 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/soc/intel/common/block/acpi/acpi.c b/src/soc/intel/common/block/acpi/acpi.c index 4b790b7..4139054 100644 --- a/src/soc/intel/common/block/acpi/acpi.c +++ b/src/soc/intel/common/block/acpi/acpi.c @@ -414,7 +414,7 @@ /* Soc specific power states generation */ soc_power_states_generation(core_id, num_virt); - acpigen_pop_len(); + acpigen_write_processor_device_end(); } } /* PPKG is usually used for thermal management -- To view, visit https://review.coreboot.org/c/coreboot/+/74395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4e21732ac31076a1a97a774e03c8466d17c5f29 Gerrit-Change-Number: 74395 Gerrit-PatchSet: 2 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: acpi/acpi.c: Follow spec more closely for MADT
by Kyösti Mälkki (Code Review) 14 Apr '23

14 Apr '23

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69225 ) Change subject: acpi/acpi.c: Follow spec more closely for MADT ...................................................................... acpi/acpi.c: Follow spec more closely for MADT Secondary threads need to be added after the primary threads. Change-Id: I3a98560760b662a7ba7efb46f5f7882fb0f7bb1f Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69225 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com> Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> --- M src/acpi/acpi.c 1 file changed, 46 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Lean Sheng Tan: Looks good to me, approved Maximilian Brune: Looks good to me, but someone else must approve diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index f4b6d20..de5abe0 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -157,20 +157,39 @@ return current; } +/* Increase if necessary. Currently all x86 CPUs only have 2 SMP threads */ +#define MAX_THREAD_ID 1 +/* + * From ACPI 6.4 spec: + * "The advent of multi-threaded processors yielded multiple logical processors + * executing on common processor hardware. ACPI defines logical processors in + * an identical manner as physical processors. To ensure that non + * multi-threading aware OSPM implementations realize optimal performance on + * platforms containing multi-threaded processors, two guidelines should be + * followed. The first is the same as above, that is, OSPM should initialize + * processors in the order that they appear in the MADT. The second is that + * platform firmware should list the first logical processor of each of the + * individual multi-threaded processors in the MADT before listing any of the + * second logical processors. This approach should be used for all successive + * logical processors." + */ static unsigned long acpi_create_madt_lapics(unsigned long current) { struct device *cpu; - int index, apic_ids[CONFIG_MAX_CPUS] = {0}, num_cpus = 0; - - for (cpu = all_devices; cpu; cpu = cpu->next) { - if (!is_enabled_cpu(cpu)) - continue; - if (num_cpus >= ARRAY_SIZE(apic_ids)) - break; - apic_ids[num_cpus++] = cpu->path.apic.apic_id; + int index, apic_ids[CONFIG_MAX_CPUS] = {0}, num_cpus = 0, sort_start = 0; + for (unsigned int thread_id = 0; thread_id <= MAX_THREAD_ID; thread_id++) { + for (cpu = all_devices; cpu; cpu = cpu->next) { + if (!is_enabled_cpu(cpu)) + continue; + if (num_cpus >= ARRAY_SIZE(apic_ids)) + break; + if (cpu->path.apic.thread_id != thread_id) + continue; + apic_ids[num_cpus++] = cpu->path.apic.apic_id; + } + bubblesort(&apic_ids[sort_start], num_cpus - sort_start, NUM_ASCENDING); + sort_start = num_cpus; } - if (num_cpus > 1) - bubblesort(apic_ids, num_cpus, NUM_ASCENDING); for (index = 0; index < num_cpus; index++) current = acpi_create_madt_one_lapic(current, index, apic_ids[index]); -- To view, visit https://review.coreboot.org/c/coreboot/+/69225 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a98560760b662a7ba7efb46f5f7882fb0f7bb1f Gerrit-Change-Number: 69225 Gerrit-PatchSet: 28 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: Mainboard/lenovo/x200: Configure ck505 clockgen
by Arthur Heymans (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Stefan Ott, Alexander Couzens. Hello Stefan Ott, Alexander Couzens, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74418 to look at the new patch set (#2). Change subject: Mainboard/lenovo/x200: Configure ck505 clockgen ...................................................................... Mainboard/lenovo/x200: Configure ck505 clockgen The clockgen is behind an smbus mux which is implemented with a gpio. This adds a callback inside the ck505 code to configure the mux. TESTED on x200: boots fine TOCHECK: x301 Change-Id: Iec9bbbaed43b74e8782f6963e491dbfa3b8b5247 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/i2c/ck505/ck505.c A src/drivers/i2c/ck505/ck505.h M src/mainboard/lenovo/x200/Kconfig M src/mainboard/lenovo/x200/devicetree.cb M src/mainboard/lenovo/x200/mainboard.c 5 files changed, 62 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/74418/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74418 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec9bbbaed43b74e8782f6963e491dbfa3b8b5247 Gerrit-Change-Number: 74418 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: Mainboard/lenovo/x200: Configure ck505 clockgen
by Arthur Heymans (Code Review) 14 Apr '23

14 Apr '23

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/74418 ) Change subject: Mainboard/lenovo/x200: Configure ck505 clockgen ...................................................................... Mainboard/lenovo/x200: Configure ck505 clockgen The clockgen is behind an smbus mux which is implemented with a gpio. This adds a callback inside the ck505 code to configure the mux. UNTESTED and not checked if applicable on x301. Change-Id: Iec9bbbaed43b74e8782f6963e491dbfa3b8b5247 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/i2c/ck505/ck505.c M src/mainboard/lenovo/x200/Kconfig M src/mainboard/lenovo/x200/devicetree.cb M src/mainboard/lenovo/x200/mainboard.c 4 files changed, 50 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/74418/1 diff --git a/src/drivers/i2c/ck505/ck505.c b/src/drivers/i2c/ck505/ck505.c index 58eaec6..a3858ad 100644 --- a/src/drivers/i2c/ck505/ck505.c +++ b/src/drivers/i2c/ck505/ck505.c @@ -5,9 +5,18 @@ #include <device/device.h> #include <device/smbus.h> #include "chip.h" +#include "ck505.h" #define SMBUS_BLOCK_SIZE 32 +__weak void ck505_set_mux_enable(void) +{ +} + +__weak void ck505_set_mux_disable(void) +{ +} + static void ck505_init(struct device *dev) { struct drivers_i2c_ck505_config *config; @@ -18,6 +27,8 @@ if (!dev->enabled || dev->path.type != DEVICE_PATH_I2C) return; + ck505_set_mux_enable(); + config = dev->chip_info; dev_nregs = smbus_block_read(dev, 0, sizeof(block), block); @@ -41,6 +52,8 @@ if (smbus_block_write(dev, 0, dev_nregs, block) < 0) printk(BIOS_ERR, "Failed writing ck505 configuration!\n"); + + ck505_set_mux_disable(); } static struct device_operations ck505_operations = { diff --git a/src/mainboard/lenovo/x200/Kconfig b/src/mainboard/lenovo/x200/Kconfig index 6b22fb3..c926691 100644 --- a/src/mainboard/lenovo/x200/Kconfig +++ b/src/mainboard/lenovo/x200/Kconfig @@ -21,6 +21,7 @@ select DRIVERS_LENOVO_WACOM select MAINBOARD_HAS_LIBGFXINIT select MAINBOARD_USES_IFD_GBE_REGION + select DRIVERS_I2C_CK505 config VBOOT select VBOOT_VBNV_FLASH diff --git a/src/mainboard/lenovo/x200/devicetree.cb b/src/mainboard/lenovo/x200/devicetree.cb index 7871cfd..ca1dc9e 100644 --- a/src/mainboard/lenovo/x200/devicetree.cb +++ b/src/mainboard/lenovo/x200/devicetree.cb @@ -165,6 +165,15 @@ device i2c 5e on end device i2c 5f on end end + chip drivers/i2c/ck505 + register "mask" = "{ 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff }" + register "regs" = "{ 0x11, 0xf1, 0x9f, + 0xdf, 0x41, 0x00, 0x25, 0x06, + 0x80, 0x00 }" + device i2c 69 on end + end end device pci 1f.5 off end # SATA/IDE 2 device pci 1f.6 off end # Thermal diff --git a/src/mainboard/lenovo/x200/mainboard.c b/src/mainboard/lenovo/x200/mainboard.c index 4b2f253..1c96c69 100644 --- a/src/mainboard/lenovo/x200/mainboard.c +++ b/src/mainboard/lenovo/x200/mainboard.c @@ -3,6 +3,18 @@ #include <device/device.h> #include <drivers/intel/gma/int15.h> #include <drivers/lenovo/lenovo.h> +#include <drivers/i2c/ck505/ck505.h> +#include <southbridge/intel/common/gpio.h> + +void ck505_set_mux_enable(void) +{ + set_gpio(42, GPIO_LEVEL_HIGH); +} + +void ck505_set_mux_disable(void) +{ + set_gpio(42, GPIO_LEVEL_LOW); +} static void fill_ssdt(const struct device *device) { -- To view, visit https://review.coreboot.org/c/coreboot/+/74418 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec9bbbaed43b74e8782f6963e491dbfa3b8b5247 Gerrit-Change-Number: 74418 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: util/cbfstool: Qualify struct e820entry as packed
by Arthur Heymans (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Bill XIE, Paul Menzel. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74270 ) Change subject: util/cbfstool: Qualify struct e820entry as packed ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74270 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I09955c90e4eec337adca383e628a8821075381d6 Gerrit-Change-Number: 74270 Gerrit-PatchSet: 6 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 14 Apr 2023 08:03:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/cmn/cse: Move API to get FW partition info into cse_lite.c
by Subrata Banik (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Tarun Tuli, SRIDHAR SIRICILLA, Dinesh Gehlot, Kapil Porwal, Sridhar Siricilla. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74382 ) Change subject: soc/intel/cmn/cse: Move API to get FW partition info into cse_lite.c ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74382/comment/070dfe59_dfc4d5b5 PS1, Line 9: The patch moves API that gets the CSE FW partition information into : CSE Lite specific file aka cse_lite.c because the consumer of this API : is the cse_lite specific ChromeOS devices hence, it's meaningful to : move the cse lite specific implementation inside cse_lite.c file. > cse.c represents library, provides API. I don't see any need to move the GET IMAGE FW INFO command handler into cse_lite.c. I request you to reconsider the option of moving the API into cse_lite.c. Thanks! cse.c can't be a library because it has other pci ops entries. Today the R&R of cse. c and cse_lite.c are not very clear. IMO, CSE.c is something which is common between cse consumer/corporate/lite sku where else cse_lite.c is specifically to meet the chromeos devices usecase. If you wish to represent cse.c as a library then it has to be renamed as cselib.c like cpulib.c file and refactored accordingly. Coming to the reason why i moved this API into cse_lite.c. seems like this whole patchsets and incremental patch train is very specific to CSE lite implementation and chromeos use case, where we would like to store the cse rw version after CSE sync being done (either in romstage or ramstage). Additionally, get ISH partition version also having the same dependency towards cse sync being done, when to call into it. what is the point of making it public when the other non-CSE lite aka consumer/corporate skus can't utilize the entire implementation. The last patch in this train is eventually making this API as static. In future, if we felt other non-CSE lite users would like to use this API (which is not likely the case) then it can be declared as public. -- To view, visit https://review.coreboot.org/c/coreboot/+/74382 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I49ffaec467f6fb24327de3b2882e37bf31eeb7cf Gerrit-Change-Number: 74382 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: SRIDHAR SIRICILLA Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: SRIDHAR SIRICILLA Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Fri, 14 Apr 2023 07:54:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/dedede/var/boten: Generate SPD ID for supported memory part
by Kevin3 Yang (Code Review) 14 Apr '23

14 Apr '23

Attention is currently required from: Henry Sun, Paul Fagerburg, Karthik Ramasubramanian, Felix Held. Hello build bot (Jenkins), Henry Sun, Paul Fagerburg, Karthik Ramasubramanian, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/74416 to look at the new patch set (#5). Change subject: mb/google/dedede/var/boten: Generate SPD ID for supported memory part ...................................................................... mb/google/dedede/var/boten: Generate SPD ID for supported memory part Add boten supported memory parts in mem_parts_used.txt, generate SPD id for this part. 1. Samsung K4U6E3S4AB-MGCL BUG=b:278138388 TEST=Use part_id_gen to generate related settings Signed-off-by: kevin3.yang <kevin3.yang(a)lcfc.corp-partner.google.com> Change-Id: I5f910393847c6494f77c009cb11f50b31bebffb6 --- M src/mainboard/google/dedede/variants/boten/memory/Makefile.inc M src/mainboard/google/dedede/variants/boten/memory/dram_id.generated.txt M src/mainboard/google/dedede/variants/boten/memory/mem_parts_used.txt 3 files changed, 21 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/74416/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/74416 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f910393847c6494f77c009cb11f50b31bebffb6 Gerrit-Change-Number: 74416 Gerrit-PatchSet: 5 Gerrit-Owner: Kevin3 Yang <kevin3.yang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: Leo Chou <leo.chou(a)lcfc.corp-partner.google.com> Gerrit-CC: Stanley Wu <stanley1.wu(a)lcfc.corp-partner.google.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023