Jeremy Soller has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
ec/system76: Add console support
This adds support for line-buffered console output to System76 EC firmware.
Tested on system76/lemp9 with CONSOLE_SYSTEM76_EC enabled.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I861bf3e22f40dd6c3ec7ba1d73711b399358e332 --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 5 files changed, 102 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/43718/1
diff --git a/src/console/Kconfig b/src/console/Kconfig index 7c6e9bc..bad6c56 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -302,6 +302,13 @@ This is currently working only in ramstage due to how the spi drivers are written.
+config CONSOLE_SYSTEM76_EC + bool "System76 EC console output" + default n + depends on EC_SYSTEM76_EC + help + Send coreboot debug output to a System76 embedded controller. + config CONSOLE_OVERRIDE_LOGLEVEL bool help diff --git a/src/console/console.c b/src/console/console.c index bc9d918..2f544a8 100644 --- a/src/console/console.c +++ b/src/console/console.c @@ -9,6 +9,7 @@ #include <console/usb.h> #include <console/spi.h> #include <console/flash.h> +#include <console/system76_ec.h>
void console_hw_init(void) { @@ -21,6 +22,7 @@ __usbdebug_init(); __spiconsole_init(); __flashconsole_init(); + __system76_ec_init(); }
void console_tx_byte(unsigned char byte) @@ -42,6 +44,7 @@ __usb_tx_byte(byte); __spiconsole_tx_byte(byte); __flashconsole_tx_byte(byte); + __system76_ec_tx_byte(byte); }
void console_tx_flush(void) @@ -50,6 +53,7 @@ __ne2k_tx_flush(); __usb_tx_flush(); __flashconsole_tx_flush(); + __system76_ec_tx_flush(); }
void console_write_line(uint8_t *buffer, size_t number_of_bytes) diff --git a/src/ec/system76/ec/Makefile.inc b/src/ec/system76/ec/Makefile.inc new file mode 100644 index 0000000..382daa6 --- /dev/null +++ b/src/ec/system76/ec/Makefile.inc @@ -0,0 +1,6 @@ +ifeq ($(CONFIG_EC_SYSTEM76_EC),y) + +all-y += system76_ec.c +smm-$(CONFIG_DEBUG_SMI) += system76_ec.c + +endif diff --git a/src/ec/system76/ec/system76_ec.c b/src/ec/system76/ec/system76_ec.c new file mode 100644 index 0000000..238e630 --- /dev/null +++ b/src/ec/system76/ec/system76_ec.c @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/io.h> +#include <console/system76_ec.h> +#include <timer.h> + +#define SYSTEM76_EC_BASE 0x0E00 + +static inline uint8_t system76_ec_read(uint8_t addr) +{ + return inb(SYSTEM76_EC_BASE + (uint16_t)addr); +} + +static inline void system76_ec_write(uint8_t addr, uint8_t data) +{ + outb(data, SYSTEM76_EC_BASE + (uint16_t)addr); +} + +void system76_ec_init(void) +{ + // Clear entire command region + for (int i = 0; i < 256; i++) + system76_ec_write((uint8_t)i, 0); +} + +void system76_ec_flush(void) +{ + // Send command + system76_ec_write(0, 4); + + // Wait for command completion, for up to 10 milliseconds + wait_us(10000, system76_ec_read(0) == 0); + + // Clear length + system76_ec_write(3, 0); +} + +void system76_ec_print(uint8_t byte) +{ + // Read length + uint8_t len = system76_ec_read(3); + // Write data at offset + system76_ec_write(len + 4, byte); + // Update length + system76_ec_write(3, len + 1); + + // If we hit the end of the buffer, or were given a newline, flush + if (byte == '\n' || len >= 128) + system76_ec_flush(); +} diff --git a/src/include/console/system76_ec.h b/src/include/console/system76_ec.h new file mode 100644 index 0000000..616e46f --- /dev/null +++ b/src/include/console/system76_ec.h @@ -0,0 +1,35 @@ +#ifndef CONSOLE_SYSTEM76_EC_H +#define CONSOLE_SYSTEM76_EC_H 1 + +#include <stddef.h> +#include <stdint.h> + +void system76_ec_init(void); +void system76_ec_flush(void); +void system76_ec_print(uint8_t byte); + +#define __CONSOLE_SYSTEM76_EC_ENABLE__ (CONFIG(CONSOLE_SYSTEM76_EC) && \ + (ENV_BOOTBLOCK || ENV_ROMSTAGE || ENV_RAMSTAGE \ + || ENV_SEPARATE_VERSTAGE || ENV_POSTCAR \ + || (ENV_SMM && CONFIG(DEBUG_SMI)))) + +#if __CONSOLE_SYSTEM76_EC_ENABLE__ +static inline void __system76_ec_init(void) +{ + system76_ec_init(); +} +static inline void __system76_ec_tx_flush(void) +{ + system76_ec_flush(); +} +static inline void __system76_ec_tx_byte(unsigned char byte) +{ + system76_ec_print(byte); +} +#else +static inline void __system76_ec_init(void) {} +static inline void __system76_ec_tx_flush(void) {} +static inline void __system76_ec_tx_byte(unsigned char byte) {} +#endif + +#endif
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43718
to look at the new patch set (#2).
Change subject: ec/system76: Add console support ......................................................................
ec/system76: Add console support
This adds support for line-buffered console output to System76 EC firmware.
Tested on system76/lemp9 with CONSOLE_SYSTEM76_EC enabled.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I861bf3e22f40dd6c3ec7ba1d73711b399358e332 --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 5 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/43718/2
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG@9 PS2, Line 9: This adds support for line-buffered console output to System76 EC firmware. How does this console work? That is, how would one grab logs from the System76 Ec?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG@9 PS2, Line 9: This adds support for line-buffered console output to System76 EC firmware.
How does this console work? That is, how would one grab logs from the System76 Ec?
Once the print command is received, the EC firmware multiplexes the output to any enabled console on the EC. This can be a memory ringbuffer, a parallel port (using the keyboard connector), or i2c (using the battery connector). Once the entire buffer is sent, it sets the command register to 0, indicating completion.
Those methods can be seen here: https://github.com/system76/ec/blob/master/src/board/system76/common/stdio.c...
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG@9 PS2, Line 9: This adds support for line-buffered console output to System76 EC firmware.
Once the print command is received, the EC firmware multiplexes the output to any enabled console on […]
I see. It would be good to mention this in the commit message. I was wondering why the generic 8250io driver couldn't be used for this EC.
Hello Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43718
to look at the new patch set (#3).
Change subject: ec/system76: Add console support ......................................................................
ec/system76: Add console support
This adds support for line-buffered console output to System76 EC firmware.
Once the print command is received, the EC firmware multiplexes the output to any enabled console on the EC. This can be a memory ringbuffer, a parallel port (using the keyboard connector), or i2c (using the battery connector). Once the entire buffer is sent, it sets the command register to 0, indicating completion.
Tested on system76/lemp9 with CONSOLE_SYSTEM76_EC enabled.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I861bf3e22f40dd6c3ec7ba1d73711b399358e332 --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 5 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/43718/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43718/2//COMMIT_MSG@9 PS2, Line 9: This adds support for line-buffered console output to System76 EC firmware.
I see. It would be good to mention this in the commit message. […]
Done, thank you!
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 3: Code-Review+1
Cool! Is there any documentation on how to use this? Like how to configure, compile and flash the EC firmware, how to wire things correctly, ..
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 3: Code-Review+1
Patch Set 3: Code-Review+1
Cool! Is there any documentation on how to use this? Like how to configure, compile and flash the EC firmware, how to wire things correctly, ..
Well, flashing is documented https://github.com/system76/ec
But indeed, documentation on that console feature would be good to have
Nice work!
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/3/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43718/3/src/ec/system76/ec/system76... PS3, Line 7: // This is the command region for System76 EC firmware. It must be : // enabled for LPC in the mainboard. : use /* comment style? (it's not a *must* according to coreboot style guide but it's what is normally used)
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 3:
Cool! Is there any documentation on how to use this? Like how to configure, compile and flash the EC firmware, how to wire things correctly, ..
Just found the documentation here: https://github.com/system76/ec/blob/master/doc/debugging.md
Please put the link in the commit message.
Hello Felix Singer, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons, Michael Niewöhner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43718
to look at the new patch set (#4).
Change subject: ec/system76: Add console support ......................................................................
ec/system76: Add console support
This adds support for line-buffered console output to System76 EC firmware.
Once the print command is received, the EC firmware multiplexes the output to any enabled console on the EC. This can be a memory ringbuffer, a parallel port (using the keyboard connector), or i2c (using the battery connector). Once the entire buffer is sent, it sets the command register to 0, indicating completion. For more information, please see: https://github.com/system76/ec/blob/master/doc/debugging.md
Tested on system76/lemp9 with CONSOLE_SYSTEM76_EC enabled.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I861bf3e22f40dd6c3ec7ba1d73711b399358e332 --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 5 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/43718/4
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 4:
Patch Set 3:
Cool! Is there any documentation on how to use this? Like how to configure, compile and flash the EC firmware, how to wire things correctly, ..
Just found the documentation here: https://github.com/system76/ec/blob/master/doc/debugging.md
Please put the link in the commit message.
Done, in patch set 4
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 4: Code-Review+1
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 4: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43718/3/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43718/3/src/ec/system76/ec/system76... PS3, Line 7: // This is the command region for System76 EC firmware. It must be : // enabled for LPC in the mainboard. :
use /* comment style? (it's not a *must* according to coreboot style guide but it's what is normally […]
Done
Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43718 )
Change subject: ec/system76: Add console support ......................................................................
ec/system76: Add console support
This adds support for line-buffered console output to System76 EC firmware.
Once the print command is received, the EC firmware multiplexes the output to any enabled console on the EC. This can be a memory ringbuffer, a parallel port (using the keyboard connector), or i2c (using the battery connector). Once the entire buffer is sent, it sets the command register to 0, indicating completion. For more information, please see: https://github.com/system76/ec/blob/master/doc/debugging.md
Tested on system76/lemp9 with CONSOLE_SYSTEM76_EC enabled.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I861bf3e22f40dd6c3ec7ba1d73711b399358e332 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43718 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 5 files changed, 113 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/console/Kconfig b/src/console/Kconfig index 7c6e9bc..bad6c56 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -302,6 +302,13 @@ This is currently working only in ramstage due to how the spi drivers are written.
+config CONSOLE_SYSTEM76_EC + bool "System76 EC console output" + default n + depends on EC_SYSTEM76_EC + help + Send coreboot debug output to a System76 embedded controller. + config CONSOLE_OVERRIDE_LOGLEVEL bool help diff --git a/src/console/console.c b/src/console/console.c index bc9d918..2f544a8 100644 --- a/src/console/console.c +++ b/src/console/console.c @@ -9,6 +9,7 @@ #include <console/usb.h> #include <console/spi.h> #include <console/flash.h> +#include <console/system76_ec.h>
void console_hw_init(void) { @@ -21,6 +22,7 @@ __usbdebug_init(); __spiconsole_init(); __flashconsole_init(); + __system76_ec_init(); }
void console_tx_byte(unsigned char byte) @@ -42,6 +44,7 @@ __usb_tx_byte(byte); __spiconsole_tx_byte(byte); __flashconsole_tx_byte(byte); + __system76_ec_tx_byte(byte); }
void console_tx_flush(void) @@ -50,6 +53,7 @@ __ne2k_tx_flush(); __usb_tx_flush(); __flashconsole_tx_flush(); + __system76_ec_tx_flush(); }
void console_write_line(uint8_t *buffer, size_t number_of_bytes) diff --git a/src/ec/system76/ec/Makefile.inc b/src/ec/system76/ec/Makefile.inc new file mode 100644 index 0000000..382daa6 --- /dev/null +++ b/src/ec/system76/ec/Makefile.inc @@ -0,0 +1,6 @@ +ifeq ($(CONFIG_EC_SYSTEM76_EC),y) + +all-y += system76_ec.c +smm-$(CONFIG_DEBUG_SMI) += system76_ec.c + +endif diff --git a/src/ec/system76/ec/system76_ec.c b/src/ec/system76/ec/system76_ec.c new file mode 100644 index 0000000..ddcb602 --- /dev/null +++ b/src/ec/system76/ec/system76_ec.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <arch/io.h> +#include <console/system76_ec.h> +#include <timer.h> + +// This is the command region for System76 EC firmware. It must be +// enabled for LPC in the mainboard. +#define SYSTEM76_EC_BASE 0x0E00 +#define SYSTEM76_EC_SIZE 256 + +#define REG_CMD 0 +#define REG_RESULT 1 + +// When command register is 0, command is complete +#define CMD_FINISHED 0 + +// Print command. Registers are unique for each command +#define CMD_PRINT 4 +#define CMD_PRINT_REG_FLAGS 2 +#define CMD_PRINT_REG_LEN 3 +#define CMD_PRINT_REG_DATA 4 + +static inline uint8_t system76_ec_read(uint8_t addr) +{ + return inb(SYSTEM76_EC_BASE + (uint16_t)addr); +} + +static inline void system76_ec_write(uint8_t addr, uint8_t data) +{ + outb(data, SYSTEM76_EC_BASE + (uint16_t)addr); +} + +void system76_ec_init(void) +{ + // Clear entire command region + for (int i = 0; i < SYSTEM76_EC_SIZE; i++) + system76_ec_write((uint8_t)i, 0); +} + +void system76_ec_flush(void) +{ + system76_ec_write(REG_CMD, CMD_PRINT); + + // Wait for command completion, for up to 10 milliseconds, with a + // test period of 1 microsecond + wait_us(10000, system76_ec_read(REG_CMD) == CMD_FINISHED); + + system76_ec_write(CMD_PRINT_REG_LEN, 0); +} + +void system76_ec_print(uint8_t byte) +{ + uint8_t len = system76_ec_read(CMD_PRINT_REG_LEN); + system76_ec_write(CMD_PRINT_REG_DATA + len, byte); + system76_ec_write(CMD_PRINT_REG_LEN, len + 1); + + // If we hit the end of the buffer, or were given a newline, flush + if (byte == '\n' || len >= (SYSTEM76_EC_SIZE - CMD_PRINT_REG_DATA)) + system76_ec_flush(); +} diff --git a/src/include/console/system76_ec.h b/src/include/console/system76_ec.h new file mode 100644 index 0000000..616e46f --- /dev/null +++ b/src/include/console/system76_ec.h @@ -0,0 +1,35 @@ +#ifndef CONSOLE_SYSTEM76_EC_H +#define CONSOLE_SYSTEM76_EC_H 1 + +#include <stddef.h> +#include <stdint.h> + +void system76_ec_init(void); +void system76_ec_flush(void); +void system76_ec_print(uint8_t byte); + +#define __CONSOLE_SYSTEM76_EC_ENABLE__ (CONFIG(CONSOLE_SYSTEM76_EC) && \ + (ENV_BOOTBLOCK || ENV_ROMSTAGE || ENV_RAMSTAGE \ + || ENV_SEPARATE_VERSTAGE || ENV_POSTCAR \ + || (ENV_SMM && CONFIG(DEBUG_SMI)))) + +#if __CONSOLE_SYSTEM76_EC_ENABLE__ +static inline void __system76_ec_init(void) +{ + system76_ec_init(); +} +static inline void __system76_ec_tx_flush(void) +{ + system76_ec_flush(); +} +static inline void __system76_ec_tx_byte(unsigned char byte) +{ + system76_ec_print(byte); +} +#else +static inline void __system76_ec_init(void) {} +static inline void __system76_ec_tx_flush(void) {} +static inline void __system76_ec_tx_byte(unsigned char byte) {} +#endif + +#endif
-
Angel Pons (Code Review) -
Felix Singer (Code Review)
-
Jeremy Soller (Code Review)
-
Michael Niewöhner (Code Review)
-
Philipp Deppenwiese (Code Review)