Attention is currently required from: Yidi Lin.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78888?usp=email )
Change subject: libpayload/libc/time: Fix possible overflow in multiplication
......................................................................
Patch Set 11:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/78888/comment/2b6cbc42_ffbce9d7 :
PS11, Line 15: counter should never be that fast.
> Sorry, forgot to mention, Paul ran into the assertion.
Oh okay, yeah, then we need to do something. Which platform did he see it on?
The quickest solution would probably to just switch it back to 64-bit and implement a `gcd64()`... I think we only made it 32 for efficiency because we thought it wouldn't make a difference.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78888?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia55532490651fcf47128b83a8554751f050bcc89
Gerrit-Change-Number: 78888
Gerrit-PatchSet: 11
Gerrit-Owner: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.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: Yidi Lin <yidilin(a)google.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 20:55:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Benjamin Doron, David Milosevic, Maximilian Brune.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80184?usp=email )
Change subject: arch/arm64/armv8: Add exception output without printk
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
File src/arch/arm64/armv8/exception.c:
https://review.coreboot.org/c/coreboot/+/80184/comment/ef9a24c8_0136381a :
PS1, Line 113: if (idx < 10)
: __uart_tx_byte('0' + (char)idx);
: else if (idx < 16)
: __uart_tx_byte('A' - 10 + (char)idx);
: __uart_tx_byte('!');
: UART_DIRECT_PRINT(raw_read_esr(), 32)
: __uart_tx_byte('!');
> Sounds good. But I will have to write it like this (even if it is taking a bit more space): […]
Hmm... yeah okay, there seems to be no way to force GCC to use PC-relative addressing for that string. That's a bit frustrating. Guess your solution is the best we can do.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80184?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2f858730469fff3cae120fd7c32fec53b3d309ca
Gerrit-Change-Number: 80184
Gerrit-PatchSet: 4
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: David Milosevic <David.Milosevic(a)9elements.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: David Milosevic <David.Milosevic(a)9elements.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 20:27:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80330?usp=email )
Change subject: arch/x86/ioapic: always write IOAPIC ID in set_ioapic_id
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80330/comment/6b950b5c_b56c2523 :
PS3, Line 18: The only
: southbridge that doesn't call register_new_ioapic_gsi0
Which one is it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80330?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic8538f82a6b10f16eeb228669db197dc8e326ffd
Gerrit-Change-Number: 80330
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
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: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 06 Feb 2024 20:27:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80261?usp=email )
Change subject: mb/up/squared: Make mini PCIe port mode configurable
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80261/comment/e9f7d412_61d2ad97 :
PS2, Line 9: Add ENABLE_MSATA config knob and pad configuration to put Mini PCIe port
: into mSATA mode.
> Did you test if the PCIe functionality still works? I'm wondering because you are not disabling the […]
Good point. My usecase always was mSATA mode, I never explicitly tested whether PCIe mode works (mainly because I have no suitable hardware, it's not very common).
The official documentation (https://doc.coreboot.org/mainboard/up/squared/index.html) lists mini PCIe as untested.
What do you suggest?
Patchset:
PS2:
Thanks for your review.
File src/mainboard/up/squared/Kconfig:
https://review.coreboot.org/c/coreboot/+/80261/comment/654ab227_f01e7a8f :
PS2, Line 77: config ENABLE_MSATA
: bool "Use mini-PCIe port for mSATA"
: default n
> I think a choice menu would be better since the distinction between both functions (SATA vs PCIe) is […]
I used `mb/compulab/intense_pc` as a template, which provides the same `ENABLE_MSATA` knob. Will update the patch if you still prefer the choice menu variant.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80261?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic2da1dd4252ebb5e373bc65418e321f566d4c10f
Gerrit-Change-Number: 80261
Gerrit-PatchSet: 2
Gerrit-Owner: Reto Buerki <reet(a)codelabs.ch>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Comment-Date: Tue, 06 Feb 2024 20:08:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-MessageType: comment
Attention is currently required from: Jérémy Compostella.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80359?usp=email )
Change subject: [WIP] arch/x86/include/mpspec: add comment to mpc_config_ioapic
......................................................................
[WIP] arch/x86/include/mpspec: add comment to mpc_config_ioapic
The size of void * depends on whether coreboot is built for 32 or 64 bit
mode, so this smells like a possible bug to me.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I8ecd924b53cc8ea9505e13808ca53b04c3334965
---
M src/arch/x86/include/arch/smp/mpspec.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/80359/1
diff --git a/src/arch/x86/include/arch/smp/mpspec.h b/src/arch/x86/include/arch/smp/mpspec.h
index 80ae7e8..31213f2 100644
--- a/src/arch/x86/include/arch/smp/mpspec.h
+++ b/src/arch/x86/include/arch/smp/mpspec.h
@@ -102,7 +102,7 @@
u8 mpc_apicver;
u8 mpc_flags;
#define MPC_APIC_USABLE 0x01
- void *mpc_apicaddr;
+ void *mpc_apicaddr;//32 vs 64 bit?!
} __packed;
struct mpc_config_intsrc {
--
To view, visit https://review.coreboot.org/c/coreboot/+/80359?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8ecd924b53cc8ea9505e13808ca53b04c3334965
Gerrit-Change-Number: 80359
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-MessageType: newchange
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80356?usp=email )
Change subject: soc/amd/genoa_poc/chip: print data fabric MMIO decoding configuration
......................................................................
soc/amd/genoa_poc/chip: print data fabric MMIO decoding configuration
Printing the data fabric MMIO decode window configuration might be
useful and it also aligns this SoC more with the other AMD family 17h+
SoCs.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I52f6655a5c63e31165549dcb6f5f95d4e74bad3d
---
M src/soc/amd/genoa_poc/chip.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/80356/1
diff --git a/src/soc/amd/genoa_poc/chip.c b/src/soc/amd/genoa_poc/chip.c
index 13ede84..a763dab 100644
--- a/src/soc/amd/genoa_poc/chip.c
+++ b/src/soc/amd/genoa_poc/chip.c
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#include <amdblocks/acpi.h>
+#include <amdblocks/data_fabric.h>
#include <device/device.h>
#include <soc/southbridge.h>
#include <soc/southbridge.h>
@@ -13,6 +14,8 @@
setup_opensil();
opensil_xSIM_timepoint_1();
+ data_fabric_print_mmio_conf();
+
fch_init(chip_info);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80356?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I52f6655a5c63e31165549dcb6f5f95d4e74bad3d
Gerrit-Change-Number: 80356
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80355?usp=email )
Change subject: soc/amd: drop unneeded data_fabric_set_mmio_np
......................................................................
soc/amd: drop unneeded data_fabric_set_mmio_np
Drop the unneeded data_fabric_set_mmio_np function and the corresponding
SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION Kconfig symbol. In systems
with only one FCH, its MMIO region will be subtractively decoded and
there's no need to add a non-posted data fabric MMIO region after the
FSP/openSIL has already configured the data fabric decode windows. In
systems with more than one FCH, openSIL will already take care of
initializing everything for the additional FCH, so we also won't need to
do anything in that case. Since dropping this function also removes both
data_fabric_print_mmio_conf calls before and after adding the unneeded
non-posted MMIO region, replace the data_fabric_set_mmio_np call with a
data_fabric_print_mmio_conf call to still print the data fabric MMIO
decode regions set up by the FSP/openSIL.
TEST=Mandolin still boots successfully
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I474b6e066060abb3fe5b78505521c7782cc192ee
---
M src/soc/amd/cezanne/Kconfig
M src/soc/amd/cezanne/chip.c
M src/soc/amd/common/block/data_fabric/Kconfig
M src/soc/amd/common/block/data_fabric/Makefile.mk
D src/soc/amd/common/block/data_fabric/np_region.c
M src/soc/amd/common/block/include/amdblocks/data_fabric.h
M src/soc/amd/glinda/Kconfig
M src/soc/amd/glinda/chip.c
M src/soc/amd/mendocino/Kconfig
M src/soc/amd/mendocino/chip.c
M src/soc/amd/phoenix/Kconfig
M src/soc/amd/phoenix/chip.c
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/chip.c
14 files changed, 6 insertions(+), 139 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/80355/1
diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig
index 0ef658c..4e22005 100644
--- a/src/soc/amd/cezanne/Kconfig
+++ b/src/soc/amd/cezanne/Kconfig
@@ -43,7 +43,6 @@
select SOC_AMD_COMMON_BLOCK_CPUFREQ_FAM17H_19H
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
- select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
select SOC_AMD_COMMON_BLOCK_EMMC
select SOC_AMD_COMMON_BLOCK_GRAPHICS
select SOC_AMD_COMMON_BLOCK_HAS_ESPI
diff --git a/src/soc/amd/cezanne/chip.c b/src/soc/amd/cezanne/chip.c
index 7d315cb..83fd5c6 100644
--- a/src/soc/amd/cezanne/chip.c
+++ b/src/soc/amd/cezanne/chip.c
@@ -41,7 +41,7 @@
amd_fsp_silicon_init();
- data_fabric_set_mmio_np();
+ data_fabric_print_mmio_conf();
fch_init(chip_info);
}
diff --git a/src/soc/amd/common/block/data_fabric/Kconfig b/src/soc/amd/common/block/data_fabric/Kconfig
index da35dae..add2374 100644
--- a/src/soc/amd/common/block/data_fabric/Kconfig
+++ b/src/soc/amd/common/block/data_fabric/Kconfig
@@ -4,14 +4,6 @@
Select this option to add data fabric configuration related
functionality to the build.
-config SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
- bool
- depends on SOC_AMD_COMMON_BLOCK_DATA_FABRIC
- help
- Select this option to include the code to make sure that there's a
- non-posted MMIO region configured in the data fabric registers that
- covers the FCH MMIO from the HPET up to right below the LAPIC.
-
config SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
bool
depends on SOC_AMD_COMMON_BLOCK_DATA_FABRIC
diff --git a/src/soc/amd/common/block/data_fabric/Makefile.mk b/src/soc/amd/common/block/data_fabric/Makefile.mk
index b0684f4..55a4d3f 100644
--- a/src/soc/amd/common/block/data_fabric/Makefile.mk
+++ b/src/soc/amd/common/block/data_fabric/Makefile.mk
@@ -2,7 +2,6 @@
ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_DATA_FABRIC) += data_fabric_helper.c
ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN) += domain.c
-ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION) += np_region.c
ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_DATA_FABRIC_MULTI_PCI_SEGMENT),y)
ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN) += pci_segment_multi.c
diff --git a/src/soc/amd/common/block/data_fabric/np_region.c b/src/soc/amd/common/block/data_fabric/np_region.c
deleted file mode 100644
index dd82e5e..0000000
--- a/src/soc/amd/common/block/data_fabric/np_region.c
+++ /dev/null
@@ -1,119 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#include <amdblocks/data_fabric.h>
-#include <arch/hpet.h>
-#include <console/console.h>
-#include <cpu/x86/lapic_def.h>
-#include <soc/data_fabric.h>
-#include <types.h>
-
-static void data_fabric_disable_mmio_reg(unsigned int reg)
-{
- union df_mmio_control ctrl = { .dst_fabric_id = IOMS0_FABRIC_ID };
- data_fabric_broadcast_write32(DF_MMIO_CONTROL(reg), ctrl.raw);
- data_fabric_broadcast_write32(DF_MMIO_BASE(reg), 0);
- data_fabric_broadcast_write32(DF_MMIO_LIMIT(reg), 0);
-}
-
-static bool is_mmio_reg_disabled(unsigned int reg)
-{
- union df_mmio_control ctrl;
- ctrl.raw = data_fabric_broadcast_read32(DF_MMIO_CONTROL(reg));
- return !(ctrl.we || ctrl.re);
-}
-
-static int data_fabric_find_unused_mmio_reg(void)
-{
- for (unsigned int i = 0; i < DF_MMIO_REG_SET_COUNT; i++) {
- if (is_mmio_reg_disabled(i))
- return i;
- }
- return -1;
-}
-
-void data_fabric_set_mmio_np(void)
-{
- /*
- * Mark region from HPET-LAPIC or 0xfed00000-0xfee00000-1 as NP.
- *
- * AGESA has already programmed the NB MMIO routing, however nothing
- * is yet marked as non-posted.
- *
- * If there exists an overlapping routing base/limit pair, trim its
- * base or limit to avoid the new NP region. If any pair exists
- * completely within HPET-LAPIC range, remove it. If any pair surrounds
- * HPET-LAPIC, it must be split into two regions.
- *
- * TODO(b/156296146): Remove the settings from AGESA and allow coreboot
- * to own everything. If not practical, consider erasing all settings
- * and have coreboot reprogram them. At that time, make the source
- * below more flexible.
- * * Note that the code relies on the granularity of the HPET and
- * LAPIC addresses being sufficiently large that the shifted limits
- * +/-1 are always equivalent to the non-shifted values +/-1.
- */
-
- unsigned int i;
- int reg;
- uint32_t base, limit;
- union df_mmio_control ctrl;
- const uint32_t np_bot = HPET_BASE_ADDRESS >> DF_MMIO_SHIFT;
- const uint32_t np_top = (LAPIC_DEFAULT_BASE - 1) >> DF_MMIO_SHIFT;
-
- data_fabric_print_mmio_conf();
-
- for (i = 0; i < DF_MMIO_REG_SET_COUNT; i++) {
- /* Adjust all registers that overlap */
- ctrl.raw = data_fabric_broadcast_read32(DF_MMIO_CONTROL(i));
- if (!(ctrl.we || ctrl.re))
- continue; /* not enabled */
-
- base = data_fabric_broadcast_read32(DF_MMIO_BASE(i));
- limit = data_fabric_broadcast_read32(DF_MMIO_LIMIT(i));
-
- if (base > np_top || limit < np_bot)
- continue; /* no overlap at all */
-
- if (base >= np_bot && limit <= np_top) {
- data_fabric_disable_mmio_reg(i); /* 100% within, so remove */
- continue;
- }
-
- if (base < np_bot && limit > np_top) {
- /* Split the configured region */
- data_fabric_broadcast_write32(DF_MMIO_LIMIT(i), np_bot - 1);
- reg = data_fabric_find_unused_mmio_reg();
- if (reg < 0) {
- /* Although a pair could be freed later, this condition is
- * very unusual and deserves analysis. Flag an error and
- * leave the topmost part unconfigured. */
- printk(BIOS_ERR, "Not enough NB MMIO routing registers\n");
- continue;
- }
- data_fabric_broadcast_write32(DF_MMIO_BASE(reg), np_top + 1);
- data_fabric_broadcast_write32(DF_MMIO_LIMIT(reg), limit);
- data_fabric_broadcast_write32(DF_MMIO_CONTROL(reg), ctrl.raw);
- continue;
- }
-
- /* If still here, adjust only the base or limit */
- if (base <= np_bot)
- data_fabric_broadcast_write32(DF_MMIO_LIMIT(i), np_bot - 1);
- else
- data_fabric_broadcast_write32(DF_MMIO_BASE(i), np_top + 1);
- }
-
- reg = data_fabric_find_unused_mmio_reg();
- if (reg < 0) {
- printk(BIOS_ERR, "cannot configure region as NP\n");
- return;
- }
-
- union df_mmio_control np_ctrl = { .dst_fabric_id = IOMS0_FABRIC_ID,
- .np = 1, .we = 1, .re = 1 };
- data_fabric_broadcast_write32(DF_MMIO_BASE(reg), np_bot);
- data_fabric_broadcast_write32(DF_MMIO_LIMIT(reg), np_top);
- data_fabric_broadcast_write32(DF_MMIO_CONTROL(reg), np_ctrl.raw);
-
- data_fabric_print_mmio_conf();
-}
diff --git a/src/soc/amd/common/block/include/amdblocks/data_fabric.h b/src/soc/amd/common/block/include/amdblocks/data_fabric.h
index 9dbf5da..f0073df 100644
--- a/src/soc/amd/common/block/include/amdblocks/data_fabric.h
+++ b/src/soc/amd/common/block/include/amdblocks/data_fabric.h
@@ -44,7 +44,6 @@
}
void data_fabric_print_mmio_conf(void);
-void data_fabric_set_mmio_np(void);
enum cb_err data_fabric_get_pci_bus_numbers(struct device *domain, uint8_t *segment_group,
uint8_t *first_bus, uint8_t *last_bus);
diff --git a/src/soc/amd/glinda/Kconfig b/src/soc/amd/glinda/Kconfig
index d3194a2..a042ea2 100644
--- a/src/soc/amd/glinda/Kconfig
+++ b/src/soc/amd/glinda/Kconfig
@@ -45,7 +45,6 @@
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_MULTI_PCI_SEGMENT
- select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
select SOC_AMD_COMMON_BLOCK_ESPI_EXTENDED_DECODE_RANGES # TODO: Check if this is still correct
select SOC_AMD_COMMON_BLOCK_GRAPHICS # TODO: Check if this is still correct
select SOC_AMD_COMMON_BLOCK_HAS_ESPI # TODO: Check if this is still correct
diff --git a/src/soc/amd/glinda/chip.c b/src/soc/amd/glinda/chip.c
index c8b4d08..515580d 100644
--- a/src/soc/amd/glinda/chip.c
+++ b/src/soc/amd/glinda/chip.c
@@ -43,7 +43,7 @@
amd_fsp_silicon_init();
- data_fabric_set_mmio_np();
+ data_fabric_print_mmio_conf();
fch_init(chip_info);
}
diff --git a/src/soc/amd/mendocino/Kconfig b/src/soc/amd/mendocino/Kconfig
index 194b775..7f61338 100644
--- a/src/soc/amd/mendocino/Kconfig
+++ b/src/soc/amd/mendocino/Kconfig
@@ -47,7 +47,6 @@
select SOC_AMD_COMMON_BLOCK_CPU_SYNC_PSP_ADDR_MSR
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
- select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
select SOC_AMD_COMMON_BLOCK_ESPI_EXTENDED_DECODE_RANGES
select SOC_AMD_COMMON_BLOCK_GRAPHICS
select SOC_AMD_COMMON_BLOCK_HAS_ESPI
diff --git a/src/soc/amd/mendocino/chip.c b/src/soc/amd/mendocino/chip.c
index 99f57aa..729f97a 100644
--- a/src/soc/amd/mendocino/chip.c
+++ b/src/soc/amd/mendocino/chip.c
@@ -41,7 +41,7 @@
amd_fsp_silicon_init();
- data_fabric_set_mmio_np();
+ data_fabric_print_mmio_conf();
fch_init(chip_info);
}
diff --git a/src/soc/amd/phoenix/Kconfig b/src/soc/amd/phoenix/Kconfig
index f37dd20..0acf62b 100644
--- a/src/soc/amd/phoenix/Kconfig
+++ b/src/soc/amd/phoenix/Kconfig
@@ -42,7 +42,6 @@
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_MULTI_PCI_SEGMENT
- select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
select SOC_AMD_COMMON_BLOCK_ESPI_EXTENDED_DECODE_RANGES
select SOC_AMD_COMMON_BLOCK_GRAPHICS # TODO: Check if this is still correct
select SOC_AMD_COMMON_BLOCK_HAS_ESPI
diff --git a/src/soc/amd/phoenix/chip.c b/src/soc/amd/phoenix/chip.c
index b08db2b..f45e7b8 100644
--- a/src/soc/amd/phoenix/chip.c
+++ b/src/soc/amd/phoenix/chip.c
@@ -49,7 +49,7 @@
opensil_xSIM_timepoint_1();
}
- data_fabric_set_mmio_np();
+ data_fabric_print_mmio_conf();
fch_init(chip_info);
}
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 42fe66f..3768cfb 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -37,7 +37,6 @@
select SOC_AMD_COMMON_BLOCK_CPUFREQ_FAM17H_19H
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC
select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_DOMAIN
- select SOC_AMD_COMMON_BLOCK_DATA_FABRIC_NP_REGION
select SOC_AMD_COMMON_BLOCK_EMMC
select SOC_AMD_COMMON_BLOCK_EMMC_SKIP_POWEROFF
select SOC_AMD_COMMON_BLOCK_GRAPHICS
diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c
index d2070c1..7411948 100644
--- a/src/soc/amd/picasso/chip.c
+++ b/src/soc/amd/picasso/chip.c
@@ -42,7 +42,8 @@
amd_fsp_silicon_init();
- data_fabric_set_mmio_np();
+ data_fabric_print_mmio_conf();
+
fch_init(chip_info);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/80355?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I474b6e066060abb3fe5b78505521c7782cc192ee
Gerrit-Change-Number: 80355
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Eric Lai, Ivy Jian, Nick Vaccaro, Shelley Chen.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80300?usp=email )
Change subject: mb/google/brox: Initialize TCHSCR_RST_L/EN_PP3300_TCHSCR to 0
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80300/comment/20e140fc_e6be0fab :
PS2, Line 176: register "generic.detect" = "1"
> Does this mean linux kernel has to probe for the touchscreen? That is not the direction we want to g […]
As per my discussion with EE team, there is no harm in probing the touchscreen in the firmware. EE team is concerned with two things:
1) Power sequencing
2) Touchscreen being on during suspend.
The right power sequencing is to enable the touchscreen before de-asserting the reset. Here is how it can be done
In romstage_gpio_table
```
/* GPP_F7 : [NF6: USB_C_GPP_F7] ==> EN_PP3300_TCHSCR */
PAD_CFG_GPO(GPP_F7, 1, PLTRST),
/* GPP_F17 : [NF3: THC1_SPI2_RST# NF6: USB_C_GPP_F17] ==> TCHSCR_RST_L */
PAD_CFG_GPO(GPP_F17, 0, DEEP),
```
In ramstage_gpio_table
```
/* GPP_F7 : [NF6: USB_C_GPP_F7] ==> EN_PP3300_TCHSCR */
PAD_CFG_GPO(GPP_F7, 1, PLTRST),
/* GPP_F17 : [NF3: THC1_SPI2_RST# NF6: USB_C_GPP_F17] ==> TCHSCR_RST_L */
PAD_CFG_GPO(GPP_F17, 1, DEEP),
```
Regarding touchscreen being on during S0ix, has_power_resource = 1 will add the required power resources in the ACPI table which will be used by OS to turn off the Touchscreen during suspend.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80300?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5bf1901a3a40a38237b950abcb758f96aebcc1cf
Gerrit-Change-Number: 80300
Gerrit-PatchSet: 2
Gerrit-Owner: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Lai <ericllai(a)google.com>
Gerrit-CC: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Tue, 06 Feb 2024 19:12:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment