The FSP-S changes the ITSS priorities. The code that tries to save it before running FSP-S and restore it afterwards does not work as U-Boot relocates in between the save and restore. This means that the driver data saved before relocation is lost and the new driver just sees zeroes.

Fix this by allocating space in the relocated memory for the ITSS data. Save it there and access it from the driver after relocation.

This fixes interrupt handling on coral.

Also drop the log_msg_ret() in irq_first_device_type() since this function can be called speculatively in places where we are not sure if there is an interrupt controller of that type. The resulting log errors are confusing when there is no error.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---

(no changes since v2)

Changes in v2: - Add mention of why log_msg_ret() is dropped

arch/x86/cpu/apollolake/fsp_s.c | 11 +++++------ arch/x86/cpu/cpu.c | 18 +++++++++++++++--- arch/x86/cpu/intel_common/itss.c | 25 +++++++++++++++++++++++-- arch/x86/include/asm/global_data.h | 1 + arch/x86/include/asm/itss.h | 2 +- drivers/misc/irq-uclass.c | 2 +- 6 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c index 3a54297a28..e54b0ac104 100644 --- a/arch/x86/cpu/apollolake/fsp_s.c +++ b/arch/x86/cpu/apollolake/fsp_s.c @@ -160,11 +160,6 @@ int arch_fsps_preinit(void) ret = irq_first_device_type(X86_IRQT_ITSS, &itss); if (ret) return log_msg_ret("no itss", ret); - /* - * Snapshot the current GPIO IRQ polarities. FSP is setting a default - * policy that doesn't honour boards' requirements - */ - irq_snapshot_polarities(itss);

/* * Clear the GPI interrupt status and enable registers. These @@ -203,7 +198,11 @@ int arch_fsp_init_r(void) ret = irq_first_device_type(X86_IRQT_ITSS, &itss); if (ret) return log_msg_ret("no itss", ret); - /* Restore GPIO IRQ polarities back to previous settings */ + + /* + * Restore GPIO IRQ polarities back to previous settings. This was + * stored in reserve_arch() - see X86_IRQT_ITSS + */ irq_restore_polarities(itss);

/* soc_init() */ diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 23a4d633d2..9ef797b41b 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -25,6 +25,7 @@ #include <dm.h> #include <errno.h> #include <init.h> +#include <irq.h> #include <log.h> #include <malloc.h> #include <syscon.h> @@ -269,9 +270,11 @@ int cpu_init_r(void) #ifndef CONFIG_EFI_STUB int reserve_arch(void) { -#ifdef CONFIG_ENABLE_MRC_CACHE - mrccache_reserve(); -#endif + struct udevice *itss; + int ret; + + if (IS_ENABLED(CONFIG_ENABLE_MRC_CACHE)) + mrccache_reserve();

#ifdef CONFIG_SEABIOS high_table_reserve(); @@ -288,6 +291,15 @@ int reserve_arch(void) fsp_save_s3_stack(); } } + ret = irq_first_device_type(X86_IRQT_ITSS, &itss); + if (!ret) { + /* + * Snapshot the current GPIO IRQ polarities. FSP-S is about to + * run and will set a default policy that doesn't honour boards' + * requirements + */ + irq_snapshot_polarities(itss); + }

return 0; } diff --git a/arch/x86/cpu/intel_common/itss.c b/arch/x86/cpu/intel_common/itss.c index 963afa8f5b..fe84ebe29f 100644 --- a/arch/x86/cpu/intel_common/itss.c +++ b/arch/x86/cpu/intel_common/itss.c @@ -65,14 +65,23 @@ static int snapshot_polarities(struct udevice *dev) int i;

reg_start = start / IRQS_PER_IPC; - reg_end = (end + IRQS_PER_IPC - 1) / IRQS_PER_IPC; + reg_end = DIV_ROUND_UP(end, IRQS_PER_IPC);

+ log_info("ITSS IRQ Polarities snapshot %p\n", priv->irq_snapshot); for (i = reg_start; i < reg_end; i++) { uint reg = PCR_ITSS_IPC0_CONF + sizeof(u32) * i;

priv->irq_snapshot[i] = pcr_read32(dev, reg); + log_debug(" - %d, reg %x: irq_snapshot[i] %x\n", i, reg, + priv->irq_snapshot[i]); }

+ /* Save the snapshot for use after relocation */ + gd->start_addr_sp -= sizeof(*priv); + gd->start_addr_sp &= ~0xf; + gd->arch.itss_priv = (void *)gd->start_addr_sp; + memcpy(gd->arch.itss_priv, priv, sizeof(*priv)); + return 0; }

@@ -91,16 +100,26 @@ static void show_polarities(struct udevice *dev, const char *msg) static int restore_polarities(struct udevice *dev) { struct itss_priv *priv = dev_get_priv(dev); + struct itss_priv *old_priv; const int start = GPIO_IRQ_START; const int end = GPIO_IRQ_END; int reg_start; int reg_end; int i;

+ /* Get the snapshot which was stored by the pre-reloc device */ + old_priv = gd->arch.itss_priv; + if (!old_priv) + return log_msg_ret("priv", -EFAULT); + memcpy(priv->irq_snapshot, old_priv->irq_snapshot, + sizeof(priv->irq_snapshot)); + show_polarities(dev, "Before"); + log_info("priv->irq_snapshot %p\n", priv->irq_snapshot);

reg_start = start / IRQS_PER_IPC; - reg_end = (end + IRQS_PER_IPC - 1) / IRQS_PER_IPC; + reg_end = DIV_ROUND_UP(end, IRQS_PER_IPC); +

for (i = reg_start; i < reg_end; i++) { u32 mask; @@ -125,6 +144,8 @@ static int restore_polarities(struct udevice *dev) mask &= ~((1U << irq_start) - 1);

reg = PCR_ITSS_IPC0_CONF + sizeof(u32) * i; + log_debug(" - %d, reg %x: mask %x, irq_snapshot[i] %x\n", + i, reg, mask, priv->irq_snapshot[i]); pcr_clrsetbits32(dev, reg, mask, mask & priv->irq_snapshot[i]); }

diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 0e64c8a46d..5bc251c0dd 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -121,6 +121,7 @@ struct arch_global_data { #ifdef CONFIG_FSP_VERSION2 struct fsp_header *fsp_s_hdr; /* Pointer to FSP-S header */ #endif + void *itss_priv; /* Private ITSS data pointer */ ulong acpi_start; /* Start address of ACPI tables */ };

diff --git a/arch/x86/include/asm/itss.h b/arch/x86/include/asm/itss.h index c75d8fe8c2..f7d3240384 100644 --- a/arch/x86/include/asm/itss.h +++ b/arch/x86/include/asm/itss.h @@ -16,7 +16,7 @@

#define ITSS_MAX_IRQ 119 #define IRQS_PER_IPC 32 -#define NUM_IPC_REGS ((ITSS_MAX_IRQ + IRQS_PER_IPC - 1) / IRQS_PER_IPC) +#define NUM_IPC_REGS DIV_ROUND_UP(ITSS_MAX_IRQ, IRQS_PER_IPC)

/* Max PXRC registers in ITSS */ #define MAX_PXRC_CONFIG (PCR_ITSS_PIRQH_ROUT - PCR_ITSS_PIRQA_ROUT + 1) diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c index 8727a33dd9..94fa233f19 100644 --- a/drivers/misc/irq-uclass.c +++ b/drivers/misc/irq-uclass.c @@ -168,7 +168,7 @@ int irq_first_device_type(enum irq_dev_t type, struct udevice **devp)

ret = uclass_first_device_drvdata(UCLASS_IRQ, type, devp); if (ret) - return log_msg_ret("find", ret); + return ret;

return 0; }

If there is MRC information we should run FSP-M with a different boot_mode flag since it is supposed to do a 'fast path' through the memory init. Fix this.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com

Reviewed-by: Bin Meng bmeng.cn@gmail.com ---

(no changes since v2)

Changes in v2: - Add a new commit to handle the boot_mode fix

arch/x86/cpu/apollolake/fsp_m.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c index 65461d85b8..e19a2b0826 100644 --- a/arch/x86/cpu/apollolake/fsp_m.c +++ b/arch/x86/cpu/apollolake/fsp_m.c @@ -26,7 +26,8 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd) return log_msg_ret("mrc", cache_ret); arch->stack_base = (void *)0xfef96000; arch->boot_loader_tolum_size = 0; - arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION; + arch->boot_mode = cache_ret ? FSP_BOOT_WITH_FULL_CONFIGURATION : + FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES;

node = dev_ofnode(dev); if (!ofnode_valid(node))

At present this information is used to locate and parse the tables but is not stored. Store it so that we can display it to the user, e.g. with the 'bdinfo' command.

Note that now the GD_FLG_SKIP_LL_INIT flag is set in get_coreboot_info(), so it is always set when booting from coreboot.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---

Changes in v3: - Update the commit message with an explanation of GD_FLG_SKIP_LL_INIT

arch/x86/cpu/coreboot/tables.c | 8 +++++++- arch/x86/cpu/i386/cpu.c | 7 ++++++- arch/x86/include/asm/global_data.h | 1 + 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/cpu/coreboot/tables.c b/arch/x86/cpu/coreboot/tables.c index a5d31d1dea..1594b4a8b2 100644 --- a/arch/x86/cpu/coreboot/tables.c +++ b/arch/x86/cpu/coreboot/tables.c @@ -10,6 +10,8 @@ #include <net.h> #include <asm/arch/sysinfo.h>

+DECLARE_GLOBAL_DATA_PTR; + /* * This needs to be in the .data section so that it's copied over during * relocation. By default it's put in the .bss section which is simply filled @@ -243,6 +245,10 @@ int get_coreboot_info(struct sysinfo_t *info) if (addr < 0) return addr; ret = cb_parse_header((void *)addr, 0x1000, info); + if (!ret) + return -ENOENT; + gd->arch.coreboot_table = addr; + gd->flags |= GD_FLG_SKIP_LL_INIT;

- return ret == 1 ? 0 : -ENOENT; + return 0; } diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c index d27324cb4e..a6a6afec8c 100644 --- a/arch/x86/cpu/i386/cpu.c +++ b/arch/x86/cpu/i386/cpu.c @@ -455,10 +455,15 @@ int x86_cpu_init_f(void)

int x86_cpu_reinit_f(void) { + long addr; + setup_identity(); setup_pci_ram_top(); - if (locate_coreboot_table() >= 0) + addr = locate_coreboot_table(); + if (addr >= 0) { + gd->arch.coreboot_table = addr; gd->flags |= GD_FLG_SKIP_LL_INIT; + }

return 0; } diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 5bc251c0dd..3e4044593c 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -123,6 +123,7 @@ struct arch_global_data { #endif void *itss_priv; /* Private ITSS data pointer */ ulong acpi_start; /* Start address of ACPI tables */ + ulong coreboot_table; /* Address of coreboot table */ };

#endif

This function does not exist anymore. Drop it from the header file.

Signed-off-by: Simon Glass sjg@chromium.org

Reviewed-by: Bin Meng bmeng.cn@gmail.com ---

(no changes since v2)

Changes in v2: - Remove the function from zimage.c also

arch/x86/include/asm/u-boot-x86.h | 2 -- arch/x86/lib/zimage.c | 10 ---------- 2 files changed, 12 deletions(-)

diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index bd3f44014c..d732661f6d 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -83,8 +83,6 @@ int default_print_cpuinfo(void); /* Set up a UART which can be used with printch(), printhex8(), etc. */ int setup_internal_uart(int enable);

-void setup_pcat_compatibility(void); - void isa_unmap_rom(u32 addr); u32 isa_map_rom(u32 bus_addr, int size);

diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c index 64d14e8911..d2b6002008 100644 --- a/arch/x86/lib/zimage.c +++ b/arch/x86/lib/zimage.c @@ -304,13 +304,6 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot, return 0; }

-void setup_pcat_compatibility(void) - __attribute__((weak, alias("__setup_pcat_compatibility"))); - -void __setup_pcat_compatibility(void) -{ -} - int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct boot_params *base_ptr; @@ -323,9 +316,6 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])

disable_interrupts();

- /* Setup board for maximum PC/AT Compatibility */ - setup_pcat_compatibility(); - if (argc >= 2) { /* argv[1] holds the address of the bzImage */ s = argv[1];

This function sounds like something that is called when U-Boot is about to jump to Linux. In fact it is an init function.

Rename it to reduce confusion.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com ---

(no changes since v1)

arch/x86/cpu/coreboot/coreboot.c | 4 ++-- arch/x86/cpu/cpu.c | 8 ++++---- arch/x86/cpu/efi/app.c | 2 +- arch/x86/cpu/quark/quark.c | 2 +- arch/x86/lib/fsp/fsp_common.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c index d44db1347b..22a93254a9 100644 --- a/arch/x86/cpu/coreboot/coreboot.c +++ b/arch/x86/cpu/coreboot/coreboot.c @@ -42,7 +42,7 @@ int print_cpuinfo(void) return default_print_cpuinfo(); }

-static void board_final_cleanup(void) +static void board_final_init(void) { /* * Un-cache the ROM so the kernel has one @@ -80,7 +80,7 @@ int last_stage_init(void) if (CONFIG_IS_ENABLED(USB_KEYBOARD)) usb_init();

- board_final_cleanup(); + board_final_init();

return 0; } diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 9ef797b41b..98ed66e67d 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -179,10 +179,10 @@ void show_boot_progress(int val)

#if !defined(CONFIG_SYS_COREBOOT) && !defined(CONFIG_EFI_STUB) /* - * Implement a weak default function for boards that optionally - * need to clean up the system before jumping to the kernel. + * Implement a weak default function for boards that need to do some final init + * before the system is ready. */ -__weak void board_final_cleanup(void) +__weak void board_final_init(void) { }

@@ -190,7 +190,7 @@ int last_stage_init(void) { struct acpi_fadt __maybe_unused *fadt;

- board_final_cleanup(); + board_final_init();

if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) { fadt = acpi_find_fadt(); diff --git a/arch/x86/cpu/efi/app.c b/arch/x86/cpu/efi/app.c index 10677ecbc2..f754489784 100644 --- a/arch/x86/cpu/efi/app.c +++ b/arch/x86/cpu/efi/app.c @@ -24,7 +24,7 @@ int print_cpuinfo(void) return default_print_cpuinfo(); }

-void board_final_cleanup(void) +void board_final_init(void) { }

diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c index ddad02e375..30b4711b9a 100644 --- a/arch/x86/cpu/quark/quark.c +++ b/arch/x86/cpu/quark/quark.c @@ -363,7 +363,7 @@ int arch_misc_init(void) return 0; }

-void board_final_cleanup(void) +void board_final_init(void) { struct quark_rcba *rcba; u32 base, val; diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c index 8e3082d4c8..ea52954725 100644 --- a/arch/x86/lib/fsp/fsp_common.c +++ b/arch/x86/lib/fsp/fsp_common.c @@ -47,7 +47,7 @@ int fsp_init_phase_pci(void) return status ? -EPERM : 0; }

-void board_final_cleanup(void) +void board_final_init(void) { u32 status;