coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3243 discussions

Change in coreboot[master]: mb/google/dedede/var/boten: Add LTE power on/off sequence
by Peichao Wang (Code Review) 03 Nov '20

03 Nov '20

Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44588 ) Change subject: mb/google/dedede/var/boten: Add LTE power on/off sequence ...................................................................... Patch Set 10: (2 comments) https://review.coreboot.org/c/coreboot/+/44588/10/src/mainboard/google/dede… File src/mainboard/google/dedede/variants/boten/gpio.c: https://review.coreboot.org/c/coreboot/+/44588/10/src/mainboard/google/dede… PS10, Line 11: 1 > That is what I wrote in patchset 9 and it was verified to meet the power sequence requirement. […] Dear Furquan and Karthik, if set its value equal 0, LTE module can not work, we have do the experiment like ticket: https://partnerissuetracker.corp.google.com/issues/163100335#comment94 Thanks a lot https://review.coreboot.org/c/coreboot/+/44588/10/src/mainboard/google/dede… PS10, Line 45: 1 > Shouldn't this be 0? Since it is being deasserted by the ACPI methods? I think ACPI still not validation if set value equal 0 in here. -- To view, visit https://review.coreboot.org/c/coreboot/+/44588 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic6d5d21ce5267f147b332a4c9b01a29b3b8ccfb8 Gerrit-Change-Number: 44588 Gerrit-PatchSet: 10 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Alec Wang <alec.wang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Marco Chen <marcochen(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Peichao Wang <pwang12(a)lenovo.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Tue, 03 Nov 2020 17:30:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/xeon_sp/acpi: Fix uncore dsdt for multiple cpus
by Marc Jones (Code Review) 03 Nov '20

03 Nov '20

Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47060 ) Change subject: soc/intel/xeon_sp/acpi: Fix uncore dsdt for multiple cpus ...................................................................... soc/intel/xeon_sp/acpi: Fix uncore dsdt for multiple cpus Fix the asl to use CONFIG_MAX_CPUS to create entries for multiple cpu uncores. Don't add the RTxx resource entries multiple times. The function is called for each CPUs. Change-Id: Ia4eb9716ae4bd72fb4eb98649105be629623cbef Signed-off-by: Marc Jones <marcjones(a)sysproconsulting.com> --- M src/soc/intel/xeon_sp/acpi/iiostack.asl M src/soc/intel/xeon_sp/cpx/soc_acpi.c M src/soc/intel/xeon_sp/skx/soc_acpi.c 3 files changed, 11 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/47060/1 diff --git a/src/soc/intel/xeon_sp/acpi/iiostack.asl b/src/soc/intel/xeon_sp/acpi/iiostack.asl index dca5569..4b2b65b 100644 --- a/src/soc/intel/xeon_sp/acpi/iiostack.asl +++ b/src/soc/intel/xeon_sp/acpi/iiostack.asl @@ -69,7 +69,7 @@ MAKE_IIO_DEV(02, 20) MAKE_IIO_DEV(03, 28) -#if MAX_SOCKET > 1 +#if (CONFIG_MAX_SOCKET > 1) MAKE_IIO_DEV(06, 40) MAKE_IIO_DEV(07, 50) MAKE_IIO_DEV(08, 60) diff --git a/src/soc/intel/xeon_sp/cpx/soc_acpi.c b/src/soc/intel/xeon_sp/cpx/soc_acpi.c index d60684b..f055913 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/cpx/soc_acpi.c @@ -60,6 +60,10 @@ { struct iiostack_resource stack_info = {0}; + /* Only add RTxx entries once. */ + if (dev->bus->secondary != 0) + return; + get_iiostack_info(&stack_info); acpigen_write_scope("\\_SB"); diff --git a/src/soc/intel/xeon_sp/skx/soc_acpi.c b/src/soc/intel/xeon_sp/skx/soc_acpi.c index df2550c..0182b18 100644 --- a/src/soc/intel/xeon_sp/skx/soc_acpi.c +++ b/src/soc/intel/xeon_sp/skx/soc_acpi.c @@ -6,6 +6,7 @@ #include <assert.h> #include <cbmem.h> #include <cpu/intel/turbo.h> +#include <device/device.h> #include <device/mmio.h> #include <device/pci.h> #include <intelblocks/acpi.h> @@ -97,13 +98,17 @@ } -void uncore_inject_dsdt(const struct device *device) +void uncore_inject_dsdt(const struct device *dev) { size_t hob_size; const uint8_t uds_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID; const IIO_UDS *hob = fsp_find_extension_hob_by_guid(uds_guid, &hob_size); assert(hob != NULL && hob_size != 0); + /* Only add RTxx entries once. */ + if (dev->bus->secondary != 0) + return; + acpigen_write_scope("\\_SB"); for (int socket = 0; socket < hob->PlatformData.numofIIO; ++socket) { IIO_RESOURCE_INSTANCE iio_resource = -- To view, visit https://review.coreboot.org/c/coreboot/+/47060 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia4eb9716ae4bd72fb4eb98649105be629623cbef Gerrit-Change-Number: 47060 Gerrit-PatchSet: 1 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 5

Change in coreboot[master]: soc/intel/xeon_sp; Use soc specific stack-port function
by Marc Jones (Code Review) 03 Nov '20

03 Nov '20

Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46971 ) Change subject: soc/intel/xeon_sp; Use soc specific stack-port function ...................................................................... soc/intel/xeon_sp; Use soc specific stack-port function Separate the get_stack_for_port into soc specific functions. This removes a #if in common code. Change-Id: Ib38a7d66947ded9b56193a9163e5128b2523e99c Signed-off-by: Marc Jones <marcjones(a)sysproconsulting.com> --- M src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h M src/soc/intel/xeon_sp/cpx/soc_util.c M src/soc/intel/xeon_sp/nb_acpi.c M src/soc/intel/xeon_sp/skx/include/soc/soc_util.h M src/soc/intel/xeon_sp/skx/soc_util.c 5 files changed, 51 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/46971/1 diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h index f0c2575..5f4a6f9 100644 --- a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h @@ -26,5 +26,6 @@ const struct SystemMemoryMapHob *get_system_memory_map(void); void set_bios_init_completion(void); +int soc_get_stack_for_port(int port); #endif /* _SOC_UTIL_H_ */ diff --git a/src/soc/intel/xeon_sp/cpx/soc_util.c b/src/soc/intel/xeon_sp/cpx/soc_util.c index bc4a1e1..de6d852 100644 --- a/src/soc/intel/xeon_sp/cpx/soc_util.c +++ b/src/soc/intel/xeon_sp/cpx/soc_util.c @@ -319,3 +319,25 @@ /* And finally, take care of the SBSP */ set_bios_init_completion_for_package(sbsp_socket_id); } +/* + * EX: CPX-SP + * Ports Stack Stack(HOB) IioConfigIou + * ========================================== + * 0 CSTACK stack 0 IOU0 + * 1A..1D PSTACKZ stack 1 IOU1 + * 2A..2D PSTACK1 stack 2 IOU2 + * 3A..3D PSTACK2 stack 4 IOU3 + */ +int soc_get_stack_for_port(int port) +{ + if (port == PORT_0) + return CSTACK; + else if (port >= PORT_1A && port <= PORT_1D) + return PSTACK0; + else if (port >= PORT_2A && port <= PORT_2D) + return PSTACK1; + else if (port >= PORT_3A && port <= PORT_3D) + return PSTACK2; + else + return -1; +} diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c index 5955fa0..f6bf6ed 100644 --- a/src/soc/intel/xeon_sp/nb_acpi.c +++ b/src/soc/intel/xeon_sp/nb_acpi.c @@ -143,48 +143,6 @@ } /* - * EX: CPX-SP - * Ports Stack Stack(HOB) IioConfigIou - * ========================================== - * 0 CSTACK stack 0 IOU0 - * 1A..1D PSTACKZ stack 1 IOU1 - * 2A..2D PSTACK1 stack 2 IOU2 - * 3A..3D PSTACK2 stack 4 IOU3 - */ -static int get_stack_for_port(int port) -{ -#if (CONFIG(SOC_INTEL_COOPERLAKE_SP)) - if (port == PORT_0) - return CSTACK; - else if (port >= PORT_1A && port <= PORT_1D) - return PSTACK0; - else if (port >= PORT_2A && port <= PORT_2D) - return PSTACK1; - else if (port >= PORT_3A && port <= PORT_3D) - return PSTACK2; - else - return -1; -#endif /* SOC_INTEL_COOPERLAKE_SP */ - -#if (CONFIG(SOC_INTEL_SKYLAKE_SP)) - if (port == PORT_0) - return CSTACK; - else if (port >= PORT_1A && port <= PORT_1D) - return PSTACK0; - else if (port >= PORT_2A && port <= PORT_2D) - return PSTACK1; - else if (port >= PORT_3A && port <= PORT_3D) - return PSTACK2; - else if (port >= PORT_4A && port <= PORT_4D) - return PSTACK3; // MCP0 - else if (port >= PORT_5A && port <= PORT_5D) - return PSTACK4; // MCP1 - else - return -1; -#endif /* SOC_INTEL_SKYLAKE_SP */ -} - -/* * This function adds PCIe bridge device entry in DMAR table. If it is called * in the context of ATSR subtable, it adds ATSR subtable when it is first called. */ @@ -193,7 +151,7 @@ bool is_atsr, bool *first) { - if (get_stack_for_port(port) != stack) + if (soc_get_stack_for_port(port) != stack) return 0; const uint32_t bus = iio_resource.StackRes[stack].BusBase; diff --git a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h index 2566830..cce542a 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/soc_util.h @@ -27,5 +27,6 @@ void set_bios_init_completion(void); unsigned int soc_get_num_cpus(void); +int soc_get_stack_for_port(int port); #endif /* _SOC_UTIL_H_ */ diff --git a/src/soc/intel/xeon_sp/skx/soc_util.c b/src/soc/intel/xeon_sp/skx/soc_util.c index 7dd954f..5c9c3ad 100644 --- a/src/soc/intel/xeon_sp/skx/soc_util.c +++ b/src/soc/intel/xeon_sp/skx/soc_util.c @@ -394,4 +394,30 @@ } } +/* + * EX: CPX-SP + * Ports Stack Stack(HOB) IioConfigIou + * ========================================== + * 0 CSTACK stack 0 IOU0 + * 1A..1D PSTACKZ stack 1 IOU1 + * 2A..2D PSTACK1 stack 2 IOU2 + * 3A..3D PSTACK2 stack 4 IOU3 + */ +int soc_get_stack_for_port(int port) +{ + if (port == PORT_0) + return CSTACK; + else if (port >= PORT_1A && port <= PORT_1D) + return PSTACK0; + else if (port >= PORT_2A && port <= PORT_2D) + return PSTACK1; + else if (port >= PORT_3A && port <= PORT_3D) + return PSTACK2; + else if (port >= PORT_4A && port <= PORT_4D) + return PSTACK3; // MCP0 + else if (port >= PORT_5A && port <= PORT_5D) + return PSTACK4; // MCP1 + else + return -1; +} #endif -- To view, visit https://review.coreboot.org/c/coreboot/+/46971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib38a7d66947ded9b56193a9163e5128b2523e99c Gerrit-Change-Number: 46971 Gerrit-PatchSet: 1 Gerrit-Owner: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

2 5

Change in coreboot[master]: soc/intel/common/block/systemagent/memmap.c: Align cached region
by Angel Pons (Code Review) 03 Nov '20

03 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45930 ) Change subject: soc/intel/common/block/systemagent/memmap.c: Align cached region ...................................................................... soc/intel/common/block/systemagent/memmap.c: Align cached region When asked to place cbmem_top(), FSP does not seem to care about alignment. It can return an address that is MTRR poison, which will exhaust all variable MTRRs when trying to set up caching for CBMEM. This will make memory-mapped flash and TSEG caching fail as well. Safeguard against this by aligning the region to cache to half of its size, and move it upwards to compensate. It is assumed that caching memory above the provided bootloader TOLUM address is inconsequential. TEST=Boot out-of-tree Skylake board, observe no MTRR exhaustion error messages in console. The boot process also feels more fluid. Change-Id: Ic64fd6d3d9e8ab4c78d68b910a476f9c4eb2d353 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/soc/intel/common/block/systemagent/memmap.c 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/45930/1 diff --git a/src/soc/intel/common/block/systemagent/memmap.c b/src/soc/intel/common/block/systemagent/memmap.c index 985f2c4..c59d93e 100644 --- a/src/soc/intel/common/block/systemagent/memmap.c +++ b/src/soc/intel/common/block/systemagent/memmap.c @@ -6,6 +6,7 @@ #include <cpu/x86/mtrr.h> #include <cpu/x86/smm.h> #include <intelblocks/systemagent.h> +#include <types.h> /* * Expected Host Memory Map (we don't know 100% and not all regions are present on all SoCs): @@ -55,18 +56,18 @@ void fill_postcar_frame(struct postcar_frame *pcf) { - uintptr_t top_of_ram; + /* FSP does not seem to bother w.r.t. alignment when asked to place cbmem_top() */ + uintptr_t top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8 * MiB); /* * We need to make sure ramstage will be run cached. At this * point exact location of ramstage in cbmem is not known. - * Instruct postcar to cache 16 megs under cbmem top which is + * Instruct postcar to cache 16 megs around cbmem top which is * a safe bet to cover ramstage. */ - top_of_ram = (uintptr_t) cbmem_top(); printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); - top_of_ram -= 16*MiB; - postcar_frame_add_mtrr(pcf, top_of_ram, 16*MiB, MTRR_TYPE_WRBACK); + + postcar_frame_add_mtrr(pcf, top_of_ram - 8 * MiB, 16 * MiB, MTRR_TYPE_WRBACK); /* Cache the TSEG region */ postcar_enable_tseg_cache(pcf); -- To view, visit https://review.coreboot.org/c/coreboot/+/45930 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic64fd6d3d9e8ab4c78d68b910a476f9c4eb2d353 Gerrit-Change-Number: 45930 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 15

Change in coreboot[master]: driver/usb/acpi: Add power resources for devices on USB ports
by Tim Wawrzynczak (Code Review) 03 Nov '20

03 Nov '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46713 ) Change subject: driver/usb/acpi: Add power resources for devices on USB ports ...................................................................... Patch Set 2: > Patch Set 2: > > (1 comment) > > > Patch Set 2: > > > > (1 comment) > > > > > Patch Set 2: > > > > > > (1 comment) > > > > > > > Patch Set 2: > > > > > > > > (1 comment) > > > > > > > > > Patch Set 2: > > > > > > > > > > (1 comment) > > > > > > > > > > Hmm, this makes me want to attack the power resource problem again... > > > > > With the devtree alias patches in, expressing power resource dependencies is even cleaner, e.g.,: > > > > > > > > > > ``` > > > > > chip drivers/generic/power_resource/ > > > > > register "on" = "{{GPIO_L(GPP_A1), 50}, > > > > > {GPIO_L(GPIO_B2), 1},}" > > > > > register "off" = "{{GPIO_L(GPP_B2), 1}, > > > > > {GPIO(GPP_A1), 10},}" > > > > > # or even the current interface in generic/2c with the stop_gpio, reset_gpio, etc. > > > > > # but I kind of like this explicit sequencing you get here. > > > > > device generic 0 alias i2c1_power_res on end > > > > > end > > > > > chip drivers/i2c/generic > > > > > use i2c1_power_res as power_resource; > > > > > device i2c 1f on end > > > > > end > > > > > chip drivers/spi/generic > > > > > use i2c1_power_res as power_resource; > > > > > device spi 0 on end > > > > > end > > > > > ``` > > > > > > > > > > Each "resource" in the 'on' and 'off' list (i.e., GPIOs) can emit enable/disable methods that use reference counting to keep track of when it's safe to actually assert/deassert the pins. > > > > > > > > > > WDYT? If you don't want to tackle that right now, factoring out the power resource fields into a common `struct power_resource_config` or similar would be better than copy-pasting these fields around into different drivers. > > > > > > > > We might want to eventually do something like you recommended i.e. having a separate power resource device especially if multiple devices have to share power resource. Last week you mentioned that this might probably be a use case for the camera device. > > > > > > > > One thing that we will probably have to think about some more is -- if a driver wants to expose the GPIOs in _CRS in addition to generating the power resource, then it will require both on/off as well as reset/enable GPIOs to be provided by mainboard in the devicetree entry. It might be helpful to write these thoughts in a doc/bug so that we can capture all scenarios. > > > > > > > > About adding a power_resource structure - I think that can be a quick way forward right now especially if Karthik is looking to unblock the mainboard CLs. However, if you plan to refactor and add a new power_resource driver, do you want to wait until then to decide what the structure should really look like? > > > > > > Just a strawman for the moment 😊 > > > > > > There are other considerations too, like ensuring that _PR0/_PR3 get emitted too. I have some thoughts, I should collect them into a doc. > > > > I will start with aggregating the fields into a common power resource structure and a simple wrapper function to generate the ACPI identifiers. This can be organized something like > > drivers/power_resource/chip.h > > drivers/power_resource/power_resource.c > > > > Once we have identified down all the use-cases, I can transform them into a driver covering all the documented use-cases. Does that sound good? > > Using the common power_resource structure in existing drivers (I2C,SPI,UART) is going to require changing all devicetrees to use power_resource.xyz instead of xyz. I think for now let's just maintain the way things are in this CL and do the clean up in one go for all drivers when we want to add a separate power resource device. Let's see what Tim thinks. Agreed -- To view, visit https://review.coreboot.org/c/coreboot/+/46713 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icc1aebfb9e3e646a7f608f0cd391079fd30dd1c0 Gerrit-Change-Number: 46713 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 03 Nov 2020 16:12:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: include/list.h: Add support for GCC9+
by Patrick Rudolph (Code Review) 03 Nov '20

03 Nov '20

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44573 ) Change subject: include/list.h: Add support for GCC9+ ...................................................................... include/list.h: Add support for GCC9+ When getting the address of a structure's member that is not on offset 0, GCC9+ assumes that the address never can be NULL. However the code relied on the fact that it can be NULL by letting the pointer intentionally overflow. Manually calculate the address using uintptr_t. This allows to gracefully terminate the list_for_each MACRO instead of crashing at the end of the list. Tested on qemu-system-arm: coreboot no longer crashed in the devicetree parser and is able to boot Linux 5.5. Change-Id: I0d569b59a23d1269f8575fcbbe92a5a6816aa1f7 Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M src/include/list.h 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44573/1 diff --git a/src/include/list.h b/src/include/list.h index 3944878..6f0b54d 100644 --- a/src/include/list.h +++ b/src/include/list.h @@ -16,10 +16,10 @@ // Insert list_node node before list_node before in a doubly linked list. void list_insert_before(struct list_node *node, struct list_node *before); -#define list_for_each(ptr, head, member) \ - for ((ptr) = container_of((head).next, typeof(*(ptr)), member); \ - &((ptr)->member); \ - (ptr) = container_of((ptr)->member.next, \ +#define list_for_each(ptr, head, member) \ + for ((ptr) = container_of((head).next, typeof(*(ptr)), member); \ + (uintptr_t)ptr + (uintptr_t)offsetof(typeof(*(ptr)), member); \ + (ptr) = container_of((ptr)->member.next, \ typeof(*(ptr)), member)) #endif /* __LIST_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/44573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0d569b59a23d1269f8575fcbbe92a5a6816aa1f7 Gerrit-Change-Number: 44573 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

7 12

Change in coreboot[master]: util/cbfstool: Add Alignment for CBnT
by Christian Walter (Code Review) 03 Nov '20

03 Nov '20

Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45652 ) Change subject: util/cbfstool: Add Alignment for CBnT ...................................................................... util/cbfstool: Add Alignment for CBnT Change-Id: Ibd94539a6025e7c3912730a6d90454379d85fb59 Signed-off-by: Christian Walter <christian.walter(a)9elements.com> --- M util/cbfstool/cbfstool.c M util/cbfstool/fit.c 2 files changed, 27 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/45652/1 diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index d2df1cc..ce72835 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -70,6 +70,7 @@ bool machine_parseable; bool unprocessed; bool ibb; + bool cbnt; enum comp_algo compression; int precompression; enum vb2_hash_algorithm hash; @@ -551,6 +552,18 @@ len_align = 16; } + if (param.cbnt) { + ERROR("MARKING ENTRY AS CBNT\n"); + /* Mark as Initial Boot Block */ + struct cbfs_file_attribute *attrs = cbfs_add_file_attr(header, + CBFS_FILE_ATTR_TAG_IBB, + sizeof(struct cbfs_file_attribute)); + if (attrs == NULL) + return -1; + /* For Intel CBNT minimum align is 64 */ + len_align = 64; + } + if (param.padding) { const uint32_t hs = sizeof(struct cbfs_file_attribute); uint32_t size = MAX(hs, param.padding); @@ -1290,6 +1303,7 @@ /* begin after ASCII characters */ LONGOPT_START = 256, LONGOPT_IBB = LONGOPT_START, + LONGOPT_CBNT = 257, LONGOPT_END, }; @@ -1333,6 +1347,7 @@ {"mach-parseable",no_argument, 0, 'k' }, {"unprocessed", no_argument, 0, 'U' }, {"ibb", no_argument, 0, LONGOPT_IBB }, + {"cbnt", no_argument, 0, LONGOPT_CBNT }, {NULL, 0, 0, 0 } }; @@ -1406,7 +1421,7 @@ " add [-r image,regions] -f FILE -n NAME -t TYPE [-A hash] \\\n" " [-c compression] [-b base-address | -a alignment] \\\n" " [-p padding size] [-y|--xip if TYPE is FSP] \\\n" - " [-j topswap-size] (Intel CPUs only) [--ibb] " + " [-j topswap-size] (Intel CPUs only) [--ibb] [--cbnt] " "Add a component\n" " " " -j valid size: 0x10000 0x20000 0x40000 0x80000 0x100000 \n" @@ -1417,7 +1432,7 @@ " add-stage [-r image,regions] -f FILE -n NAME [-A hash] \\\n" " [-c compression] [-b base] [-S section-to-ignore] \\\n" " [-a alignment] [-P page-size] [-Q|--pow2page] \\\n" - " [-y|--xip] [--ibb] " + " [-y|--xip] [--ibb] [--cbnt] " "Add a stage to the ROM\n" " add-flat-binary [-r image,regions] -f FILE -n NAME \\\n" " [-A hash] -l load-address -e entry-point \\\n" @@ -1750,6 +1765,9 @@ case LONGOPT_IBB: param.ibb = true; break; + case LONGOPT_CBNT: + param.cbnt = true; + break; case 'h': case '?': usage(argv[0]); diff --git a/util/cbfstool/fit.c b/util/cbfstool/fit.c index e7aa8d7..b04cf90 100644 --- a/util/cbfstool/fit.c +++ b/util/cbfstool/fit.c @@ -463,9 +463,10 @@ uint32_t boot_policy_size) { entry->address = boot_policy_addr; - /* Boot Policy Manifest size is not multiple of 16 bytes */ - fit_entry_update_size(entry, boot_policy_size << 4); entry->type_checksum_valid = FIT_TYPE_BOOT_POLICY; + /* Boot Policy Manifest size is not multiple of 16 bytes */ + fit_entry_update_size(entry, boot_policy_size); + entry->size_reserved = boot_policy_size; entry->version = FIT_TXT_VERSION; entry->checksum = 0; fit_entry_add_size(&fit->header, sizeof(struct fit_entry)); @@ -482,9 +483,11 @@ uint32_t key_manifest_size) { entry->address = key_manifest_addr; - /* Key Manifest size is not multiple of 16 bytes */ - fit_entry_update_size(entry, key_manifest_size << 4); + entry->type_checksum_valid = FIT_TYPE_KEY_MANIFEST; + /* Key Manifest size is not multiple of 16 bytes */ + fit_entry_update_size(entry, key_manifest_size); + entry->size_reserved = key_manifest_size; entry->version = FIT_TXT_VERSION; entry->checksum = 0; fit_entry_add_size(&fit->header, sizeof(struct fit_entry)); -- To view, visit https://review.coreboot.org/c/coreboot/+/45652 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibd94539a6025e7c3912730a6d90454379d85fb59 Gerrit-Change-Number: 45652 Gerrit-PatchSet: 1 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-MessageType: newchange

3 2

Change in coreboot[master]: soc/intel/jasperlake: Correct GPIO pad sequence for community pad group
by Furquan Shaikh (Code Review) 03 Nov '20

03 Nov '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46842 ) Change subject: soc/intel/jasperlake: Correct GPIO pad sequence for community pad group ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… File src/soc/intel/jasperlake/gpio.c: https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… PS6, Line 37: INTEL_GPP(GPP_F0, GPIO_RSVD_0, GPIO_RSVD_8), BTW, I think the kernel has a bug: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/t… It cannot skip these 9 reserved GPIOs when numbering the pads in community 0. That will cause it to calculate the DW0-2 config register offsets incorrectly for the pads after this in the community. I think if you try to make the kernel configure any of the DW registers for a pad in GPP_A/S/R, you will see that it is writing to the wrong offset. Can you please follow up on this? If the experiment proves that the behavior is wrong, this will have to be fixed in the kernel as well. -- To view, visit https://review.coreboot.org/c/coreboot/+/46842 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6013914c88c50f4b8c60ca9a9285a8e1b214d11 Gerrit-Change-Number: 46842 Gerrit-PatchSet: 6 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 03 Nov 2020 07:33:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/jasperlake: Correct GPIO pad sequence for community pad group
by Furquan Shaikh (Code Review) 03 Nov '20

03 Nov '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46842 ) Change subject: soc/intel/jasperlake: Correct GPIO pad sequence for community pad group ...................................................................... Patch Set 6: (5 comments) https://review.coreboot.org/c/coreboot/+/46842/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/46842/6//COMMIT_MSG@27 PS6, Line 27: Correcting the sequence by moving some of the reserved gpio within : community (that's also same in pinctrl driver), helps with the issue. What you are really doing is creating pad groups as per the EDS. That is also what the linux kernel driver does. https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… File src/soc/intel/jasperlake/gpio.c: https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… PS6, Line 36: 0 This is not correct. It should be 320: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/t… https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… PS6, Line 38: GPIO_RSVD_10 Not for this change, but I think this GPIO should be named as per EDS. The name is already being used in kernel, so I don't think there is any problem to update it in coreboot too. https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… PS6, Line 39: GPIO_RSVD_11 Same here and the other GPIO_RSVD* below. https://review.coreboot.org/c/coreboot/+/46842/6/src/soc/intel/jasperlake/g… PS6, Line 64: 320 This is 0 as per the above kernel link. -- To view, visit https://review.coreboot.org/c/coreboot/+/46842 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6013914c88c50f4b8c60ca9a9285a8e1b214d11 Gerrit-Change-Number: 46842 Gerrit-PatchSet: 6 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 03 Nov 2020 07:00:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: configs: Add a weird config for Portwell M107
by Angel Pons (Code Review) 03 Nov '20

03 Nov '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45974 ) Change subject: configs: Add a weird config for Portwell M107 ...................................................................... configs: Add a weird config for Portwell M107 This is not meant for actual use, but to build-test several options. Please do not try to use it on real hardware. Or maybe do try. Change-Id: Ife40d055e4c9b295c54cfc6a27af06e9358f7761 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A configs/config.portwell_m107.debug_smmstore_oxpcie_em100spi 1 file changed, 38 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/45974/1 diff --git a/configs/config.portwell_m107.debug_smmstore_oxpcie_em100spi b/configs/config.portwell_m107.debug_smmstore_oxpcie_em100spi new file mode 100644 index 0000000..448193d --- /dev/null +++ b/configs/config.portwell_m107.debug_smmstore_oxpcie_em100spi @@ -0,0 +1,38 @@ +# Not meant for actual use. Exercises, among other things: +# + SMMSTORE +# + OXPCIE support +# + FSP MP init +# + EM100Pro SPI console +# + Debug options +CONFIG_VENDOR_PORTWELL=y +CONFIG_CONSOLE_POST=y +# CONFIG_CONSOLE_SERIAL is not set +CONFIG_ENABLE_BUILTIN_COM1=y +CONFIG_ONBOARD_MEM_KINGSTON=y +CONFIG_USE_INTEL_FSP_MP_INIT=y +CONFIG_SOC_INTEL_COMMON_BLOCK_SMM_TCO_ENABLE=y +CONFIG_SOC_INTEL_DEBUG_CONSENT=y +CONFIG_PCIEXP_HOTPLUG=y +CONFIG_PCIEXP_HOTPLUG_PREFETCH_MEM_BELOW_4G=y +CONFIG_SOFTWARE_I2C=y +CONFIG_SMMSTORE=y +CONFIG_SPI_FLASH_NO_FAST_READ=y +CONFIG_DRIVERS_UART_OXPCIE=y +CONFIG_DRIVERS_GENESYSLOGIC_GL9755=y +CONFIG_DISPLAY_HOBS=y +CONFIG_DISPLAY_VBT=y +CONFIG_DISPLAY_FSP_ENTRY_POINTS=y +CONFIG_DISPLAY_UPD_DATA=y +CONFIG_EM100PRO_SPI_CONSOLE=y +CONFIG_DISPLAY_MTRRS=y +CONFIG_GDB_STUB=y +CONFIG_GDB_WAIT=y +CONFIG_FATAL_ASSERTS=y +CONFIG_DEBUG_CBFS=y +CONFIG_DEBUG_SMBUS=y +CONFIG_DEBUG_SMI=y +CONFIG_DEBUG_PERIODIC_SMI=y +CONFIG_DEBUG_MALLOC=y +CONFIG_DEBUG_CONSOLE_INIT=y +CONFIG_REALMODE_DEBUG=y +CONFIG_DEBUG_BOOT_STATE=y -- To view, visit https://review.coreboot.org/c/coreboot/+/45974 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ife40d055e4c9b295c54cfc6a27af06e9358f7761 Gerrit-Change-Number: 45974 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 9

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020