coreboot-gerrit December 2020

coreboot-gerrit@coreboot.org

1 participants
3168 discussions

Change in coreboot[master]: device + util/sconfig: introduce new device `gpio`
by Michael Niewöhner (Code Review) 14 Dec '20

14 Dec '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48582 ) Change subject: device + util/sconfig: introduce new device `gpio` ...................................................................... Patch Set 4: (3 comments) https://review.coreboot.org/c/coreboot/+/48582/4/src/device/device_const.c File src/device/device_const.c: https://review.coreboot.org/c/coreboot/+/48582/4/src/device/device_const.c@… PS4, Line 166: generic.id > `gpio. […] Done https://review.coreboot.org/c/coreboot/+/48582/4/src/device/device_util.c File src/device/device_util.c: https://review.coreboot.org/c/coreboot/+/48582/4/src/device/device_util.c@1… PS4, Line 134: generic.id > `gpio. […] Done https://review.coreboot.org/c/coreboot/+/48582/4/src/device/device_util.c@2… PS4, Line 230: %d.%d", dev->path.generic.id > I guess this should be just `%d", dev->path.gpio.id);`? yup, indeed > Also, do I miss something? is `-Wformat` not enabled? Hmm, nope, doesn't loke like it ever was -- To view, visit https://review.coreboot.org/c/coreboot/+/48582 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4572ad8b37bd1afd2fb213b2c67fb8aec536786 Gerrit-Change-Number: 48582 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 15 Dec 2020 00:21:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in em100[master]: Add 4bm switch to tracemode console
by Stefan Reinauer (Code Review) 14 Dec '20

14 Dec '20

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/48600 ) Change subject: Add 4bm switch to tracemode console ...................................................................... Add 4bm switch to tracemode console Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Change-Id: I4bd71b1bdb71c03eb1c55e0ebea9b5df664dcf9b --- M trace.c 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/em100 refs/changes/00/48600/1 diff --git a/trace.c b/trace.c index 7fe6ab8..3ff7904 100644 --- a/trace.c +++ b/trace.c @@ -353,6 +353,16 @@ /* new command */ cmdid = cmd; + /* Special commands */ + switch (spi_command) { + case 0xb7: + address_mode = 4; + break; + case 0xe9: + address_mode = 3; + break; + } + /* set up address if used by this command */ if (!spi_cmd_vals->uses_address) { j = 1; /* skip command byte */ -- To view, visit https://review.coreboot.org/c/em100/+/48600 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: em100 Gerrit-Branch: master Gerrit-Change-Id: I4bd71b1bdb71c03eb1c55e0ebea9b5df664dcf9b Gerrit-Change-Number: 48600 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: soc/intel: hook up new gpio device in the soc chips
by Furquan Shaikh (Code Review) 14 Dec '20

14 Dec '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 ) Change subject: soc/intel: hook up new gpio device in the soc chips ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… File src/soc/intel/alderlake/chip.c: https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO) : dev->ops = &soc_gpio_ops; > I agree on what Nico says. It's not a chip and we shouldn't fake it. I will try to move the struct. Having a separate chip is helpful especially when you want to add some configs which are specific to that chip. You don't need to add all these configs to soc chip. I don't think there is a problem in providing a chip for the GPIO block. It also relieves each SoC from duplicating the same code to set up GPIO device ops. -- To view, visit https://review.coreboot.org/c/coreboot/+/48583 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81dbbf5397b28ffa7537465c53332779245b39f6 Gerrit-Change-Number: 48583 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 15 Dec 2020 00:00:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: hook up new gpio device in the soc chips
by Michael Niewöhner (Code Review) 14 Dec '20

14 Dec '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 ) Change subject: soc/intel: hook up new gpio device in the soc chips ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… File src/soc/intel/alderlake/chip.c: https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO) : dev->ops = &soc_gpio_ops; > > With your code the block gpio driver will get assigned to both gpio devices which is obviously wrong. > > You will have to decorate the gpio device with chip soc/intel/common/block/gpio. Then `intel_gpio_enable()` gets called only for that specific device. Similarly for the other chip_gpio, it's own enable_dev will be called. Thus, there should not be any problem with the above proposal I made. I agree on what Nico says. It's not a chip and we shouldn't fake it. I will try to move the struct. > The `struct gpio_operations` could definitely move to `common/block/`, > that should be easy. Without adding another "chip", you could either make > it `extern` and hook it up here, or call such a intel_gpio_enable() from > here (and each soc). Easy? :'D You have no idea how many different no-working variants I tested to get this moved to block :'D :P ... calling `intel_gpio_enabe` sounds like a good idea, though. -- To view, visit https://review.coreboot.org/c/coreboot/+/48583 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81dbbf5397b28ffa7537465c53332779245b39f6 Gerrit-Change-Number: 48583 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 14 Dec 2020 23:56:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in em100[master]: trace/console: factor out error handling
by Stefan Reinauer (Code Review) 14 Dec '20

14 Dec '20

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/48598 ) Change subject: trace/console: factor out error handling ...................................................................... trace/console: factor out error handling Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Change-Id: I313dc6d3433921af38a7deea810dc51a45572ea6 --- M em100.c 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/em100 refs/changes/98/48598/1 diff --git a/em100.c b/em100.c index ca6e683..4b0a121 100644 --- a/em100.c +++ b/em100.c @@ -1233,14 +1233,14 @@ printf(". Press CTL-C to exit.\n\n"); while (!exit_requested && usb_errors < MAX_USB_ERRORS) { + int ret; if (trace) { - if (!read_spi_trace(em100, terminal, - address_offset)) - usb_errors++; + ret = read_spi_trace(em100, terminal, address_offset); } else { - if (!read_spi_terminal(em100, 0)) - usb_errors++; + ret = read_spi_terminal(em100, 0); } + if (ret == 0) + usb_errors++; } if (usb_errors >= MAX_USB_ERRORS) printf("Error: Bailed out with too many USB errors.\n"); -- To view, visit https://review.coreboot.org/c/em100/+/48598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: em100 Gerrit-Branch: master Gerrit-Change-Id: I313dc6d3433921af38a7deea810dc51a45572ea6 Gerrit-Change-Number: 48598 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newchange

2 2

Change in em100[master]: Fix comment
by Stefan Reinauer (Code Review) 14 Dec '20

14 Dec '20

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/48588 ) Change subject: Fix comment ...................................................................... Fix comment Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Change-Id: I99d18e12afb0bdd991423305807b3b7a78494605 --- M trace.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/em100 refs/changes/88/48588/1 diff --git a/trace.c b/trace.c index 933575e..22517f9 100644 --- a/trace.c +++ b/trace.c @@ -241,7 +241,7 @@ break; } - /* set up address if used by this command*/ + /* set up address if used by this command */ if (!spi_cmd_vals->uses_address) { j = 1; /* skip command byte */ } else { -- To view, visit https://review.coreboot.org/c/em100/+/48588 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: em100 Gerrit-Branch: master Gerrit-Change-Id: I99d18e12afb0bdd991423305807b3b7a78494605 Gerrit-Change-Number: 48588 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newchange

2 3

Change in coreboot[master]: src/soc/intel: Add support for CAR_HAS_SF_MASKS and select for TGL
by Shreesh Chhabbi (Code Review) 14 Dec '20

14 Dec '20

Hello Shreesh Chhabbi, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48286 to review the following change. Change subject: src/soc/intel: Add support for CAR_HAS_SF_MASKS and select for TGL ...................................................................... src/soc/intel: Add support for CAR_HAS_SF_MASKS and select for TGL Program IA32_CR_SF_QOS_MASK_x MSRs under CAR_HAS_SF_MASKS config option. Select CAR_HAS_SF_MASKS for Tigerlake. Bug=b:171601324 BRANCH=volteer Test=[to be updated] Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Change-Id: Iabf7f387fb5887aca10158788599452c3f2df7e8 Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> --- M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/car/cache_as_ram.S M src/soc/intel/tigerlake/Kconfig 3 files changed, 38 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/48286/1 diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig index 912760e..2b630d0 100644 --- a/src/soc/intel/common/block/cpu/Kconfig +++ b/src/soc/intel/common/block/cpu/Kconfig @@ -51,6 +51,14 @@ ENHANCED NEM guarantees that modified data is always kept in cache while clean data is replaced. +config CAR_HAS_SF_MASKS + bool + depends on INTEL_CAR_NEM_ENHANCED + help + In the case of non-inclusive cache architecture Snoop Filter MSR + IA32_L3_SF_MASK_x programming is required along with the data ways. + This is applicable for TGL and beyond. + config COS_MAPPED_TO_MSB bool depends on INTEL_CAR_NEM_ENHANCED diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index 7408822..93f5f95 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -414,6 +414,35 @@ set_eviction_mask: mov %ebx, %ecx /* back up the number of ways */ mov %eax, %ebx /* back up the non-eviction mask*/ + mov %ecx, %edi /* back up the number of ways */ + +#if CONFIG(CAR_HAS_SF_MASKS) + /* + * If total number of available ways is X, SF mask is ((0x1 << X*2) - 1) + */ + mov $0x02, %eax + mul %ecx + mov %eax, %ecx + mov $0x01, %eax + shl %cl, %eax + subl $0x01, %eax /* Holds the SF mask */ + /* + * Program MSR 0x1891 IA32_CR_SF_QOS_MASK_1 with + * total number of LLC ways + */ + movl $IA32_CR_SF_QOS_MASK_1, %ecx + xorl %edx, %edx + wrmsr + /* + * Program MSR 0x1892 IA32_CR_SF_QOS_MASK_2 with + * total number of LLC ways + */ + movl $IA32_CR_SF_QOS_MASK_2, %ecx + xorl %edx, %edx + wrmsr + mov %edi, %ecx /* Restore number of ways */ +#endif + /* * Program MSR 0xC91 IA32_L3_MASK_1 * This MSR contain one bit per each way of LLC diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index f53c18a..dd14ad2 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -24,6 +24,7 @@ select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE select INTEL_CAR_NEM_ENHANCED if !INTEL_CAR_NEM + select CAR_HAS_SF_MASKS if INTEL_CAR_NEM_ENHANCED select COS_MAPPED_TO_MSB if INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP -- To view, visit https://review.coreboot.org/c/coreboot/+/48286 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iabf7f387fb5887aca10158788599452c3f2df7e8 Gerrit-Change-Number: 48286 Gerrit-PatchSet: 1 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-MessageType: newchange

5 80

Change in coreboot[master]: soc/intel: Remove INTEL_CAR_NEM_ENHANCED_V2 config option
by Shreesh Chhabbi (Code Review) 14 Dec '20

14 Dec '20

Hello Shreesh Chhabbi, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/48284 to review the following change. Change subject: soc/intel: Remove INTEL_CAR_NEM_ENHANCED_V2 config option ...................................................................... soc/intel: Remove INTEL_CAR_NEM_ENHANCED_V2 config option SF Mask MSRs' Programming which was done under this config selection will be moved under a new config option called CAR_HAS_SF_MASKS. This segregates the eNEM programming sequence based on sub features supported in each processor. Change-Id: If4d8d1ec52b7b79965fe1a957c48f571ec56dc63 Signed-off-by: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/cpu/Kconfig M src/soc/intel/common/block/cpu/car/cache_as_ram.S M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/jasperlake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/intel/tigerlake/Kconfig 8 files changed, 8 insertions(+), 31 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/48284/1 diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index f427340..06f8c2e 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -315,7 +315,7 @@ config USE_CANNONLAKE_CAR_NEM_ENHANCED bool "Enhanced Non-evict mode" select SOC_INTEL_COMMON_BLOCK_CAR - select USE_CAR_NEM_ENHANCED_V1 + select INTEL_CAR_NEM_ENHANCED help A current limitation of NEM (Non-Evict mode) is that code and data sizes are derived from the requirement to not write out any modified diff --git a/src/soc/intel/common/block/cpu/Kconfig b/src/soc/intel/common/block/cpu/Kconfig index 9023b58..912760e 100644 --- a/src/soc/intel/common/block/cpu/Kconfig +++ b/src/soc/intel/common/block/cpu/Kconfig @@ -51,21 +51,6 @@ ENHANCED NEM guarantees that modified data is always kept in cache while clean data is replaced. -config USE_CAR_NEM_ENHANCED_V1 - bool - select INTEL_CAR_NEM_ENHANCED - help - This config supports INTEL_CAR_NEM_ENHANCED mode on - SKL, KBL, CNL, WHL, CML and ICL and JSL platforms. - -config USE_CAR_NEM_ENHANCED_V2 - bool - select INTEL_CAR_NEM_ENHANCED - select COS_MAPPED_TO_MSB - help - This config supports INTEL_CAR_NEM_ENHANCED mode on - TGL platform. - config COS_MAPPED_TO_MSB bool depends on INTEL_CAR_NEM_ENHANCED diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram.S b/src/soc/intel/common/block/cpu/car/cache_as_ram.S index 167342f..7408822 100644 --- a/src/soc/intel/common/block/cpu/car/cache_as_ram.S +++ b/src/soc/intel/common/block/cpu/car/cache_as_ram.S @@ -415,7 +415,7 @@ mov %ebx, %ecx /* back up the number of ways */ mov %eax, %ebx /* back up the non-eviction mask*/ /* - * Set MSR 0xC91 IA32_L3_MASK_1 or MSR 0x1891 IA32_CR_SF_QOS_MASK_1 + * Program MSR 0xC91 IA32_L3_MASK_1 * This MSR contain one bit per each way of LLC * - If this bit is '0' - the way is protected from eviction * - If this bit is '1' - the way is not protected from eviction @@ -428,26 +428,18 @@ xor $~0, %eax /* invert 32 bits */ and %ecx, %eax -#if CONFIG(USE_CAR_NEM_ENHANCED_V1) movl $IA32_L3_MASK_1, %ecx -#elif CONFIG(USE_CAR_NEM_ENHANCED_V2) - movl $IA32_CR_SF_QOS_MASK_1, %ecx -#endif xorl %edx, %edx wrmsr /* - * Set MSR 0xC92 IA32_L3_MASK_1 or MSR 0x1892 IA32_CR_SF_QOS_MASK_2 + * Program MSR 0xC92 IA32_L3_MASK_2 * This MSR contain one bit per each way of LLC * - If this bit is '0' - the way is protected from eviction * - If this bit is '1' - the way is not protected from eviction */ mov %ebx, %eax -#if CONFIG(USE_CAR_NEM_ENHANCED_V1) movl $IA32_L3_MASK_2, %ecx -#elif CONFIG(USE_CAR_NEM_ENHANCED_V2) - movl $IA32_CR_SF_QOS_MASK_2, %ecx -#endif xorl %edx, %edx wrmsr /* diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig index 4981205..4fe491f 100644 --- a/src/soc/intel/denverton_ns/Kconfig +++ b/src/soc/intel/denverton_ns/Kconfig @@ -170,7 +170,7 @@ config USE_DENVERTON_NS_CAR_NEM_ENHANCED bool "Enhanced Non-evict mode" select SOC_INTEL_COMMON_BLOCK_CAR - select USE_CAR_NEM_ENHANCED_V1 + select INTEL_CAR_NEM_ENHANCED help A current limitation of NEM (Non-Evict mode) is that code and data sizes are derived from the requirement to not write out any modified cache line. diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 52e9a74..fee52e6 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -23,6 +23,7 @@ select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE + select INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP select IOAPIC @@ -64,7 +65,6 @@ select DISPLAY_FSP_VERSION_INFO select HECI_DISABLE_USING_SMM select USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI - select USE_CAR_NEM_ENHANCED_V1 config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/jasperlake/Kconfig b/src/soc/intel/jasperlake/Kconfig index 294f19f8..0a15f05 100644 --- a/src/soc/intel/jasperlake/Kconfig +++ b/src/soc/intel/jasperlake/Kconfig @@ -23,6 +23,7 @@ select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE + select INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP select IOAPIC @@ -61,7 +62,6 @@ select TSC_MONOTONIC_TIMER select UDELAY_TSC select UDK_202005_BINDING - select USE_CAR_NEM_ENHANCED_V1 select DISPLAY_FSP_VERSION_INFO select HECI_DISABLE_USING_SMM diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index ba3af84..26ada09 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -30,6 +30,7 @@ select GENERIC_GPIO_LIB select HAVE_FSP_GOP select HAVE_FSP_LOGO_SUPPORT + select INTEL_CAR_NEM_ENHANCED select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select INTEL_GMA_ACPI @@ -77,7 +78,6 @@ select TSC_SYNC_MFENCE select UDELAY_TSC select UDK_2015_BINDING - select USE_CAR_NEM_ENHANCED_V1 config FSP_HYPERTHREADING bool "Enable Hyper-Threading" diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index b97b92e..f53c18a 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -23,7 +23,7 @@ select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE - select USE_CAR_NEM_ENHANCED_V1 if !INTEL_CAR_NEM + select INTEL_CAR_NEM_ENHANCED if !INTEL_CAR_NEM select COS_MAPPED_TO_MSB if INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP -- To view, visit https://review.coreboot.org/c/coreboot/+/48284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If4d8d1ec52b7b79965fe1a957c48f571ec56dc63 Gerrit-Change-Number: 48284 Gerrit-PatchSet: 1 Gerrit-Owner: Shreesh Chhabbi <shreesh.chhabbi(a)intel.com> Gerrit-Reviewer: Shreesh Chhabbi <shreesh.chhabbi(a)intel.corp-partner.google.com> Gerrit-MessageType: newchange

4 11

Change in coreboot[master]: soc/intel: hook up new gpio device in the soc chips
by Furquan Shaikh (Code Review) 14 Dec '20

14 Dec '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 ) Change subject: soc/intel: hook up new gpio device in the soc chips ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… File src/soc/intel/alderlake/chip.c: https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO) : dev->ops = &soc_gpio_ops; > With your code the block gpio driver will get assigned to both gpio devices which is obviously wrong. You will have to decorate the gpio device with chip soc/intel/common/block/gpio. Then `intel_gpio_enable()` gets called only for that specific device. Similarly for the other chip_gpio, it's own enable_dev will be called. Thus, there should not be any problem with the above proposal I made. -- To view, visit https://review.coreboot.org/c/coreboot/+/48583 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81dbbf5397b28ffa7537465c53332779245b39f6 Gerrit-Change-Number: 48583 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 14 Dec 2020 22:05:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel: hook up new gpio device in the soc chips
by Nico Huber (Code Review) 14 Dec '20

14 Dec '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 ) Change subject: soc/intel: hook up new gpio device in the soc chips ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… File src/soc/intel/alderlake/chip.c: https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/ch… PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO) : dev->ops = &soc_gpio_ops; > With your code the block gpio driver will get assigned to both gpio devices which is obviously wrong. Only if intel_gpio_enable() gets called (it shouldn't be). But as I mentioned elsewhere, I'm not a fan of fake chips. The `struct gpio_operations` could definitely move to `common/block/`, that should be easy. Without adding another "chip", you could either make it `extern` and hook it up here, or call such a intel_gpio_enable() from here (and each soc). -- To view, visit https://review.coreboot.org/c/coreboot/+/48583 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I81dbbf5397b28ffa7537465c53332779245b39f6 Gerrit-Change-Number: 48583 Gerrit-PatchSet: 7 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 14 Dec 2020 22:02:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2020