Hello Iru Cai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44065
to review the following change.
Change subject: cpu/intel/common/fsb.c: add Crystal Well support
......................................................................
cpu/intel/common/fsb.c: add Crystal Well support
Without this change, there will be no console output when using a
Crystal Well CPU.
Tested with i5-4570R (with LGA1150 mod) on ASRock H81M-HDS.
Change-Id: Id18645c52d9c4a4ea7acb602bcb39b796d9e24b9
Signed-off-by: Iru Cai <mytbk920423(a)gmail.com>
---
M src/cpu/intel/common/fsb.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/44065/1
diff --git a/src/cpu/intel/common/fsb.c b/src/cpu/intel/common/fsb.c
index 7772171..3d46bbc 100644
--- a/src/cpu/intel/common/fsb.c
+++ b/src/cpu/intel/common/fsb.c
@@ -45,6 +45,7 @@
case 0x3a: /* IvyBridge BCLK fixed at 100MHz */
case 0x3c: /* Haswell BCLK fixed at 100MHz */
case 0x45: /* Haswell-ULT BCLK fixed at 100MHz */
+ case 0x46: /* Haswell-GT3e BCLK fixed at 100MHz */
*fsb = 100;
*ratio = (rdmsr(MSR_PLATFORM_INFO).lo >> 8) & 0xff;
break;
--
To view, visit https://review.coreboot.org/c/coreboot/+/44065
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id18645c52d9c4a4ea7acb602bcb39b796d9e24b9
Gerrit-Change-Number: 44065
Gerrit-PatchSet: 1
Gerrit-Owner: Iru Cai (vimacs) <mytbk920423(a)gmail.com>
Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Hello Shelley Chen, Furquan Shaikh, Angel Pons,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44044
to review the following change.
Change subject: assert.h: Try to evaluate assertions at compile time
......................................................................
assert.h: Try to evaluate assertions at compile time
Many places in coreboot seem to like to do things like
assert(CONFIG(SOME_KCONFIG));
This is somewhat suboptimal since assert() is a runtime check, so you
don't see that this fails until someone actually tries to boot it even
though the compiler is totally aware of it already. We already have the
dead_code() macro to do this better:
if (CONFIG(SOME_KCONFIG))
dead_code();
Rather than fixing all these and trying to carefully educate people
about which type of check is more appropriate in what situation, we can
just employ the magic of __builtin_constant_p() to automatically make
the former statement behave like the latter.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I06691b732598eb2a847a17167a1cb92149710916
---
M src/include/assert.h
1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/44044/1
diff --git a/src/include/assert.h b/src/include/assert.h
index 0463175..d9883fd 100644
--- a/src/include/assert.h
+++ b/src/include/assert.h
@@ -22,9 +22,12 @@
#define __ASSERT_LINE__ __LINE__
#endif
+#define __build_time_assert(x) (__builtin_constant_p(x) ? \
+ ((x) ? 1 : dead_code_t(int)) : 0)
+
/* GCC and CAR versions */
#define ASSERT(x) { \
- if (!(x)) { \
+ if (!__build_time_assert(x) && !(x)) { \
printk(BIOS_EMERG, \
"ASSERTION ERROR: file '%s', line %d\n", \
__ASSERT_FILE__, __ASSERT_LINE__); \
@@ -33,7 +36,7 @@
} \
}
#define ASSERT_MSG(x, msg) { \
- if (!(x)) { \
+ if (!__build_time_assert(x) && !(x)) { \
printk(BIOS_EMERG, \
"ASSERTION ERROR: file '%s', line %d\n", \
__ASSERT_FILE__, __ASSERT_LINE__); \
--
To view, visit https://review.coreboot.org/c/coreboot/+/44044
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I06691b732598eb2a847a17167a1cb92149710916
Gerrit-Change-Number: 44044
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-MessageType: newchange
Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44047 )
Change subject: Change all assert(0) to BUG()
......................................................................
Change all assert(0) to BUG()
I would like to make assertions evaluate at compile time where possible,
but sometimes people used a literal assert(0) to force an assertion in a
certain code path. We already have BUG() for that so let's just replace
those instances with that.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: I674e5f8ec7f5fe8b92b1c7c95d9f9202d422ce32
---
M src/device/device_const.c
M src/soc/amd/common/block/gpio_banks/gpio.c
M src/soc/amd/picasso/reset.c
M src/soc/intel/common/block/gpio/gpio.c
M src/soc/mediatek/mt8183/mt6358.c
M src/southbridge/amd/pi/hudson/early_setup.c
6 files changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/44047/1
diff --git a/src/device/device_const.c b/src/device/device_const.c
index 79f025d..2dc583c 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -184,7 +184,7 @@
DEVTREE_CONST struct device *child;
if (!parent) {
- assert(0);
+ BUG();
/* Return NULL in case asserts are considered non-fatal. */
return NULL;
}
@@ -282,7 +282,7 @@
pci_devfn_t devfn)
{
if (!bridge || (bridge->path.type != DEVICE_PATH_PCI)) {
- assert(0);
+ BUG();
/* Return NULL in case asserts are non-fatal. */
return NULL;
}
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c
index b9646b9..4f1b842 100644
--- a/src/soc/amd/common/block/gpio_banks/gpio.c
+++ b/src/soc/amd/common/block/gpio_banks/gpio.c
@@ -28,7 +28,7 @@
if (!is_gpio_event_level_triggered(flags)) {
printk(BIOS_ERR, "ERROR: %s - Only level trigger allowed for SMI!\n", __func__);
- assert(0);
+ BUG();
return;
}
diff --git a/src/soc/amd/picasso/reset.c b/src/soc/amd/picasso/reset.c
index b6aeb1f..902538ff 100644
--- a/src/soc/amd/picasso/reset.c
+++ b/src/soc/amd/picasso/reset.c
@@ -49,6 +49,6 @@
{
printk(BIOS_ERR, "Error: unexpected call to %s(0x%08x). Doing cold reset.\n",
__func__, status);
- assert(0);
+ BUG();
do_cold_reset();
}
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c
index ee91aa6..d6958b1 100644
--- a/src/soc/intel/common/block/gpio/gpio.c
+++ b/src/soc/intel/common/block/gpio/gpio.c
@@ -78,7 +78,7 @@
}
printk(BIOS_ERR, "%s: pad %d is not found in community %s!\n",
__func__, relative_pad, comm->name);
- assert(0);
+ BUG();
return i;
}
diff --git a/src/soc/mediatek/mt8183/mt6358.c b/src/soc/mediatek/mt8183/mt6358.c
index 7b4febc..6e6e43a 100644
--- a/src/soc/mediatek/mt8183/mt6358.c
+++ b/src/soc/mediatek/mt8183/mt6358.c
@@ -831,7 +831,7 @@
break;
default:
- assert(0);
+ BUG();
return;
};
diff --git a/src/southbridge/amd/pi/hudson/early_setup.c b/src/southbridge/amd/pi/hudson/early_setup.c
index 14bb5e2..f43dbb9 100644
--- a/src/southbridge/amd/pi/hudson/early_setup.c
+++ b/src/southbridge/amd/pi/hudson/early_setup.c
@@ -187,7 +187,7 @@
pci_write_config32(dev, LPC_WIDEIO2_GENERIC_PORT, tmp);
enable_wideio(2, size);
} else { /* All WIDEIO locations used*/
- assert(0);
+ BUG();
}
}
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/44047
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I674e5f8ec7f5fe8b92b1c7c95d9f9202d422ce32
Gerrit-Change-Number: 44047
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: newchange
Hello Alex Levin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44089
to review the following change.
Change subject: mb/google/volteer: Change wake to be triggered on a raising edge
......................................................................
mb/google/volteer: Change wake to be triggered on a raising edge
ACPI_GPIO_IRQ_EDGE_BOTH sets both edges as wake. The desired behavior is wake on rising edge, change to ACPI_GPIO_INPUT_ACTIVE_LOW.
Fixing for both Volteer and Volteer2 variants.
BUG=b:146083964
BRANCH=None
TEST=tested on a Volteer
Change-Id: I2d3339151bf4e2cbae60aaf97ba1bd7909a2b9a9
---
M src/mainboard/google/volteer/variants/volteer/overridetree.cb
M src/mainboard/google/volteer/variants/volteer2/overridetree.cb
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/44089/1
diff --git a/src/mainboard/google/volteer/variants/volteer/overridetree.cb b/src/mainboard/google/volteer/variants/volteer/overridetree.cb
index c7350cb..d4bd7f4 100644
--- a/src/mainboard/google/volteer/variants/volteer/overridetree.cb
+++ b/src/mainboard/google/volteer/variants/volteer/overridetree.cb
@@ -114,7 +114,7 @@
chip drivers/generic/gpio_keys
register "name" = ""PENH""
# GPP_B3 is the IRQ source, and GPP_E1 is the wake source
- register "gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_B3)"
+ register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_LOW(GPP_B3)"
register "key.wake_gpe" = "GPE0_DW2_01"
register "key.wakeup_route" = "WAKEUP_ROUTE_SCI"
register "key.wakeup_event_action" = "EV_ACT_DEASSERTED"
diff --git a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb
index 8e5985e..62749fc 100644
--- a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb
+++ b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb
@@ -70,7 +70,7 @@
chip drivers/generic/gpio_keys
register "name" = ""PENH""
# GPP_B3 is the IRQ source, and GPP_E1 is the wake source
- register "gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_B3)"
+ register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_LOW(GPP_B3)"
register "key.wake_gpe" = "GPE0_DW2_01"
register "key.wakeup_route" = "WAKEUP_ROUTE_SCI"
register "key.wakeup_event_action" = "EV_ACT_DEASSERTED"
--
To view, visit https://review.coreboot.org/c/coreboot/+/44089
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2d3339151bf4e2cbae60aaf97ba1bd7909a2b9a9
Gerrit-Change-Number: 44089
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Alex Levin <levinale(a)chromium.org>
Gerrit-MessageType: newchange
Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39187 )
Change subject: mb/emulation/qemu-armv7: Fix board
......................................................................
mb/emulation/qemu-armv7: Fix board
Fix multiple issues allowing to boot until "Payload not loaded":
* The FMAP_CACHE was placed in memory mapped flash
- Place the FMAP_CACHE in DRAM.
* The FMAP_CACHE was overlapping the BOOTBLOCK, which has a default size
of 128KiB.
- Increase the bootblock size in memlayout to 128KiB to match the FMAP.
* The heap in bootblock wasn't usable.
- Move the bootblock to DRAM and add custom relocation code.
* A FIT payload couldn't be compiled in as the POSTRAM_CBFS_CACHE was
missing.
- Add the POSTRAM_CBFS_CACHE to memlayout.
* The coreboot log is spammed with missing timestamp table error messages
- Add TIMESTAMP table to memlayout.
Tested on QEMU armv7 vexpress.
Change-Id: Ib9357a5c059ca179826c5a7e7616a5c688ec2e95
Signed-off-by: Patrick Rudolph <siro(a)das-labor.org>
---
M src/mainboard/emulation/qemu-armv7/Kconfig
M src/mainboard/emulation/qemu-armv7/Makefile.inc
A src/mainboard/emulation/qemu-armv7/bootblock_asm.S
M src/mainboard/emulation/qemu-armv7/memlayout.ld
4 files changed, 117 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/39187/1
diff --git a/src/mainboard/emulation/qemu-armv7/Kconfig b/src/mainboard/emulation/qemu-armv7/Kconfig
index 181f9a4..8c7551f 100644
--- a/src/mainboard/emulation/qemu-armv7/Kconfig
+++ b/src/mainboard/emulation/qemu-armv7/Kconfig
@@ -36,6 +36,7 @@
select BOOT_DEVICE_NOT_SPI_FLASH
select MISSING_BOARD_RESET
select NO_MONOTONIC_TIMER
+ select BOOTBLOCK_CUSTOM
config MAINBOARD_DIR
string
diff --git a/src/mainboard/emulation/qemu-armv7/Makefile.inc b/src/mainboard/emulation/qemu-armv7/Makefile.inc
index c62915b..8b350af 100644
--- a/src/mainboard/emulation/qemu-armv7/Makefile.inc
+++ b/src/mainboard/emulation/qemu-armv7/Makefile.inc
@@ -12,6 +12,8 @@
## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
## GNU General Public License for more details.
+bootblock-y += bootblock_asm.S
+
romstage-y += romstage.c
romstage-y += cbmem.c
diff --git a/src/mainboard/emulation/qemu-armv7/bootblock_asm.S b/src/mainboard/emulation/qemu-armv7/bootblock_asm.S
new file mode 100644
index 0000000..d2c8b75
--- /dev/null
+++ b/src/mainboard/emulation/qemu-armv7/bootblock_asm.S
@@ -0,0 +1,106 @@
+/*
+ * Early initialization code for ARM architecture.
+ *
+ * This file is based off of the OMAP3530/ARM Cortex start.S file from Das
+ * U-Boot, which itself got the file from armboot.
+ *
+ * Copyright (c) 2004 Texas Instruments <r-woodruff2(a)ti.com>
+ * Copyright (c) 2001 Marius Gröger <mag(a)sysgo.de>
+ * Copyright (c) 2002 Alex Züpke <azu(a)sysgo.de>
+ * Copyright (c) 2002 Gary Jennejohn <garyj(a)denx.de>
+ * Copyright (c) 2003 Richard Woodruff <r-woodruff2(a)ti.com>
+ * Copyright (c) 2003 Kshitij <kshitij(a)ti.com>
+ * Copyright (c) 2006-2008 Syed Mohammed Khasim <x0khasim(a)ti.com>
+ * Copyright (c) 2013 The Chromium OS Authors
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <arch/asm.h>
+.arm
+
+ENTRY(_start)
+ /*
+ * Set the CPU to System mode with IRQ and FIQ disabled. Prefetch/Data
+ * aborts may happen early and crash before the abort handlers are
+ * installed, but at least the problem will show up near the code that
+ * causes it.
+ */
+ msr cpsr_cxf, #0xdf
+
+ /*
+ * From Cortex-A Series Programmer's Guide:
+ * Only CPU 0 performs initialization. Other CPUs go into WFI
+ * to do this, first work out which CPU this is
+ * this code typically is run before any other initialization step
+ */
+ mrc p15, 0, r1, c0, c0, 5 @ Read Multiprocessor Affinity Register
+ and r1, r1, #0x3 @ Extract CPU ID bits
+ cmp r1, #0
+ bne wait_for_interrupt @ If this is not core0, wait
+
+ ldr r0, =_bootblock
+ ldr r1, =_ebootblock
+ adr r2, _start
+
+ cmp r0, r2
+ beq relocated
+
+relocate_program:
+ ldmia r2!, {r9-r10}
+ stmia r0!, {r9-r10}
+ cmp r0, r1
+ bne relocate_program
+
+ /* Jump to it... */
+ ldr r0, =_bootblock
+ adr r1, _start
+ add lr, r0, r1
+ mov pc, lr
+
+relocated:
+ /*
+ * Initialize the stack to a known value. This is used to check for
+ * stack overflow later in the boot process.
+ */
+ ldr r0, =_stack
+ ldr r1, =_estack
+ ldr r2, =0xdeadbeef
+init_stack_loop:
+ str r2, [r0]
+ add r0, #4
+ cmp r0, r1
+ bne init_stack_loop
+
+/* Set stackpointer in internal RAM to call bootblock main() */
+call_bootblock:
+ ldr sp, =_estack /* Set up stack pointer */
+ ldr r0,=0x00000000
+ /*
+ * The current design of cpu_info places the
+ * struct at the top of the stack. The number of
+ * words pushed must be at least as large as that
+ * struct.
+ */
+ push {r0-r2}
+ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
+ /*
+ * Use "bl" instead of "b" even though we do not intend to return.
+ * "bl" gets compiled to "blx" if we're transitioning from ARM to
+ * Thumb. However, "b" will not and GCC may attempt to create a
+ * wrapper which is currently broken.
+ */
+ bl main
+
+wait_for_interrupt:
+ wfi
+ mov pc, lr @ back to my caller
+ENDPROC(_start)
diff --git a/src/mainboard/emulation/qemu-armv7/memlayout.ld b/src/mainboard/emulation/qemu-armv7/memlayout.ld
index 2b33cb3..3fa2234 100644
--- a/src/mainboard/emulation/qemu-armv7/memlayout.ld
+++ b/src/mainboard/emulation/qemu-armv7/memlayout.ld
@@ -41,14 +41,16 @@
{
/* TODO: does this thing emulate SRAM? */
- BOOTBLOCK(0x00000, 64K)
- FMAP_CACHE(0x10000, 2K)
+ REGION(flash, 0, CONFIG_ROM_SIZE, 4K)
DRAM_START(0x60000000)
STACK(0x60000000, 64K)
- ROMSTAGE(0x60010000, 128K)
- RAMSTAGE(0x60030000, 16M)
-
+ BOOTBLOCK(0x60010000, 128K)
+ FMAP_CACHE(0x60030000, 4K)
+ TIMESTAMP(0x60031000, 1K)
/* TODO: Implement MMU support and move TTB to a better location. */
- TTB(0x61030000, 16K)
+ TTB(0x60034000, 16K)
+ ROMSTAGE(0x60038000, 128K)
+ RAMSTAGE(0x60060000, 16M)
+ POSTRAM_CBFS_CACHE(0x61060000, 8M)
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/39187
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib9357a5c059ca179826c5a7e7616a5c688ec2e95
Gerrit-Change-Number: 39187
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange