coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: mb/google/guybrush: Move variant_has_fpmcu() after eSPI init
by Martin Roth (Code Review) 31 May '21

31 May '21

Attention is currently required from: Raul Rangel, Furquan Shaikh, Martin Roth. Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Felix Held, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54740 to look at the new patch set (#3). Change subject: mb/google/guybrush: Move variant_has_fpmcu() after eSPI init ...................................................................... mb/google/guybrush: Move variant_has_fpmcu() after eSPI init Currently variant_has_fpmcu() is called very early in bootblock, before eSPI is initialized. When checking CBI for its presence, that causes an error and nothing else can be read from CBI in bootblock. Moving it slightly later in bootblock doesn't hurt anything from a timing standpoint, and allows CBI to be read. BUG=None TEST=See CBI get read and the FPMCU field read correctly. Signed-off-by: Martin Roth <martinroth(a)chromium.org> Change-Id: I6de44119e92c8820b266f9f07287706c7d4eb505 --- M src/mainboard/google/guybrush/bootblock.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/54740/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/54740 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6de44119e92c8820b266f9f07287706c7d4eb505 Gerrit-Change-Number: 54740 Gerrit-PatchSet: 3 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.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-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/brya: Create primus variant
by Scott Chao (Code Review) 31 May '21

31 May '21

Scott Chao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55086 ) Change subject: mb/google/brya: Create primus variant ...................................................................... mb/google/brya: Create primus variant Create the primus variant of the brya0 reference board by copying the template files to a new directory named for the variant. (Auto-generated by create_coreboot_variant.sh version 4.5.0) BUG=b:188272162 BRANCH=None TEST=util/abuild/abuild -p none -t google/brya -x -a make sure the build includes GOOGLE_PRIMUS Signed-off-by: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Change-Id: I969c9b9dd15d95c11ab8d6a04c7d5a87f4f8a222 --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/Kconfig.name A src/mainboard/google/brya/variants/primus/include/variant/ec.h A src/mainboard/google/brya/variants/primus/include/variant/gpio.h A src/mainboard/google/brya/variants/primus/memory/Makefile.inc A src/mainboard/google/brya/variants/primus/memory/dram_id.generated.txt A src/mainboard/google/brya/variants/primus/memory/mem_parts_used.txt A src/mainboard/google/brya/variants/primus/overridetree.cb 8 files changed, 46 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/55086/1 diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index c28c293..3acb07e 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -72,10 +72,12 @@ config MAINBOARD_PART_NUMBER string default "Brya" if BOARD_GOOGLE_BRYA0 + default "Primus" if BOARD_GOOGLE_PRIMUS config VARIANT_DIR string default "brya0" if BOARD_GOOGLE_BRYA0 + default "primus" if BOARD_GOOGLE_PRIMUS config DIMM_SPD_SIZE int diff --git a/src/mainboard/google/brya/Kconfig.name b/src/mainboard/google/brya/Kconfig.name index 70af2d2..4bf5ca4 100644 --- a/src/mainboard/google/brya/Kconfig.name +++ b/src/mainboard/google/brya/Kconfig.name @@ -5,3 +5,8 @@ select BOARD_ROMSIZE_KB_32768 select ADL_ENABLE_USB4_PCIE_RESOURCES select DRIVERS_GENESYSLOGIC_GL9755 + +config BOARD_GOOGLE_PRIMUS + bool "-> Primus" + select BOARD_GOOGLE_BASEBOARD_BRYA + select BASEBOARD_BRYA_LAPTOP diff --git a/src/mainboard/google/brya/variants/primus/include/variant/ec.h b/src/mainboard/google/brya/variants/primus/include/variant/ec.h new file mode 100644 index 0000000..7a2a6ff --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/include/variant/ec.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef __VARIANT_EC_H__ +#define __VARIANT_EC_H__ + +#include <baseboard/ec.h> + +#endif diff --git a/src/mainboard/google/brya/variants/primus/include/variant/gpio.h b/src/mainboard/google/brya/variants/primus/include/variant/gpio.h new file mode 100644 index 0000000..c4fe342 --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/include/variant/gpio.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef VARIANT_GPIO_H +#define VARIANT_GPIO_H + +#include <baseboard/gpio.h> + +#endif diff --git a/src/mainboard/google/brya/variants/primus/memory/Makefile.inc b/src/mainboard/google/brya/variants/primus/memory/Makefile.inc new file mode 100644 index 0000000..b0ca222 --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/memory/Makefile.inc @@ -0,0 +1,5 @@ +## SPDX-License-Identifier: GPL-2.0-or-later +## This is an auto-generated file. Do not edit!! +## Add memory parts in mem_parts_used.txt and run spd_tools to regenerate. + +SPD_SOURCES = placeholder.spd.hex diff --git a/src/mainboard/google/brya/variants/primus/memory/dram_id.generated.txt b/src/mainboard/google/brya/variants/primus/memory/dram_id.generated.txt new file mode 100644 index 0000000..fa24790 --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/memory/dram_id.generated.txt @@ -0,0 +1 @@ +DRAM Part Name ID to assign diff --git a/src/mainboard/google/brya/variants/primus/memory/mem_parts_used.txt b/src/mainboard/google/brya/variants/primus/memory/mem_parts_used.txt new file mode 100644 index 0000000..9cff262 --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/memory/mem_parts_used.txt @@ -0,0 +1,11 @@ +# This is a CSV file containing a list of memory parts used by this variant. +# One part per line with an optional fixed ID in column 2. +# Only include a fixed ID if it is required for legacy reasons! +# Generated IDs are dependent on the order of parts in this file, +# so new parts must always be added at the end of the file! +# +# Generate an updated Makefile.inc and dram_id.generated.txt by running the +# gen_part_id tool from util/spd_tools/lp4x. +# See util/spd_tools/lp4x/README.md for more details and instructions. + +# Part Name diff --git a/src/mainboard/google/brya/variants/primus/overridetree.cb b/src/mainboard/google/brya/variants/primus/overridetree.cb new file mode 100644 index 0000000..4f2c04a --- /dev/null +++ b/src/mainboard/google/brya/variants/primus/overridetree.cb @@ -0,0 +1,6 @@ +chip soc/intel/alderlake + + device domain 0 on + end + +end -- To view, visit https://review.coreboot.org/c/coreboot/+/55086 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I969c9b9dd15d95c11ab8d6a04c7d5a87f4f8a222 Gerrit-Change-Number: 55086 Gerrit-PatchSet: 1 Gerrit-Owner: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: drivers/intel/fsp1_1/romstage.c: Remove MCU update
by Matt DeVillier (Code Review) 30 May '21

30 May '21

Attention is currently required from: Frans Hendriks, Lee Leahy, Paul Menzel, Huang Jin, Arthur Heymans, Patrick Rudolph, Wim Vervoorn. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55064 ) Change subject: drivers/intel/fsp1_1/romstage.c: Remove MCU update ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: boot tested on google/swanky -- To view, visit https://review.coreboot.org/c/coreboot/+/55064 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I72c7b821e04169ae237d8adb6a8348f06e87b047 Gerrit-Change-Number: 55064 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Huang Jin <huang.jin(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Wim Vervoorn <wvervoorn(a)eltan.com> Gerrit-Comment-Date: Sun, 30 May 2021 21:50:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add new IRQ module
by Furquan Shaikh (Code Review) 30 May '21

30 May '21

Attention is currently required from: Nico Huber, Tim Wawrzynczak. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49408 ) Change subject: soc/intel/common: Add new IRQ module ...................................................................... Patch Set 27: (18 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/49408/comment/bec10b0d_487aecb3 PS27, Line 7: Add new IRQ module As I was reading irq.c file again, I realized that majority of the work that is being done there doesn't really need to happen at runtime for every boot. Ideally, what can be done is: 1. Create a constraint table for SoC 2. Run it through a tool that emits a .c file with `pci_irq_entry` table. Only the slots that use direct irq route will have to be left empty since the GPIO table is unknown at that time. 3. Integrate this generated .c file into coreboot build. 4. At runtime, only calculate direct irq i.e. only find_free_unique_irq is required. The logic that you have implemented in irq.c is the important piece. Once that is finalized, it can be reused as is in the tool later on. The current direction in this change is okay. The suggestion for the tool is mostly for a follow up change. File src/soc/intel/common/block/include/intelblocks/irq.h: https://review.coreboot.org/c/coreboot/+/49408/comment/a622f7e1_dbcff318 PS27, Line 6: #include <device/device.h> : #include <device/pci_def.h> : #include <device/pci.h> : #include <southbridge/intel/common/acpi_pirq_gen.h> nit: These are most likely not required in this file. https://review.coreboot.org/c/coreboot/+/49408/comment/5f6b979f_03a6fe0f PS27, Line 16: BIT(0) Empty fields will have a value of 0, right? I am assuming that empty fields basically means that an empty constraint that the SoC did not fill in the constraints table probably because there is no slot/function present. https://review.coreboot.org/c/coreboot/+/49408/comment/79e5c35a_0be88375 PS27, Line 54: const struct soc_irq_constraints *soc_irq_constraints(void); This is unused and can be dropped. File src/soc/intel/common/block/irq/Kconfig: https://review.coreboot.org/c/coreboot/+/49408/comment/b6d58cc3_89f74878 PS27, Line 6: . and exposed to OS in ACPI tables? File src/soc/intel/common/block/irq/irq.c: https://review.coreboot.org/c/coreboot/+/49408/comment/a6488584_44e7e714 PS26, Line 198: constraints[i].type == FIXED_INT_PIN || constraints[i].type == IDENTITY > Interesting thought; IDENTITY is currently only applicable for PCIe RPs True. But, there is nothing that restricts the use of identity for any other slot function. In fact, identity is just a special case of saying that the slot function uses a fixed PIRQ. Also, currently, multiple of the constraint types are already required to be set, but are being implicitly assumed. Example: In assign_fixed_pins, you have: ``` if (constraints[i].type != FIXED_INT_PIN && constraints[i].type != IDENTITY) continue; ``` Basically, identity assumes a fixed pin as well. What we really need to know for each slot function is: 1. Does it need a fixed int pin? If yes, which pin? 2. Does it need a fixed pirq? If yes, which pirq? 3. What route does the irq use - none (empty function), pirq, direct irq. If we encode these at the SoC level, then there is no need to define "unique", "identity", etc. Building up a little more on these 3 questions, I am adding a comment here: https://review.coreboot.org/c/coreboot/+/49408/27/src/soc/intel/common/bloc… File src/soc/intel/common/block/irq/irq.c: https://review.coreboot.org/c/coreboot/+/49408/comment/9460b6f9_35b23697 PS27, Line 4: #include <device/device.h> Is this required? https://review.coreboot.org/c/coreboot/+/49408/comment/3cac5d05_14624b97 PS27, Line 5: pci pci_type.h? https://review.coreboot.org/c/coreboot/+/49408/comment/a0fc83cf_a12933d5 PS27, Line 8: #include <soc/irq.h> Is this required? https://review.coreboot.org/c/coreboot/+/49408/comment/616b751c_b0c7fb87 PS27, Line 22: MAX_ENTRIES Just curious: How was the max value of 64 determined? https://review.coreboot.org/c/coreboot/+/49408/comment/d223bda4_85f6e916 PS27, Line 27: SHARED_PIN nit: `SHARED_IRQ_PIN` to keep the name consistent with `UNIQUE_IRQ_PIN`? https://review.coreboot.org/c/coreboot/+/49408/comment/82cce2cf_420b0542 PS27, Line 31: bool identity_map; Based on comment https://review.coreboot.org/c/coreboot/+/49408/27/src/soc/intel/common/bloc…, this will not be required. Instead, we should maintain a pirq mapping here. ``` enum pirq pirq; ``` It will have to be set to PIRQ_INVALID by default and initialized to appropriate pirq based on: - fixed_pirq, or - pirq assigned as part of find_global_least_used_pirq https://review.coreboot.org/c/coreboot/+/49408/comment/96ddf3d4_b79a512e PS27, Line 128: const struct pin_info pin_info[PCI_INT_MAX] This is unused now. https://review.coreboot.org/c/coreboot/+/49408/comment/1e3f538e_c51c8078 PS27, Line 161: const unsigned int pin_idx = PIN2IDX(pin); : const unsigned int fn = PCI_FUNC(constraints[i].devfn); : fn_pin_map[fn] = pin; : pin_info[pin_idx].pin_state = SHARED_PIN; : pin_info[pin_idx].usage_count++; : : if (constraints[i].type == IDENTITY) : pin_info[pin_idx].identity_map = true; This part is actually repeated for all three pin assignment functions. We should create a helper function `assign_pin()` that performs all required checks and assigns pin(passed in from caller). Something like: 1. Sanity check pin (PCI_INT_A <= pin <= PCI_INT_D) 2. If irq route is PIRQ, reqd pin state is SHARED_IRQ_PIN, else UNIQUE_IRQ_PIN. 3. Check current pin state is okay to be updated to reqd pin state. 4. Update usage count and current pin state. Then each of the pin assignment functions can call `assign_pin()` to perform the same checks and work. Similarly, we should add `assign_pirq()` that can be used to perform required checks and assign pirq passed in by the caller (either from `fixed_pirq` in constraints or using `find_global_least_used_pirq()`). https://review.coreboot.org/c/coreboot/+/49408/comment/faab08a6_b53b9775 PS27, Line 228: assign_fixed_pins Can you please add a comment here that the order in which the pins are assigned matters so that stricter constraints are resolved first? FIXED -> UNIQUE -> SHARED https://review.coreboot.org/c/coreboot/+/49408/comment/3198a020_0c5fb594 PS27, Line 313: (entry_count < 0) I don't see entry_count being set to < 0 ever. Did you mean to do that on line 324? https://review.coreboot.org/c/coreboot/+/49408/comment/bf113f26_7c4fdf5a PS27, Line 314: return entries; nit: In case of error (entry_count < 0), entries is being returned here but NULL on line 325. Should this function be consistent about the return value for entries in case of error? https://review.coreboot.org/c/coreboot/+/49408/comment/207edb77_96ce08bd PS27, Line 319: UNKNOWN UNKNOWN is defined as (1 << 0). However, in the follow up CLs, empty slots are left uninitialized and thus would be set to 0. This check never really satisfies. Also, do we really need to allocate a table for every slot(0-31)? Would it work if we just encode available slots and encode the constraints for each function (empty or used) for that slot? (This is mostly a nit. If you think having a table for all slots is simpler to understand and manage, I think that's fine too). Combining this with the comment I made on patchset#26(https://review.coreboot.org/c/coreboot/+/49408/comment/d065dcfb… I am thinking something like this: ``` struct soc_irq_constraints { unsigned int slot; struct { enum pci_pin fixed_int_pin; enum pirq fixed_pirq; enum { IRQ_NONE = 0, // No IRQ assigned -- empty function IRQ_PIRQ = 1, // Use PIRQ routing - will be assigned shared IRQ 16-23 IRQ_DIRECT = 2, // No PIRQ routing - will be assigned unique IRQ > 23 } irq_route; } fns[MAX_FNS]; }; ``` and the table can be something like(copying a few entries from follow up TGL CL): ``` static const struct soc_irq_constraints irq_constraints[] = { { .slot = SA_DEV_SLOT_IGD, .fns = { [PCI_FUNC(SA_DEVFN_IGD)] = { .irq_route = IRQ_PIRQ, }, }, }, { .slot = SA_DEV_SLOT_TBT, .fns = { [PCI_FUNC(SA_DEVFN_TBT0)] = { .fixed_int_pin = PCI_INT_A, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(SA_DEVFN_TBT1)] = { .fixed_int_pin = PCI_INT_B, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(SA_DEVFN_TBT2)] = { .fixed_int_pin = PCI_INT_C, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(SA_DEVFN_TBT3)] = { .fixed_int_pin = PCI_INT_D, .irq_route = IRQ_PIRQ, }, }, }, { .slot = PCH_DEV_SLOT_ISH, .fns = { [PCI_FUNC(PCH_DEVFN_ISH)] = { .irq_route = IRQ_DIRECT, }, [PCI_FUNC(PCH_DEVFN_GSPI2)] = { .irq_route = IRQ_DIRECT, }, }, }, { .slot = PCH_DEV_SLOT_PCIE, .fns = { [PCI_FUNC(PCH_DEVFN_PCIE1)] = { .fixed_int_pin = PCI_INT_A, .fixed_pirq = PIRQ_A, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE2)] = { .fixed_int_pin = PCI_INT_B, .fixed_pirq = PIRQ_B, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE3)] = { .fixed_int_pin = PCI_INT_C, .fixed_pirq = PIRQ_C, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE4)] = { .fixed_int_pin = PCI_INT_D, .fixed_pirq = PIRQ_D, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE5)] = { .fixed_int_pin = PCI_INT_A, .fixed_pirq = PIRQ_A, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE6)] = { .fixed_int_pin = PCI_INT_B, .fixed_pirq = PIRQ_B, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE7)] = { .fixed_int_pin = PCI_INT_C, .fixed_pirq = PIRQ_C, .irq_route = IRQ_PIRQ, }, [PCI_FUNC(PCH_DEVFN_PCIE8)] = { .fixed_int_pin = PCI_INT_D, .fixed_pirq = PIRQ_D, .irq_route = IRQ_PIRQ, }, }, } ``` We can also provide some helper macros to fill the constraint entries, but I am not sure if it makes things easier to read: ``` #define ANY_PIRQ(x) [PCI_FUNC(x)] = { .fixed_int_pin = PCI_INT_NONE, \ .fixed_pirq = PIRQ_INVALID, \ .irq_route = IRQ_PIRQ, } #define FIXED_INT_ANY_PIRQ(x, pn) [PCI_FUNC(x)] = { .fixed_int_pin = pn, \ .fixed_pirq = PIRQ_INVALID, \ .irq_route = IRQ_PIRQ, } #define DIRECT_IRQ(x) [PCI_FUNC(x)] = { .fixed_int_pin = PCI_INT_NONE, \ .fixed_pirq = PIRQ_INVALID, \ .irq_route = IRQ_DIRECT, } #define FIXED_INT_PIRQ(x, pn, pq) [PCI_FUNC(x)] = { .fixed_int_pin = pn, \ .fixed_pirq = pq, \ .irq_route = IRQ_PIRQ, } ``` Above table would look something like: ``` static const struct soc_irq_constraints irq_constraints[] = { { .slot = SA_DEV_SLOT_IGD, .fns = { ANY_PIRQ(SA_DEVFN_IGD), }, }, { .slot = SA_DEV_SLOT_TBT, .fns = { FIXED_INT_ANY_PIRQ(SA_DEVFN_TBT0, PCI_INT_A), FIXED_INT_ANY_PIRQ(SA_DEVFN_TBT1, PCI_INT_B), FIXED_INT_ANY_PIRQ(SA_DEVFN_TBT2, PCI_INT_C), FIXED_INT_ANY_PIRQ(SA_DEVFN_TBT3, PCI_INT_D), }, { .slot = PCH_DEV_SLOT_ISH, .fns = { DIRECT_IRQ(PCH_DEVFN_ISH), DIRECT_IRQ(PCH_DEVFN_GSPI2), }, }, { .slot = PCH_DEV_SLOT_PCIE, .fns = { FIXED_INT_PIRQ(PCH_DEVFN_PCIE1, PCI_INT_A, PIRQ_A), FIXED_INT_PIRQ(PCH_DEVFN_PCIE2, PCI_INT_B, PIRQ_B), FIXED_INT_PIRQ(PCH_DEVFN_PCIE3, PCI_INT_C, PIRQ_C), FIXED_INT_PIRQ(PCH_DEVFN_PCIE4, PCI_INT_D, PIRQ_D), FIXED_INT_PIRQ(PCH_DEVFN_PCIE5, PCI_INT_A, PIRQ_A), FIXED_INT_PIRQ(PCH_DEVFN_PCIE6, PCI_INT_B, PIRQ_B), FIXED_INT_PIRQ(PCH_DEVFN_PCIE7, PCI_INT_C, PIRQ_C), FIXED_INT_PIRQ(PCH_DEVFN_PCIE8, PCI_INT_D, PIRQ_D), }, } ``` The benefit of having macros for irq constraints is if in the future there is a need to support more fields in constraints or make any changes to current fields, it can be done within the macros without impacting SoC code. e.g. ``` #define FIXED_INT_DIRECT_IRQ(x, p) { .fixed_int_pin = p, \ .fixed_pirq = PIRQ_INVALID, \ .irq_route = IRQ_DIRECT, } ``` It also reduces chances of mistakes in SoC code since the individual constraint fields are not initialized directly. -- To view, visit https://review.coreboot.org/c/coreboot/+/49408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c22a08ce589fa80d0bb1e637422304a3af2045c Gerrit-Change-Number: 49408 Gerrit-PatchSet: 27 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Sun, 30 May 2021 21:45:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/rx6110sa: Add a Kconfig switch to disable ACPI support
by Paul Menzel (Code Review) 30 May '21

30 May '21

Attention is currently required from: Werner Zeh. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55003 ) Change subject: drivers/i2c/rx6110sa: Add a Kconfig switch to disable ACPI support ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55003/comment/e7a01470_1489951b PS3, Line 18: RTC driver. Which Linux commit added ACPI support for the RTC driver? -- To view, visit https://review.coreboot.org/c/coreboot/+/55003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic65794d409d13a78d17275c86ec14ee6f04cd2a6 Gerrit-Change-Number: 55003 Gerrit-PatchSet: 3 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Sun, 30 May 2021 21:16:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC]device/pnp_device: Disable IO resources if base == 0
by Paul Menzel (Code Review) 30 May '21

30 May '21

Attention is currently required from: Arthur Heymans. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54686 ) Change subject: [RFC]device/pnp_device: Disable IO resources if base == 0 ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: No idea, if should fix the Asus F2A85-M PRO boot issue fixed by Idec3832d8d1ab95ae9747ce87792ee7b474540c7 (sb/amd/hudson: Skip setting up LPC decode for base < 0x20). Just as a heads-up, this change-set here does not fix the issue. it still hangs after: […] PCI: 00:14.2 init finished in 0 msecs PCI: 00:14.3 init Full log: https://paste.debian.net/1199430/ [1]: https://review.coreboot.org/c/coreboot/+/54669 -- To view, visit https://review.coreboot.org/c/coreboot/+/54686 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aeb20f0448760f1e9a23eb45288fbf7f1bbedec Gerrit-Change-Number: 54686 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> 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)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sun, 30 May 2021 21:12:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Add placeholder SPD file
by 9elements QA (Code Review) 30 May '21

30 May '21

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55021 ) Change subject: soc/intel/alderlake: Add placeholder SPD file ...................................................................... Patch Set 2: Automatic boot test returned (PASS/FAIL/TOTAL): 12 / 1 / 13 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/60800 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/60799 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/60798 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/60797 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/60796 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/60795 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/60794 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/60793 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/60792 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/60791 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/60790 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/60789 FAIL: aarch64 "QEMU AArch64" , build config EMULATION_QEMU_AARCH64_FIT_SUPPORT_TIMESTAMPS and payload LinuxBoot_u-root_kexec : https://lava.9esec.io/r/60788 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/55021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38eb4bb684c511fff5ae148091c066682e9c35cb Gerrit-Change-Number: 55021 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Sun, 30 May 2021 20:56:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common/block/espi: Explicitly assert PLTRST#
by Patrick Georgi (Code Review) 30 May '21

30 May '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/54884 ) Change subject: soc/amd/common/block/espi: Explicitly assert PLTRST# ...................................................................... soc/amd/common/block/espi: Explicitly assert PLTRST# PLTRST# is currently asserted and latched when eSPI_RST# gets asserted. If eSPI_RST# isn't used on a platform or it doesn't properly assert in all cases, then PLTRST# will never be asserted. This could result in the AP and EC being out of sync. BUG=b:188188172, b:188935533 TEST=Warm reset guybrush with partial #22 rework. Verify that peripheral channel is correctly reset. Signed-off-by: Raul E Rangel <rrangel(a)chromium.org> Change-Id: I20d12edf3efc6100096e24aa8d1aec76bbde264f Reviewed-on: https://review.coreboot.org/c/coreboot/+/54884 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Rob Barnes <robbarnes(a)google.com> --- M src/soc/amd/common/block/lpc/espi_util.c 1 file changed, 11 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Rob Barnes: Looks good to me, approved diff --git a/src/soc/amd/common/block/lpc/espi_util.c b/src/soc/amd/common/block/lpc/espi_util.c index e6ae2cb..b724408 100644 --- a/src/soc/amd/common/block/lpc/espi_util.c +++ b/src/soc/amd/common/block/lpc/espi_util.c @@ -563,7 +563,7 @@ return espi_send_command(&cmd); } -static int espi_send_pltrst_deassert(const struct espi_config *mb_cfg) +static int espi_send_pltrst(const struct espi_config *mb_cfg, bool assert) { struct espi_cmd cmd = { .hdr0 = { @@ -573,7 +573,8 @@ }, .data = { .byte0 = ESPI_VW_INDEX_SYSTEM_EVENT_3, - .byte1 = ESPI_VW_SIGNAL_HIGH(ESPI_VW_PLTRST), + .byte1 = assert ? ESPI_VW_SIGNAL_LOW(ESPI_VW_PLTRST) + : ESPI_VW_SIGNAL_HIGH(ESPI_VW_PLTRST), }, }; @@ -1006,9 +1007,15 @@ return -1; } + /* Assert PLTRST# if VW channel is enabled by mainboard. */ + if (espi_send_pltrst(cfg, true) == -1) { + printk(BIOS_ERR, "Error: PLTRST# assertion failed!\n"); + return -1; + } + /* De-assert PLTRST# if VW channel is enabled by mainboard. */ - if (espi_send_pltrst_deassert(cfg) == -1) { - printk(BIOS_ERR, "Error: PLTRST deassertion failed!\n"); + if (espi_send_pltrst(cfg, false) == -1) { + printk(BIOS_ERR, "Error: PLTRST# deassertion failed!\n"); return -1; } -- To view, visit https://review.coreboot.org/c/coreboot/+/54884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20d12edf3efc6100096e24aa8d1aec76bbde264f Gerrit-Change-Number: 54884 Gerrit-PatchSet: 3 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> 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: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: tests/Makefile.inc: Move generated headers to corresponding build dir
by Patrick Georgi (Code Review) 30 May '21

30 May '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/54917 ) Change subject: tests/Makefile.inc: Move generated headers to corresponding build dir ...................................................................... tests/Makefile.inc: Move generated headers to corresponding build dir Test-local config override headers were generated to paths missing /tests/ infix, thus creating divergent tree in build output directory. This patch fixes it moving generated config headers to the test-local build directory. Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com> Change-Id: Ic5f3ba287ba3e9f5897cbaac64e88c2809f52d73 Reviewed-on: https://review.coreboot.org/c/coreboot/+/54917 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Paul Fagerburg <pfagerburg(a)chromium.org> --- M tests/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Paul Fagerburg: Looks good to me, approved diff --git a/tests/Makefile.inc b/tests/Makefile.inc index 6cf92fa..3473ef7 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -100,7 +100,7 @@ define TEST_CC_template # Generate custom config.h redefining given symbols -$(1)-config-file := $(obj)/$(1)/config.h +$(1)-config-file := $(testobj)/$(1)/config.h $$($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER) mkdir -p $$(dir $$@) printf '// File generated by tests/Makefile.inc\n// Do not change\n' > $$@ -- To view, visit https://review.coreboot.org/c/coreboot/+/54917 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic5f3ba287ba3e9f5897cbaac64e88c2809f52d73 Gerrit-Change-Number: 54917 Gerrit-PatchSet: 2 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: drivers/pc80/mc146818rtc: Check date and time for sanity
by Patrick Georgi (Code Review) 30 May '21

30 May '21

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/54914 ) Change subject: drivers/pc80/mc146818rtc: Check date and time for sanity ...................................................................... drivers/pc80/mc146818rtc: Check date and time for sanity There are cases where the RTC_VRT bit in register D stays set after a power failure while the real date and time registers can contain rubbish values (can happen when RTC is not buffered). If we do not detect this invalid date and/or time here and keep it, Linux will use these bad values for the initial timekeeper init. This in turn can lead to dates before 1970 in user land which can break a lot assumptions. To fix this, check date and time sanity when the RTC is initialized and reset the values if needed. Change-Id: I5bc600c78bab50c70372600347f63156df127012 Signed-off-by: Werner Zeh <werner.zeh(a)siemens.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/54914 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/pc80/rtc/mc146818rtc.c M src/lib/Makefile.inc 2 files changed, 14 insertions(+), 4 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/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c index 21d3c00..42671a9 100644 --- a/src/drivers/pc80/rtc/mc146818rtc.c +++ b/src/drivers/pc80/rtc/mc146818rtc.c @@ -175,6 +175,15 @@ ; } +/* Perform a sanity check of current date and time. */ +static int cmos_date_invalid(void) +{ + struct rtc_time now; + + rtc_get(&now); + return rtc_invalid(&now); +} + /* * If the CMOS is cleared, the rtc_reg has the invalid date. That * hurts some OSes. Even if we don't set USE_OPTION_TABLE, we need @@ -190,11 +199,11 @@ year = cmos_read(RTC_CLK_YEAR); /* - * TODO: If century is 0xFF, 100% that the CMOS is cleared. - * Other than that, so far rtc_year is the only entry to check - * if the date is valid. + * If century is 0xFF, 100% that the CMOS is cleared. + * In addition, check the sanity of all values and reset the date in case of + * insane values. */ - if (century > 0x99 || year > 0x99) /* Invalid date */ + if (century > 0x99 || year > 0x99 || cmos_date_invalid()) /* Invalid date */ cmos_reset_date(); } diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 358035d..baf9c4d 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -59,6 +59,7 @@ bootblock-y += memcmp.c bootblock-y += boot_device.c bootblock-y += fmap.c +bootblcok-y += rtc.c verstage-y += prog_loaders.c verstage-y += prog_ops.c -- To view, visit https://review.coreboot.org/c/coreboot/+/54914 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5bc600c78bab50c70372600347f63156df127012 Gerrit-Change-Number: 54914 Gerrit-PatchSet: 5 Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021