Change in coreboot[master]: mb/ocp/deltalake: Override uart base address via VPD variable
Bryant Ou has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "uart_ioport" to override base address at run time, the data will be used during FSP/coreboot if UART_OVERRIDE_BASE_ADDR is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uart.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 66 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/1
diff --git a/src/mainboard/ocp/deltalake/Kconfig b/src/mainboard/ocp/deltalake/Kconfig index adce988..5671664 100644 --- a/src/mainboard/ocp/deltalake/Kconfig +++ b/src/mainboard/ocp/deltalake/Kconfig @@ -16,6 +16,7 @@ select IPMI_OCP select MAINBOARD_HAS_LPC_TPM select MAINBOARD_HAS_TPM2 + select UART_OVERRIDE_BASE_ADDR
config POWER_STATE_DEFAULT_ON_AFTER_FAILURE default n diff --git a/src/mainboard/ocp/deltalake/Makefile.inc b/src/mainboard/ocp/deltalake/Makefile.inc index be6af24..ecef891 100644 --- a/src/mainboard/ocp/deltalake/Makefile.inc +++ b/src/mainboard/ocp/deltalake/Makefile.inc @@ -8,5 +8,14 @@ ramstage-y += ramstage.c ipmi.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += fadt.c
+ifeq ($(CONFIG_UART_OVERRIDE_BASE_ADDR),y) +bootblock-y += uart.c +verstage-y += uart.c +romstage-y += uart.c +postcar-y += uart.c +ramstage-y += uart.c +smm-$(CONFIG_DEBUG_SMI) += uart.c +endif + CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include CPPFLAGS_common += -I$(CONFIG_FSP_HEADER_PATH) diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c index 71a26c8..bf7491c 100644 --- a/src/mainboard/ocp/deltalake/romstage.c +++ b/src/mainboard/ocp/deltalake/romstage.c @@ -20,6 +20,8 @@ { uint8_t val; char val_str[VPD_LEN]; + uint16_t base; + char port[VPD_LEN];
/* Send FSP log message to SOL */ if (vpd_get_bool(FSP_LOG, VPD_RW_THEN_RO, &val)) @@ -29,7 +31,12 @@ "SerialIoUartDebugEnable to %d\n", FSP_LOG, FSP_LOG_DEFAULT); mupd->FspmConfig.SerialIoUartDebugEnable = FSP_LOG_DEFAULT; } - mupd->FspmConfig.SerialIoUartDebugIoBase = 0x2f8; + + if (vpd_gets(UART_IOPORT, port, VPD_LEN, VPD_RW_THEN_RO)) + base = hex2int(port); + else + base = hex2int(UART_IOPORT_DEFAULT); + mupd->FspmConfig.SerialIoUartDebugIoBase = base;
if (mupd->FspmConfig.SerialIoUartDebugEnable) { /* FSP debug log level */ diff --git a/src/mainboard/ocp/deltalake/uart.c b/src/mainboard/ocp/deltalake/uart.c new file mode 100644 index 0000000..28801c8 --- /dev/null +++ b/src/mainboard/ocp/deltalake/uart.c @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/uart.h> +#include <drivers/vpd/vpd.h> +#include <string.h> +#include "vpd.h" + +int hex2int(const char *hexstr) +{ + int val = 0; + + if (hexstr[0] == '0' && hexstr[1] == 'x') + hexstr = hexstr + 2; + + while (*hexstr) { + char c = *hexstr++; + if (c >= '0' && c <= '9') + c = c - '0'; + else if (c >= 'a' && c <='f') + c = c - 'a' + 10; + else if (c >= 'A' && c <='F') + c = c - 'A' + 10; + val = (val << 4) | (c & 0xF); + } + + return val; +} + +#if (CONFIG(UART_OVERRIDE_BASE_ADDR)) +uintptr_t uart_platform_base(unsigned int idx) +{ + char port[VPD_LEN]; + uintptr_t base; + + if (vpd_gets(UART_IOPORT, port, VPD_LEN, VPD_RW_THEN_RO)) + base = hex2int(port); + else + base = hex2int(UART_IOPORT_DEFAULT); + + return base; +} +#endif \ No newline at end of file diff --git a/src/mainboard/ocp/deltalake/vpd.h b/src/mainboard/ocp/deltalake/vpd.h index ae2099d..4285e68 100644 --- a/src/mainboard/ocp/deltalake/vpd.h +++ b/src/mainboard/ocp/deltalake/vpd.h @@ -32,4 +32,10 @@ #define FSP_DCI "fsp_dci_enable" /* 1 or 0: enable or disable DCI */ #define FSP_DCI_DEFAULT 0 /* Default value when the VPD variable is not found */
+/* Define uart io port for FSP/coreboot/payload */ +#define UART_IOPORT "uart_ioport" +#define UART_IOPORT_DEFAULT "0x2f8" + +int hex2int(const char *hexstr); + #endif
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 1:
(18 comments)
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/uart.c:
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 10: int val = 0; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 12: if (hexstr[0] == '0' && hexstr[1] == 'x') please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 15: while (*hexstr) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 16: char c = *hexstr++; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 16: char c = *hexstr++; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 17: if (c >= '0' && c <= '9') code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 17: if (c >= '0' && c <= '9') please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 19: else if (c >= 'a' && c <='f') code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 19: else if (c >= 'a' && c <='f') please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 19: else if (c >= 'a' && c <='f') spaces required around that '<=' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 21: else if (c >= 'A' && c <='F') code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 21: else if (c >= 'A' && c <='F') please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 21: else if (c >= 'A' && c <='F') spaces required around that '<=' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 23: val = (val << 4) | (c & 0xF); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 23: val = (val << 4) | (c & 0xF); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 24: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 26: return val; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/45408/1/src/mainboard/ocp/deltalake... PS1, Line 42: #endif adding a line without newline at end of file
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#2).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "uart_ioport" to override base address at run time, the data will be used during FSP/coreboot if UART_OVERRIDE_BASE_ADDR is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uart.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 66 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/2
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45408/2/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/uart.c:
https://review.coreboot.org/c/coreboot/+/45408/2/src/mainboard/ocp/deltalake... PS2, Line 12: if (hexstr[0] == '0' && hexstr[1] == 'x') suspect code indent for conditional statements (8, 8)
https://review.coreboot.org/c/coreboot/+/45408/2/src/mainboard/ocp/deltalake... PS2, Line 19: else if (c >= 'a' && c <='f') spaces required around that '<=' (ctx:WxV)
https://review.coreboot.org/c/coreboot/+/45408/2/src/mainboard/ocp/deltalake... PS2, Line 21: else if (c >= 'A' && c <='F') spaces required around that '<=' (ctx:WxV)
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#3).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "uart_ioport" to override base address at run time, the data will be used during FSP/coreboot if UART_OVERRIDE_BASE_ADDR is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uart.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 66 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/3
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 3:
(1 comment)
I don't think you want to override the uart_init function here. You just want to change the CONFIG from 3f8 to a different port defined by VPD. That should be generic and not platform specific code.
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/vpd.h:
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 37: #define UART_IOPORT_DEFAULT "0x2f8" Why would this be a string and need to be converted?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/uart.c:
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 8: int hex2int(const char *hexstr) src/lib/hexstrtobin.c
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 35: if (vpd_gets(UART_IOPORT, port, VPD_LEN, VPD_RW_THEN_RO)) This sounds like a fundamentally bad idea. VPD was not meant to be used for things like this. We only recently added support to access VPD in pre-RAM stages at all and that was meant to be used only in exceptional cases. The UART is needed *everywhere*, so you're gonna pull the whole VPD code into every stage from the bootblock forward and you're gonna parse the VPD header again and again in every stage. Worse, you're depending the UART on this pretty complex piece of code (which also has a big slew of dependencies) so if anything goes wrong in there you're not gonna get any output at all.
Kconfig is really the appropriate way to set this. If you need it to be modifiable without rebuilding the whole image, you could also just put it in a CBFS file (e.g. fallback/uart_addr or whatever). Or maybe you can use that CMOS option table thing for this. Any of those solutions would be much better suited than VPD.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 3:
How do you make sure it doesn't conflict with other I/O ranges? Why can't you use one of the 4 regular COM port I/O addresses?
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 3:
Hi Bryant,
With "e61391738f mb/ocp/deltalake: Override coreboot log level via VPD" committed, please adjust this patch set in similar way.
Thanks, Jonathan
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Ryback Hung, Philipp Deppenwiese, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#5).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uart.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 6 files changed, 77 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Ryback Hung, Philipp Deppenwiese, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#6).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 51 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/6
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Ryback Hung, Philipp Deppenwiese, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#7).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 51 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/7
Bryant Ou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 7:
Patch Set 3:
Hi Bryant,
With "e61391738f mb/ocp/deltalake: Override coreboot log level via VPD" committed, please adjust this patch set in similar way.
Thanks, Jonathan
Hi Jonathan, I've modified the codes accordingly, please help to review it.
Thanks, Bryant
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 7:
Hi Bryant, some comments: 1. This change updates FSP-M UPD to redirect FSP console. What about coreboot itself? 2. Suggest not to add a new file (uartio_vpd.c). 3. Regarding Linuxboot console change, [CB:47055] will cover it, as kernel uses latter options to override previous options in the command line.
Bryant Ou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 7:
Patch Set 7:
Hi Bryant, some comments:
- This change updates FSP-M UPD to redirect FSP console. What about coreboot itself?
- Suggest not to add a new file (uartio_vpd.c).
- Regarding Linuxboot console change, [CB:47055] will cover it, as kernel uses latter options to override previous options in the command line.
Hi Jonathan, 1. I've committed [CB:45405] to select coreboot's uart IO. 2. Here just follow Johnny's way, and use config token to separate it, should I move the codes to his file? 3. Got it, I'll abandon my change.
Thanks, Bryant
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7:
Hi Bryant, some comments:
- This change updates FSP-M UPD to redirect FSP console. What about coreboot itself?
- Suggest not to add a new file (uartio_vpd.c).
- Regarding Linuxboot console change, [CB:47055] will cover it, as kernel uses latter options to override previous options in the command line.
Hi Jonathan,
- I've committed [CB:45405] to select coreboot's uart IO.
- Here just follow Johnny's way, and use config token to separate it, should I move the codes to his file?
- Got it, I'll abandon my change.
Thanks, Bryant
Thanks Bryant. Looks good. Please solve the build issue for both commits.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 7:
(2 comments)
Thanks, Bryant.
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/uart.c:
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 8: int hex2int(const char *hexstr)
src/lib/hexstrtobin. […]
Bryant, please just use hexstrrtobin().
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 35: if (vpd_gets(UART_IOPORT, port, VPD_LEN, VPD_RW_THEN_RO))
This sounds like a fundamentally bad idea. VPD was not meant to be used for things like this. […]
Bryant, can we switch to use CBFS file (such as fallback/uart_ioport)? We normally use SOL, and it is rare to switch to other ioport. Thanks!
Bryant Ou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 8:
(2 comments)
Patch Set 7:
(2 comments)
Thanks, Bryant.
This file is removed.
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/uart.c:
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 8: int hex2int(const char *hexstr)
Bryant, please just use hexstrrtobin().
This file is removed.
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 35: if (vpd_gets(UART_IOPORT, port, VPD_LEN, VPD_RW_THEN_RO))
Bryant, can we switch to use CBFS file (such as fallback/uart_ioport)? We normally use SOL, and it i […]
This file is removed.
Bryant Ou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/vpd.h:
https://review.coreboot.org/c/coreboot/+/45408/3/src/mainboard/ocp/deltalake... PS3, Line 37: #define UART_IOPORT_DEFAULT "0x2f8"
Why would this be a string and need to be converted?
I've modified it.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 9: Code-Review+1
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph, Jonathan Zhang, Ryback Hung, Philipp Deppenwiese, Tim Chu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45408
to look at the new patch set (#10).
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 51 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/10
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 12:
(2 comments)
This change doesn't touch bootblock_mainboard_early_init(void). How could it have been found working?
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... PS12, Line 36: atol Make use of get_uart_for_console
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... PS12, Line 44: 0x3e8 Is something decoding this address? Does FSP set LPC decode ranges for those addresses?
Tim Chu has uploaded a new patch set (#14) to the change originally created by Bryant Ou. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 48 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/14
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 14:
(2 comments)
Patch Set 12:
(2 comments)
This change doesn't touch bootblock_mainboard_early_init(void). How could it have been found working?
Another patch CB:45405 is added which makes function work.
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... PS12, Line 36: atol
Make use of get_uart_for_console
Done
https://review.coreboot.org/c/coreboot/+/45408/12/src/mainboard/ocp/deltalak... PS12, Line 44: 0x3e8
Is something decoding this address? Does FSP set LPC decode ranges for those addresses?
Yes, FSP set decode ranges for these addresses.
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... PS14, Line 49: default get_uart_for_console should check the vpd setting and return the default if the value is out of range or there will could be an fsp and console mismatch. This can be a lookup like the uart code.
static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 }; base = bases[get_uart_for_console()];
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/uartio_vpd.c:
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... PS14, Line 18: COREBOOT_UART_IO_DEFAULT no need for the else, already set above.
Tim Chu has uploaded a new patch set (#15) to the change originally created by Bryant Ou. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 32 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/15
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 15: Code-Review+1
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/romstage.c:
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... PS14, Line 49: default
get_uart_for_console should check the vpd setting and return the default if the value is out of rang […]
Done
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... File src/mainboard/ocp/deltalake/uartio_vpd.c:
https://review.coreboot.org/c/coreboot/+/45408/14/src/mainboard/ocp/deltalak... PS14, Line 18: COREBOOT_UART_IO_DEFAULT
no need for the else, already set above.
Done
Tim Chu has uploaded a new patch set (#16) to the change originally created by Bryant Ou. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 30 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/45408/16
Attention is currently required from: Bryant Ou, Tim Chu. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
Patch Set 16: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45408 )
Change subject: mb/ocp/deltalake: Override uart base address via VPD variable ......................................................................
mb/ocp/deltalake: Override uart base address via VPD variable
Use VPD of "coreboot_uart_io" to select uart io if OVERRIDE_UART_FOR_CONSOLE is selected.
Tested=On OCP Delta Lake, console messages correctly output to uart port which is defined in VPD.
Signed-off-by: Bryant Ou Bryant.Ou.Q@gmail.com Change-Id: I55a85d6f137ef1aba95466e7b094740b685bf9bd Reviewed-on: https://review.coreboot.org/c/coreboot/+/45408 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jonathan Zhang jonzhang@fb.com --- M src/mainboard/ocp/deltalake/Kconfig M src/mainboard/ocp/deltalake/Makefile.inc M src/mainboard/ocp/deltalake/romstage.c A src/mainboard/ocp/deltalake/uartio_vpd.c M src/mainboard/ocp/deltalake/vpd.h 5 files changed, 30 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Jonathan Zhang: Looks good to me, approved
diff --git a/src/mainboard/ocp/deltalake/Kconfig b/src/mainboard/ocp/deltalake/Kconfig index c439dca..b92bd96 100644 --- a/src/mainboard/ocp/deltalake/Kconfig +++ b/src/mainboard/ocp/deltalake/Kconfig @@ -16,6 +16,7 @@ select IPMI_OCP select MAINBOARD_HAS_LPC_TPM select MAINBOARD_HAS_TPM2 + select OVERRIDE_UART_FOR_CONSOLE
config UART_FOR_CONSOLE int diff --git a/src/mainboard/ocp/deltalake/Makefile.inc b/src/mainboard/ocp/deltalake/Makefile.inc index e961a34..7a92e93 100644 --- a/src/mainboard/ocp/deltalake/Makefile.inc +++ b/src/mainboard/ocp/deltalake/Makefile.inc @@ -8,5 +8,6 @@ ramstage-y += ramstage.c ipmi.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += fadt.c all-$(CONFIG_CONSOLE_OVERRIDE_LOGLEVEL) += loglevel_vpd.c +all-$(CONFIG_OVERRIDE_UART_FOR_CONSOLE) += uartio_vpd.c CPPFLAGS_common += -I$(src)/mainboard/$(MAINBOARDDIR)/include CPPFLAGS_common += -I$(CONFIG_FSP_HEADER_PATH) diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c index f833715..4272e3a 100644 --- a/src/mainboard/ocp/deltalake/romstage.c +++ b/src/mainboard/ocp/deltalake/romstage.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <console/console.h> +#include <console/uart.h> #include <drivers/ipmi/ipmi_kcs.h> #include <drivers/ipmi/ocp/ipmi_ocp.h> #include <drivers/vpd/vpd.h> @@ -29,7 +30,10 @@ "SerialIoUartDebugEnable to %d\n", FSP_LOG, FSP_LOG_DEFAULT); mupd->FspmConfig.SerialIoUartDebugEnable = FSP_LOG_DEFAULT; } - mupd->FspmConfig.SerialIoUartDebugIoBase = 0x2f8; + + /* Select UART IO of FSP */ + static const unsigned int bases[] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 }; + mupd->FspmConfig.SerialIoUartDebugIoBase = bases[get_uart_for_console()];
if (mupd->FspmConfig.SerialIoUartDebugEnable) { /* FSP debug log level */ diff --git a/src/mainboard/ocp/deltalake/uartio_vpd.c b/src/mainboard/ocp/deltalake/uartio_vpd.c new file mode 100644 index 0000000..e7689f5 --- /dev/null +++ b/src/mainboard/ocp/deltalake/uartio_vpd.c @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <console/console.h> +#include <drivers/vpd/vpd.h> +#include <console/uart.h> +#include <string.h> + +#include "vpd.h" + +unsigned int get_uart_for_console(void) +{ + int val_int; + + if (vpd_get_int(COREBOOT_UART_IO, VPD_RW_THEN_RO, (int *const) &val_int)) { + if (val_int > 3) + val_int = COREBOOT_UART_IO_DEFAULT; + } + return val_int; +} diff --git a/src/mainboard/ocp/deltalake/vpd.h b/src/mainboard/ocp/deltalake/vpd.h index 82989ff..55cbc5e 100644 --- a/src/mainboard/ocp/deltalake/vpd.h +++ b/src/mainboard/ocp/deltalake/vpd.h @@ -37,4 +37,8 @@ #define FSPM_MEMREFRESHWATERMARK "fspm_mem_refresh_watermark" #define FSPM_MEMREFRESHWATERMARK_DEFAULT 1
+/* coreboot uart io select: 0 = 0x3f8, 1 = 0x2f8, 2 = 0x3e8, 3 = 0x2e8 */ +#define COREBOOT_UART_IO "coreboot_uart_io" +#define COREBOOT_UART_IO_DEFAULT 1 + #endif
-
Bryant Ou (Code Review) -
build bot (Jenkins) (Code Review)
-
Jonathan Zhang (Code Review)
-
Julius Werner (Code Review)
-
Marc Jones (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Tim Chu (Code Review)