Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38187 )
Change subject: console/post: Split parts to arch/ ......................................................................
console/post: Split parts to arch/
Both IO port and cmos are currently arch/x86 only features.
Change-Id: I010af3f645c0be38dd856657874c36103aebbdc2 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/arch/x86/Makefile.inc A src/arch/x86/post.c M src/console/post.c M src/include/console/console.h 4 files changed, 37 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/38187/1
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 6ed93e5..534f2ce 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -99,6 +99,7 @@ all-y += memcpy.c all-y += memset.c all-y += cpu_common.c +all-y += post.c
endif
diff --git a/src/arch/x86/post.c b/src/arch/x86/post.c new file mode 100644 index 0000000..0a20bab --- /dev/null +++ b/src/arch/x86/post.c @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * 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 <stdint.h> +#include <console/console.h> +#include <arch/io.h> + +void arch_post_code(uint8_t value) +{ + if (CONFIG(POST_IO)) + outb(value, CONFIG_POST_IO_PORT); + + if (CONFIG(CMOS_POST)) + cmos_post_code(value); +} diff --git a/src/console/post.c b/src/console/post.c index b9a7557..33d85e7 100644 --- a/src/console/post.c +++ b/src/console/post.c @@ -14,11 +14,9 @@
#include <stdint.h> #include <console/console.h> -#if CONFIG(POST_IO) -#include <arch/io.h> -#endif
/* Write POST information */ +void __weak arch_post_code(uint8_t value) { }
/* Some mainboards have very nice features beyond just a simple display. * They can override this function. @@ -27,16 +25,13 @@
void post_code(uint8_t value) { -#if !CONFIG(NO_POST) -#if CONFIG(CONSOLE_POST) - printk(BIOS_EMERG, "POST: 0x%02x\n", value); -#endif -#if CONFIG(CMOS_POST) - cmos_post_code(value); -#endif -#if CONFIG(POST_IO) - outb(value, CONFIG_POST_IO_PORT); -#endif -#endif + if (!CONFIG(NO_POST)) { + /* Assume this to be the most reliable and simplest type + for displaying POST so keep it first. */ + arch_post_code(value); + + if (CONFIG(CONSOLE_POST)) + printk(BIOS_EMERG, "POST: 0x%02x\n", value); + } mainboard_post(value); } diff --git a/src/include/console/console.h b/src/include/console/console.h index d3c1d54..c573123 100644 --- a/src/include/console/console.h +++ b/src/include/console/console.h @@ -29,6 +29,7 @@ #include <console/vtxprintf.h>
void post_code(u8 value); +void arch_post_code(u8 value); void cmos_post_code(u8 value); #if CONFIG(CMOS_POST_EXTRA) void post_log_extra(u32 value);
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38187 )
Change subject: console/post: Split parts to arch/ ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38187/4/src/console/post.c File src/console/post.c:
https://review.coreboot.org/c/coreboot/+/38187/4/src/console/post.c@36 PS4, Line 36: mainboard_post(value); I wonder: shouldn't this be inside the if(!CONFIG(NO_POST)) ?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38187 )
Change subject: console/post: Split parts to arch/ ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38187/4/src/console/post.c File src/console/post.c:
https://review.coreboot.org/c/coreboot/+/38187/4/src/console/post.c@36 PS4, Line 36: mainboard_post(value);
I wonder: shouldn't this be inside the if(!CONFIG(NO_POST)) ?
It's used on two boards (see src/mainboard/google/drallion/ec.c and src/mainboard/google/sarien/ec.c) which send the post code to the EC. I guess "no post" is supposed to mean "no post whatsoever" in which case you're right. I'd do it as a separate commit though since that's a change in semantics: https://review.coreboot.org/c/coreboot/+/38408
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38187 )
Change subject: console/post: Split parts to arch/ ......................................................................
console/post: Split parts to arch/
Both IO port and cmos are currently arch/x86 only features.
Change-Id: I010af3f645c0be38dd856657874c36103aebbdc2 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38187 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/x86/Makefile.inc A src/arch/x86/post.c M src/console/post.c M src/include/console/console.h 4 files changed, 37 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc index 6ed93e5..534f2ce 100644 --- a/src/arch/x86/Makefile.inc +++ b/src/arch/x86/Makefile.inc @@ -99,6 +99,7 @@ all-y += memcpy.c all-y += memset.c all-y += cpu_common.c +all-y += post.c
endif
diff --git a/src/arch/x86/post.c b/src/arch/x86/post.c new file mode 100644 index 0000000..0a20bab --- /dev/null +++ b/src/arch/x86/post.c @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * 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 <stdint.h> +#include <console/console.h> +#include <arch/io.h> + +void arch_post_code(uint8_t value) +{ + if (CONFIG(POST_IO)) + outb(value, CONFIG_POST_IO_PORT); + + if (CONFIG(CMOS_POST)) + cmos_post_code(value); +} diff --git a/src/console/post.c b/src/console/post.c index b9a7557..33d85e7 100644 --- a/src/console/post.c +++ b/src/console/post.c @@ -14,11 +14,9 @@
#include <stdint.h> #include <console/console.h> -#if CONFIG(POST_IO) -#include <arch/io.h> -#endif
/* Write POST information */ +void __weak arch_post_code(uint8_t value) { }
/* Some mainboards have very nice features beyond just a simple display. * They can override this function. @@ -27,16 +25,13 @@
void post_code(uint8_t value) { -#if !CONFIG(NO_POST) -#if CONFIG(CONSOLE_POST) - printk(BIOS_EMERG, "POST: 0x%02x\n", value); -#endif -#if CONFIG(CMOS_POST) - cmos_post_code(value); -#endif -#if CONFIG(POST_IO) - outb(value, CONFIG_POST_IO_PORT); -#endif -#endif + if (!CONFIG(NO_POST)) { + /* Assume this to be the most reliable and simplest type + for displaying POST so keep it first. */ + arch_post_code(value); + + if (CONFIG(CONSOLE_POST)) + printk(BIOS_EMERG, "POST: 0x%02x\n", value); + } mainboard_post(value); } diff --git a/src/include/console/console.h b/src/include/console/console.h index d3c1d54..c573123 100644 --- a/src/include/console/console.h +++ b/src/include/console/console.h @@ -29,6 +29,7 @@ #include <console/vtxprintf.h>
void post_code(u8 value); +void arch_post_code(u8 value); void cmos_post_code(u8 value); #if CONFIG(CMOS_POST_EXTRA) void post_log_extra(u32 value);