Jeremy Soller has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/acpi/ac.asl A src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl A src/ec/system76/ec/acpi/ec.asl A src/ec/system76/ec/acpi/ec_ram.asl A src/ec/system76/ec/acpi/hid.asl A src/ec/system76/ec/acpi/lid.asl A src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 14 files changed, 902 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/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/Kconfig b/src/ec/system76/ec/Kconfig new file mode 100644 index 0000000..448771e --- /dev/null +++ b/src/ec/system76/ec/Kconfig @@ -0,0 +1,4 @@ +config EC_SYSTEM76_EC + bool + help + System76 EC diff --git a/src/ec/system76/ec/Makefile.inc b/src/ec/system76/ec/Makefile.inc new file mode 100644 index 0000000..d6daa7c --- /dev/null +++ b/src/ec/system76/ec/Makefile.inc @@ -0,0 +1,10 @@ +ifeq ($(CONFIG_EC_SYSTEM76_EC),y) + +bootblock-y += system76_ec.c +verstage-y += system76_ec.c +romstage-y += system76_ec.c +postcar-y += system76_ec.c +ramstage-y += system76_ec.c +smm-$(CONFIG_DEBUG_SMI) += system76_ec.c + +endif diff --git a/src/ec/system76/ec/acpi/ac.asl b/src/ec/system76/ec/acpi/ac.asl new file mode 100644 index 0000000..7326676 --- /dev/null +++ b/src/ec/system76/ec/acpi/ac.asl @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (AC) +{ + Name (_HID, "ACPI0003" /* Power Source Device */) // _HID: Hardware ID + Name (_PCL, Package (0x01) // _PCL: Power Consumer List + { + _SB + }) + + Name (ACFG, One) + + Method (_PSR, 0, NotSerialized) // _PSR: Power Source + { + Return (ACFG) + } + + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (0x0F) + } +} diff --git a/src/ec/system76/ec/acpi/battery.asl b/src/ec/system76/ec/acpi/battery.asl new file mode 100644 index 0000000..bc239dd --- /dev/null +++ b/src/ec/system76/ec/acpi/battery.asl @@ -0,0 +1,170 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (BAT0) +{ + Name (_HID, EisaId ("PNP0C0A") /* Control Method Battery */) // _HID: Hardware ID + Name (_UID, Zero) // _UID: Unique ID + Name (_PCL, Package (0x01) // _PCL: Power Consumer List + { + _SB + }) + Name (BFCC, Zero) + Method (_STA, 0, NotSerialized) // _STA: Status + { + If (^^PCI0.LPCB.EC0.ECOK) + { + If (^^PCI0.LPCB.EC0.BAT0) + { + Return (0x1F) + } + Else + { + Return (0x0F) + } + } + Else + { + Return (0x0F) + } + } + + Name (PBIF, Package (0x0D) + { + One, + 0xFFFFFFFF, + 0xFFFFFFFF, + One, + 0x39D0, + Zero, + Zero, + 0x40, + 0x40, + "BAT", + "0001", + "LION", + "Notebook" + }) + Method (IVBI, 0, NotSerialized) + { + PBIF [One] = 0xFFFFFFFF + PBIF [0x02] = 0xFFFFFFFF + PBIF [0x04] = 0xFFFFFFFF + PBIF [0x09] = " " + PBIF [0x0A] = " " + PBIF [0x0B] = " " + PBIF [0x0C] = " " + BFCC = Zero + } + + Method (UPBI, 0, NotSerialized) + { + If (^^PCI0.LPCB.EC0.BAT0) + { + Local0 = (^^PCI0.LPCB.EC0.BDC0 & 0xFFFF) + PBIF [One] = Local0 + Local0 = (^^PCI0.LPCB.EC0.BFC0 & 0xFFFF) + PBIF [0x02] = Local0 + BFCC = Local0 + Local0 = (^^PCI0.LPCB.EC0.BDV0 & 0xFFFF) + PBIF [0x04] = Local0 + Local0 = (^^PCI0.LPCB.EC0.BCW0 & 0xFFFF) + PBIF [0x05] = Local0 + Local0 = (^^PCI0.LPCB.EC0.BCL0 & 0xFFFF) + PBIF [0x06] = Local0 + PBIF [0x09] = "BAT" + PBIF [0x0A] = "0001" + PBIF [0x0B] = "LION" + PBIF [0x0C] = "Notebook" + } + Else + { + IVBI () + } + } + + Method (_BIF, 0, NotSerialized) // _BIF: Battery Information + { + If (^^PCI0.LPCB.EC0.ECOK) + { + UPBI () + } + Else + { + IVBI () + } + + Return (PBIF) /* _SB_.BAT0.PBIF */ + } + + Name (PBST, Package (0x04) + { + Zero, + 0xFFFFFFFF, + 0xFFFFFFFF, + 0x3D90 + }) + Method (IVBS, 0, NotSerialized) + { + PBST [Zero] = Zero + PBST [One] = 0xFFFFFFFF + PBST [0x02] = 0xFFFFFFFF + PBST [0x03] = 0x2710 + } + + Method (UPBS, 0, NotSerialized) + { + If (^^PCI0.LPCB.EC0.BAT0) + { + Local0 = Zero + Local1 = Zero + If (^^AC.ACFG) + { + If (((^^PCI0.LPCB.EC0.BST0 & 0x02) == 0x02)) + { + Local0 |= 0x02 + Local1 = (^^PCI0.LPCB.EC0.BPR0 & 0xFFFF) + } + } + Else + { + Local0 |= One + Local1 = (^^PCI0.LPCB.EC0.BPR0 & 0xFFFF) + } + + Local7 = (Local1 & 0x8000) + If ((Local7 == 0x8000)) + { + Local1 ^= 0xFFFF + } + + Local2 = (^^PCI0.LPCB.EC0.BRC0 & 0xFFFF) + Local3 = (^^PCI0.LPCB.EC0.BPV0 & 0xFFFF) + PBST [Zero] = Local0 + PBST [One] = Local1 + PBST [0x02] = Local2 + PBST [0x03] = Local3 + If ((BFCC != ^^PCI0.LPCB.EC0.BFC0)) + { + Notify (BAT0, 0x81) // Information Change + } + } + Else + { + IVBS () + } + } + + Method (_BST, 0, NotSerialized) // _BST: Battery Status + { + If (^^PCI0.LPCB.EC0.ECOK) + { + UPBS () + } + Else + { + IVBS () + } + + Return (PBST) /* _SB_.BAT0.PBST */ + } +} diff --git a/src/ec/system76/ec/acpi/buttons.asl b/src/ec/system76/ec/acpi/buttons.asl new file mode 100644 index 0000000..ae85c67 --- /dev/null +++ b/src/ec/system76/ec/acpi/buttons.asl @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (PWRB) +{ + Name (_HID, EisaId ("PNP0C0C")) + Name (_PRW, Package () { EC_GPE_SWI, 3 }) +} + +Device (SLPB) +{ + Name (_HID, EisaId ("PNP0C0E")) + Name (_PRW, Package () { EC_GPE_SWI, 3 }) +} diff --git a/src/ec/system76/ec/acpi/ec.asl b/src/ec/system76/ec/acpi/ec.asl new file mode 100644 index 0000000..5989f6d --- /dev/null +++ b/src/ec/system76/ec/acpi/ec.asl @@ -0,0 +1,230 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope (_SB) { + #include "ac.asl" + #include "battery.asl" + #include "buttons.asl" + #include "hid.asl" + #include "lid.asl" + #include "s76.asl" +} + +Device (_SB.PCI0.LPCB.EC0) +{ + Name (_HID, EisaId ("PNP0C09") /* Embedded Controller Device */) // _HID: Hardware ID + Name (_GPE, EC_GPE_SCI) // _GPE: General Purpose Events + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + IO (Decode16, + 0x0062, // Range Minimum + 0x0062, // Range Maximum + 0x00, // Alignment + 0x01, // Length + ) + IO (Decode16, + 0x0066, // Range Minimum + 0x0066, // Range Maximum + 0x00, // Alignment + 0x01, // Length + ) + }) + + #include "ec_ram.asl" + + Name (ECOK, Zero) + Method (_REG, 2, Serialized) // _REG: Region Availability + { + Debug = Concatenate("EC: _REG", Concatenate(ToHexString(Arg0), Concatenate(" ", ToHexString(Arg1)))) + If (((Arg0 == 0x03) && (Arg1 == One))) { + // Enable hardware touchpad lock, airplane mode, and keyboard backlight keys + ECOS = 1 + + // Enable software display brightness keys + WINF = 1 + + // Set current AC state + ^^^^AC.ACFG = ADP + // Update battery information and status + ^^^^BAT0.UPBI() + ^^^^BAT0.UPBS() + + // Notify of changes + Notify(^^^^AC, Zero) + Notify(^^^^BAT0, Zero) + + PNOT () + + // EC is now available + ECOK = Arg1 + + // Reset System76 Device + ^^^^S76D.RSET() + } + } + + Method (PTS, 1, Serialized) { + Debug = Concatenate("EC: PTS: ", ToHexString(Arg0)) + If (ECOK) { + // Clear wake cause + WFNO = Zero + } + } + + Method (WAK, 1, Serialized) { + Debug = Concatenate("EC: WAK: ", ToHexString(Arg0)) + If (ECOK) { + // Set current AC state + ^^^^AC.ACFG = ADP + + // Update battery information and status + ^^^^BAT0.UPBI() + ^^^^BAT0.UPBS() + + // Notify of changes + Notify(^^^^AC, Zero) + Notify(^^^^BAT0, Zero) + + // Reset System76 Device + ^^^^S76D.RSET() + } + } + + Method (_Q0A, 0, NotSerialized) // Touchpad Toggle + { + Debug = "EC: Touchpad Toggle" + } + + Method (_Q0B, 0, NotSerialized) // Screen Toggle + { + Debug = "EC: Screen Toggle" + } + + Method (_Q0C, 0, NotSerialized) // Mute + { + Debug = "EC: Mute" + } + + Method (_Q0D, 0, NotSerialized) // Keyboard Backlight + { + Debug = "EC: Keyboard Backlight" + } + + Method (_Q0E, 0, NotSerialized) // Volume Down + { + Debug = "EC: Volume Down" + } + + Method (_Q0F, 0, NotSerialized) // Volume Up + { + Debug = "EC: Volume Up" + } + + Method (_Q10, 0, NotSerialized) // Switch Video Mode + { + Debug = "EC: Switch Video Mode" + } + + Method (_Q11, 0, NotSerialized) // Brightness Down + { + Debug = "EC: Brightness Down" + if (^^^^HIDD.HRDY) { + ^^^^HIDD.HPEM (20) + } + } + + Method (_Q12, 0, NotSerialized) // Brightness Up + { + Debug = "EC: Brightness Up" + if (^^^^HIDD.HRDY) { + ^^^^HIDD.HPEM (19) + } + } + + Method (_Q13, 0, NotSerialized) // Camera Toggle + { + Debug = "EC: Camera Toggle" + } + + Method (_Q14, 0, NotSerialized) // Airplane Mode + { + Debug = "EC: Airplane Mode" + if (^^^^HIDD.HRDY) { + ^^^^HIDD.HPEM (8) + } + // TODO: hardware airplane mode + } + + Method (_Q15, 0, NotSerialized) // Suspend Button + { + Debug = "EC: Suspend Button" + Notify (SLPB, 0x80) + } + + Method (_Q16, 0, NotSerialized) // AC Detect + { + Debug = "EC: AC Detect" + ^^^^AC.ACFG = ADP + Notify (AC, 0x80) // Status Change + If (BAT0) + { + Notify (^^^^BAT0, 0x81) // Information Change + Notify (^^^^BAT0, 0x80) // Status Change + } + } + + Method (_Q17, 0, NotSerialized) // BAT0 Update + { + Debug = "EC: BAT0 Update (17)" + Notify (^^^^BAT0, 0x81) // Information Change + } + + Method (_Q19, 0, NotSerialized) // BAT0 Update + { + Debug = "EC: BAT0 Update (19)" + Notify (^^^^BAT0, 0x81) // Information Change + } + + Method (_Q1B, 0, NotSerialized) // Lid Close + { + Debug = "EC: Lid Close" + Notify (LID0, 0x80) + } + + Method (_Q1C, 0, NotSerialized) // Thermal Trip + { + Debug = "EC: Thermal Trip" + /* TODO + Notify (_TZ.TZ0, 0x81) // Thermal Trip Point Change + Notify (_TZ.TZ0, 0x80) // Thermal Status Change + */ + } + + Method (_Q1D, 0, NotSerialized) // Power Button + { + Debug = "EC: Power Button" + Notify (PWRB, 0x80) + } + + Method (_Q50, 0, NotSerialized) // Other Events + { + Local0 = OEM4 + If (Local0 == 0x8A) { + Debug = "EC: White Keyboard Backlight" + Notify (^^^^S76D, 0x80) + } ElseIf (Local0 == 0x9F) { + Debug = "EC: Color Keyboard Toggle" + Notify (^^^^S76D, 0x81) + } ElseIf (Local0 == 0x81) { + Debug = "EC: Color Keyboard Down" + Notify (^^^^S76D, 0x82) + } ElseIf (Local0 == 0x82) { + Debug = "EC: Color Keyboard Up" + Notify (^^^^S76D, 0x83) + } ElseIf (Local0 == 0x80) { + Debug = "EC: Color Keyboard Color Change" + Notify (^^^^S76D, 0x84) + } Else { + Debug = Concatenate("EC: Other: ", ToHexString(Local0)) + } + } +} diff --git a/src/ec/system76/ec/acpi/ec_ram.asl b/src/ec/system76/ec/acpi/ec_ram.asl new file mode 100644 index 0000000..d5f97ed --- /dev/null +++ b/src/ec/system76/ec/acpi/ec_ram.asl @@ -0,0 +1,175 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +OperationRegion (ERAM, EmbeddedControl, Zero, 0xFF) +Field (ERAM, ByteAcc, Lock, Preserve) +{ + NMSG, 8, + SLED, 4, + Offset (0x02), + MODE, 1, + FAN0, 1, + TME0, 1, + TME1, 1, + FAN1, 1, + , 2, + Offset (0x03), + LSTE, 1, + LSW0, 1, + LWKE, 1, + WAKF, 1, + , 2, + PWKE, 1, + MWKE, 1, + AC0, 8, + PSV, 8, + CRT, 8, + TMP, 8, + AC1, 8, + BBST, 8, + Offset (0x0B), + Offset (0x0C), + Offset (0x0D), + Offset (0x0E), + SLPT, 8, + SWEJ, 1, + SWCH, 1, + Offset (0x10), + ADP, 1, + AFLT, 1, + BAT0, 1, + BAT1, 1, + , 3, + PWOF, 1, + WFNO, 8, + BPU0, 32, + BDC0, 32, + BFC0, 32, + BTC0, 32, + BDV0, 32, + BST0, 32, + BPR0, 32, + BRC0, 32, + BPV0, 32, + BTP0, 16, + BRS0, 16, + BCW0, 32, + BCL0, 32, + BCG0, 32, + BG20, 32, + BMO0, 64, + BIF0, 64, + BSN0, 32, + BTY0, 64, + Offset (0x67), + Offset (0x68), + ECOS, 8, + LNXD, 8, + ECPS, 8, + Offset (0x6C), + BTMP, 16, + EVTN, 8, + Offset (0x72), + PRCL, 8, + PRC0, 8, + PRC1, 8, + PRCM, 8, + PRIN, 8, + PSTE, 8, + PCAD, 8, + PEWL, 8, + PWRL, 8, + PECD, 8, + PEHI, 8, + PECI, 8, + PEPL, 8, + PEPM, 8, + PWFC, 8, + PECC, 8, + PDT0, 8, + PDT1, 8, + PDT2, 8, + PDT3, 8, + PRFC, 8, + PRS0, 8, + PRS1, 8, + PRS2, 8, + PRS3, 8, + PRS4, 8, + PRCS, 8, + PEC0, 8, + PEC1, 8, + PEC2, 8, + PEC3, 8, + CMDR, 8, + CVRT, 8, + GTVR, 8, + FANT, 8, + SKNT, 8, + AMBT, 8, + MCRT, 8, + DIM0, 8, + DIM1, 8, + PMAX, 8, + PPDT, 8, + PECH, 8, + PMDT, 8, + TSD0, 8, + TSD1, 8, + TSD2, 8, + TSD3, 8, + CPUP, 16, + MCHP, 16, + SYSP, 16, + CPAP, 16, + MCAP, 16, + SYAP, 16, + CFSP, 16, + CPUE, 16, + Offset (0xC6), + Offset (0xC7), + VGAT, 8, + OEM1, 8, + OEM2, 8, + OEM3, 16, + OEM4, 8, + Offset (0xCE), + DUT1, 8, + DUT2, 8, + RPM1, 16, + RPM2, 16, + RPM4, 16, + Offset (0xD7), + DTHL, 8, + DTBP, 8, + AIRP, 8, + WINF, 8, + RINF, 8, + Offset (0xDD), + INF2, 8, + MUTE, 1, + Offset (0xE0), + RPM3, 16, + ECKS, 8, + Offset (0xE4), + , 4, + XTUF, 1, + EP12, 1, + Offset (0xE5), + INF3, 8, + Offset (0xE7), + GFOF, 8, + Offset (0xE9), + KPCR, 1, + Offset (0xEA), + Offset (0xF0), + PL1T, 16, + PL2T, 16, + TAUT, 8, + Offset (0xF8), + FCMD, 8, + FDAT, 8, + FBUF, 8, + FBF1, 8, + FBF2, 8, + FBF3, 8 +} diff --git a/src/ec/system76/ec/acpi/hid.asl b/src/ec/system76/ec/acpi/hid.asl new file mode 100644 index 0000000..ce8fd3f --- /dev/null +++ b/src/ec/system76/ec/acpi/hid.asl @@ -0,0 +1,50 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (HIDD) +{ + Name (_HID, "INT33D5") + Name (HBSY, Zero) + Name (HIDX, Zero) + Name (HRDY, Zero) + + Method (HDEM, 0, Serialized) + { + HBSY = Zero + Return (HIDX) + } + + Method (HDMM, 0, Serialized) + { + Return (Zero) + } + + Method (HDSM, 1, Serialized) + { + HRDY = Arg0 + } + + Method (HPEM, 1, Serialized) + { + HBSY = One + HIDX = Arg0 + + Notify (HIDD, 0xC0) + Local0 = Zero + While (((Local0 < 0xFA) && HBSY)) + { + Sleep (0x04) + Local0++ + } + + If ((HBSY == One)) + { + HBSY = Zero + HIDX = Zero + Return (One) + } + Else + { + Return (Zero) + } + } +} diff --git a/src/ec/system76/ec/acpi/lid.asl b/src/ec/system76/ec/acpi/lid.asl new file mode 100644 index 0000000..45e646a --- /dev/null +++ b/src/ec/system76/ec/acpi/lid.asl @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (LID0) +{ + Name (_HID, EisaId ("PNP0C0D")) + Name (_PRW, Package () { EC_GPE_SWI, 3 }) + + Method (_LID, 0, NotSerialized) { + DEBUG = "LID: _LID" + If (^^PCI0.LPCB.EC0.ECOK) { + Return (^^PCI0.LPCB.EC0.LSTE) + } Else { + Return (One) + } + } + + Method (_PSW, 1, NotSerialized) { + DEBUG = Concatenate("LID: _PSW: ", ToHexString(Arg0)) + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.LWKE = Arg0 + } + } +} diff --git a/src/ec/system76/ec/acpi/s76.asl b/src/ec/system76/ec/acpi/s76.asl new file mode 100644 index 0000000..b65f3ed --- /dev/null +++ b/src/ec/system76/ec/acpi/s76.asl @@ -0,0 +1,114 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +// Notifications: +// 0x80 - hardware backlight toggle +// 0x81 - backlight toggle +// 0x82 - backlight down +// 0x83 - backlight up +// 0x84 - backlight color change +Device (S76D) { + Name (_HID, "17761776") + Name (_UID, 0) + + Method (RSET, 0, Serialized) { + Debug = "S76D: RSET" + SAPL(0) + SKBL(0) + #if EC_COLOR_KEYBOARD + SKBC(0xFFFFFF) + #endif + } + + Method (INIT, 0, Serialized) { + Debug = "S76D: INIT" + RSET() + If (^^PCI0.LPCB.EC0.ECOK) { + // Set flags to use software control + ^^PCI0.LPCB.EC0.ECOS = 2 + Return (0) + } Else { + Return (1) + } + } + + Method (FINI, 0, Serialized) { + Debug = "S76D: FINI" + RSET() + If (^^PCI0.LPCB.EC0.ECOK) { + // Set flags to use hardware control + ^^PCI0.LPCB.EC0.ECOS = 1 + Return (0) + } Else { + Return (1) + } + } + + // Get Airplane LED + Method (GAPL, 0, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + If (^^PCI0.LPCB.EC0.AIRP & 0x40) { + Return (1) + } + } + Return (0) + } + + // Set Airplane LED + Method (SAPL, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + If (Arg0) { + ^^PCI0.LPCB.EC0.AIRP |= 0x40 + } Else { + ^^PCI0.LPCB.EC0.AIRP &= 0xBF + } + } + } + +#if EC_COLOR_KEYBOARD + // Set KB LED Brightness + Method (SKBL, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = 6 + ^^PCI0.LPCB.EC0.FBUF = Arg0 + ^^PCI0.LPCB.EC0.FBF1 = 0 + ^^PCI0.LPCB.EC0.FBF2 = Arg0 + ^^PCI0.LPCB.EC0.FCMD = 0xCA + } + } + + // Set Keyboard Color + Method (SKBC, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = 0x3 + ^^PCI0.LPCB.EC0.FBUF = (Arg0 & 0xFF) + ^^PCI0.LPCB.EC0.FBF1 = ((Arg0 >> 16) & 0xFF) + ^^PCI0.LPCB.EC0.FBF2 = ((Arg0 >> 8) & 0xFF) + ^^PCI0.LPCB.EC0.FCMD = 0xCA + Return (Arg0) + } Else { + Return (0) + } + } +#else + // Get KB LED + Method (GKBL, 0, Serialized) { + Local0 = 0 + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = One + ^^PCI0.LPCB.EC0.FCMD = 0xCA + Local0 = ^^PCI0.LPCB.EC0.FBUF + ^^PCI0.LPCB.EC0.FCMD = Zero + } + Return (Local0) + } + + // Set KB Led + Method (SKBL, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = Zero + ^^PCI0.LPCB.EC0.FBUF = Arg0 + ^^PCI0.LPCB.EC0.FCMD = 0xCA + } + } +#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..3bf83a7 --- /dev/null +++ b/src/ec/system76/ec/system76_ec.c @@ -0,0 +1,49 @@ +#include <arch/io.h> +#include <console/system76_ec.h> +#include <delay.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 + int timeout; + for (timeout = 10000; timeout > 0; timeout--) { + if (system76_ec_read(0) == 0) break; + udelay(1); + } + + // 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..8343788 --- /dev/null +++ b/src/include/console/system76_ec.h @@ -0,0 +1,31 @@ +#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/+/43612
to look at the new patch set (#2).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC.
Tested on system76/lemp9. An associated patch will be sent that converts system76/lemp9 over to EC_SYSTEM76_EC.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/acpi/ac.asl A src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl A src/ec/system76/ec/acpi/ec.asl A src/ec/system76/ec/acpi/ec_ram.asl A src/ec/system76/ec/acpi/hid.asl A src/ec/system76/ec/acpi/lid.asl A src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 14 files changed, 902 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG@11 PS2, Line 11: Tested on system76/lemp9. An associated patch will be sent that converts : system76/lemp9 over to EC_SYSTEM76_EC. If the conversion was done at the same time as this patch, the diffstat should be smaller. AFAIUI the ACPI code is just being moved around.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG@11 PS2, Line 11: Tested on system76/lemp9. An associated patch will be sent that converts : system76/lemp9 over to EC_SYSTEM76_EC.
If the conversion was done at the same time as this patch, the diffstat should be smaller. […]
The mainboard change is here: https://review.coreboot.org/c/coreboot/+/43613
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 3:
(35 comments)
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 7: static inline uint8_t system76_ec_read(uint8_t addr) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 8: return inb(SYSTEM76_EC_BASE + (uint16_t)addr); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 11: static inline void system76_ec_write(uint8_t addr, uint8_t data) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 12: outb(data, SYSTEM76_EC_BASE + (uint16_t)addr); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 15: void system76_ec_init(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 17: for (int i = 0; i < 256; i++) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 17: for (int i = 0; i < 256; i++) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 18: system76_ec_write((uint8_t)i, 0); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 18: system76_ec_write((uint8_t)i, 0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 19: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 22: void system76_ec_flush(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 24: system76_ec_write(0, 4); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 27: int timeout; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 28: for (timeout = 10000; timeout > 0; timeout--) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 29: if (system76_ec_read(0) == 0) break; trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 30: udelay(1); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 30: udelay(1); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 31: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 34: system76_ec_write(3, 0); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 37: void system76_ec_print(uint8_t byte) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 39: uint8_t len = system76_ec_read(3); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 41: system76_ec_write(len + 4, byte); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 43: system76_ec_write(3, len + 1); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 46: if (byte == '\n' || len >= 128) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 46: if (byte == '\n' || len >= 128) { braces {} are not necessary for single statement blocks
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 47: system76_ec_flush(); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 47: system76_ec_flush(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/ec/system76/ec/system76... PS3, Line 48: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... File src/include/console/system76_ec.h:
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 16: static inline void __system76_ec_init(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 17: system76_ec_init(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 19: static inline void __system76_ec_tx_flush(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 20: system76_ec_flush(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/3/src/include/console/system7... PS3, Line 22: static inline void __system76_ec_tx_byte(unsigned char byte) { open brace '{' following function definitions go on the next line
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43612
to look at the new patch set (#4).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC.
Tested on system76/lemp9. An associated patch will be sent that converts system76/lemp9 over to EC_SYSTEM76_EC.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/acpi/ac.asl A src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl A src/ec/system76/ec/acpi/ec.asl A src/ec/system76/ec/acpi/ec_ram.asl A src/ec/system76/ec/acpi/hid.asl A src/ec/system76/ec/acpi/lid.asl A src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 14 files changed, 912 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 4:
(9 comments)
https://review.coreboot.org/c/coreboot/+/43612/4/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/4/src/ec/system76/ec/system76... PS4, Line 22: for (int i = 0; i < 256; i++) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/43612/4/src/ec/system76/ec/system76... PS4, Line 35: for (timeout = 10000; timeout > 0; timeout--) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/43612/4/src/ec/system76/ec/system76... PS4, Line 37: if (system76_ec_read(0) == 0) break; trailing statements should be on next line
https://review.coreboot.org/c/coreboot/+/43612/4/src/ec/system76/ec/system76... PS4, Line 55: if (byte == '\n' || len >= 128) that open brace { should be on the previous line
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... File src/include/console/system76_ec.h:
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... PS4, Line 16: static inline void __system76_ec_init(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... PS4, Line 17: system76_ec_init(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... PS4, Line 19: static inline void __system76_ec_tx_flush(void) { open brace '{' following function definitions go on the next line
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... PS4, Line 20: system76_ec_flush(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/43612/4/src/include/console/system7... PS4, Line 22: static inline void __system76_ec_tx_byte(unsigned char byte) { open brace '{' following function definitions go on the next line
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43612
to look at the new patch set (#5).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC.
Tested on system76/lemp9. An associated patch will be sent that converts system76/lemp9 over to EC_SYSTEM76_EC.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc A src/ec/system76/ec/acpi/ac.asl A src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl A src/ec/system76/ec/acpi/ec.asl A src/ec/system76/ec/acpi/ec_ram.asl A src/ec/system76/ec/acpi/hid.asl A src/ec/system76/ec/acpi/lid.asl A src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h 14 files changed, 912 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43612
to look at the new patch set (#6).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 195 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/6
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43612
to look at the new patch set (#7).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 195 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/7
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 7: Code-Review+1
(7 comments)
Muuuch better!
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/2//COMMIT_MSG@11 PS2, Line 11: Tested on system76/lemp9. An associated patch will be sent that converts : system76/lemp9 over to EC_SYSTEM76_EC.
The mainboard change is here: https://review.coreboot. […]
Ah yes, the diffstat has definitely improved 😄
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... File src/ec/system76/ec/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... PS7, Line 3: bootblock-y += system76_ec.c I'd use all-y
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... File src/ec/system76/ec/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... PS7, Line 3: Scope (_SB) { nit: including these files at such a high scope is probably the reason why you need so many ^^^^ in ACPI
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... File src/ec/system76/ec/acpi/s76.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 17: #if EC_COLOR_KEYBOARD I'd keep preprocessor unindented
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 67: #if EC_COLOR_KEYBOARD I'd make this a Kconfig option
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... PS7, Line 33: for (timeout = 10000; timeout > 0; timeout--) { I'd use the timer API in <timer.h>
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... PS7, Line 149: # LPC configuration from lspci -s 1f.0 -xxx Are these changes part of the EC code?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 7:
(6 comments)
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... File src/ec/system76/ec/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... PS7, Line 3: bootblock-y += system76_ec.c
I'd use all-y
Ah, didn't know about that, thanks
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... File src/ec/system76/ec/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... PS7, Line 3: Scope (_SB) {
nit: including these files at such a high scope is probably the reason why you need so many ^^^^ in […]
True. Perhaps something I can address in another change, since the old lemp9 ACPI code did this
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... File src/ec/system76/ec/acpi/s76.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 17: #if EC_COLOR_KEYBOARD
I'd keep preprocessor unindented
Ack
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 67: #if EC_COLOR_KEYBOARD
I'd make this a Kconfig option
Ack
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... PS7, Line 33: for (timeout = 10000; timeout > 0; timeout--) {
I'd use the timer API in <timer. […]
I'll take a look
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... PS7, Line 149: # LPC configuration from lspci -s 1f.0 -xxx
Are these changes part of the EC code?
Yes. Without them the regions used by the EC will not be correctly set.
0xE00 for example is used for the console code above
Hello 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/+/43612
to look at the new patch set (#8).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 190 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/8
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... PS8, Line 32: wait_us(10000, system76_ec_read(0) == 0); please, no spaces at the start of a line
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 8:
(9 comments)
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig File src/ec/system76/ec/Kconfig:
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig@... PS8, Line 7: bool This would need to "depends on EC_SYSTEM76_EC"
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig@... PS8, Line 8: default n indent should use tabs
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... File src/ec/system76/ec/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/Makefile... PS7, Line 3: bootblock-y += system76_ec.c
Ah, didn't know about that, thanks
Done. I think `all` also includes SMM, but not a big deal. In any case, the unreferenced stuff should get culled by the compiler's optimizing magic.
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... File src/ec/system76/ec/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/ec.... PS7, Line 3: Scope (_SB) {
True. […]
Yes, I think it would be easier to handle it in a separate change.
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... File src/ec/system76/ec/acpi/s76.asl:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 17: #if EC_COLOR_KEYBOARD
Ack
Done
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/acpi/s76... PS7, Line 67: #if EC_COLOR_KEYBOARD
Ack
Done
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/7/src/ec/system76/ec/system76... PS7, Line 33: for (timeout = 10000; timeout > 0; timeout--) {
I'll take a look
Oh, nice one. There's `wait_ms` if you want to use that
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... PS8, Line 32: wait_us(10000, system76_ec_read(0) == 0); Tabs, tabs, tabs 😋
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/43612/7/src/mainboard/system76/lemp... PS7, Line 149: # LPC configuration from lspci -s 1f.0 -xxx
Yes. Without them the regions used by the EC will not be correctly set. […]
Ack
Hello 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/+/43612
to look at the new patch set (#9).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 190 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/9
Hello 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/+/43612
to look at the new patch set (#10).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 191 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/10
Hello 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/+/43612
to look at the new patch set (#11).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 191 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/11
Hello 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/+/43612
to look at the new patch set (#12).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 191 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/12
Hello 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/+/43612
to look at the new patch set (#13).
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
ec/system76_ec: Add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 191 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/13
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: Add support for System76 EC ......................................................................
Patch Set 13:
Not sure why but the build is failing with fatal: not a git repository: 3rdparty/amd_blobs/../../.git/modules/3rdparty/amd_blobs
I haven't touched .gitmodules
Hello 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/+/43612
to look at the new patch set (#14).
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
ec/system76_ec: add support for System76 EC
This adds ACPI code and console output support for System76 EC. Also converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- M src/console/Kconfig M src/console/console.c A src/ec/system76/ec/Kconfig A src/ec/system76/ec/Makefile.inc R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl A src/ec/system76/ec/system76_ec.c A src/include/console/system76_ec.h M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 19 files changed, 191 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/14
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig File src/ec/system76/ec/Kconfig:
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig@... PS8, Line 7: bool
This would need to "depends on EC_SYSTEM76_EC"
Done
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/Kconfig@... PS8, Line 8: default n
indent should use tabs
Done
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/8/src/ec/system76/ec/system76... PS8, Line 32: wait_us(10000, system76_ec_read(0) == 0);
Tabs, tabs, tabs 😋
Done
Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Removed Code-Review+2 by Angel Pons th3fanbus@gmail.com
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG@10 PS14, Line 10: EC_SYSTEM76_EC why not EC_SYSTEM76?
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init(); I'm not sure if that's the right place to init the ec. I expect the ec driver to get more functionality over time. Why not do it like the other ec and use chip_operations/enable_dev?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG@10 PS14, Line 10: EC_SYSTEM76_EC
why not EC_SYSTEM76?
The EC subfolders need to be ec/vendor/product - so I used ec/system76/ec and therefore have EC_SYSTEM76_EC as the Kconfig
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
I'm not sure if that's the right place to init the ec. […]
This is just the init for the console driver. The EC otherwise doesn't require initialization from firmware
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG@10 PS14, Line 10: EC_SYSTEM76_EC
The EC subfolders need to be ec/vendor/product - so I used ec/system76/ec and therefore have EC_SYST […]
well, the Kconfig name is not required to match the folder structure. I'd drop the trailing _EC
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
This is just the init for the console driver. […]
is that required by the ec hw or the system76 ec firmware?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
is that required by the ec hw or the system76 ec firmware?
EC firmware
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43612/14//COMMIT_MSG@10 PS14, Line 10: EC_SYSTEM76_EC
well, the Kconfig name is not required to match the folder structure. […]
The EC is "System76 EC", so the repitition is fine. Just as Google has EC_GOOGLE_CHROMEEC
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
EC firmware
so, why can't this init be done in firmware?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
so, why can't this init be done in firmware?
The purpose of this function is to clear the buffer used for the print command. The print command is buffered and only actually outputs characters on a newline or running out of buffer space purely as a performance optimization in coreboot. Therefore, coreboot must clear the buffer when it first gains control of the CPU in order to use this optimization.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/ec/system76/ec/system7... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/ec/system76/ec/system7... PS14, Line 47: // If we hit the end of the buffer, or were given a newline, flush Michael, this is pretty well explained here
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
Ideally, this would be three patches: one that adds the EC code, one that hooks it up to the console and one that hooks it up to the mainboard.
About the console output, where does it go? If it ends up on a UART for instance, it would be better to use the existing functions instead of adding yet another console interface.
https://review.coreboot.org/c/coreboot/+/43612/14/src/ec/system76/ec/system7... File src/ec/system76/ec/system76_ec.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/ec/system76/ec/system7... PS14, Line 40: // Read length Just a random thought: if you'd use defined names for the registers, it would be more readable and you could drop all these code-rephrasing comments.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
The purpose of this function is to clear the buffer used for the print command. […]
Got it but why is that different from other ec implementations (ene ec on acer es1-572 for example)? If this is something that could be used for others ecs, too, then make it generic code, please.
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
Got it but why is that different from other ec implementations (ene ec on acer es1-572 for example)? […]
Read the code in system76_ec.c
The interface for sending console data is specific to ECs running System76 EC firmware.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
Read the code in system76_ec.c […]
please stop marking this as resolved and let others have a look^^ @Nico what do you think?
Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 14:
Patch Set 14:
(1 comment)
Ideally, this would be three patches: one that adds the EC code, one that hooks it up to the console and one that hooks it up to the mainboard.
It was two patches before. Since the ec/system76/ec/acpi directory was mostly moves from the mainboard/system76/lemp9/acpi directory, it was redone as one patch. I can move the console stuff out into a separate patch.
About the console output, where does it go? If it ends up on a UART for instance, it would be better to use the existing functions instead of adding yet another console interface.
The console interface as defined in system76_ec.c will create line-buffered output that is sent to the EC to be multiplexed to any enabled EC console outputs.
An EC running System76 EC may store this in a memory ringbuffer, send it to a parallel port debugger, an I2C debugger, or a UART, depending on the EC's firmware configuration.
It is very different from the other console methods in how it works that it does not make sense to combine it with another console method, in my opinion.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43612
to look at the new patch set (#15).
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
ec/system76_ec: add support for System76 EC
This adds ACPI code for System76 EC and converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be --- A src/ec/system76/ec/Kconfig R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 14 files changed, 89 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/43612/15
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 15: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 15: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c File src/console/console.c:
https://review.coreboot.org/c/coreboot/+/43612/14/src/console/console.c@25 PS14, Line 25: __system76_ec_init();
please stop marking this as resolved and let others have a look^^ […]
Done
Philipp Deppenwiese has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
ec/system76_ec: add support for System76 EC
This adds ACPI code for System76 EC and converts system76/lemp9 to use EC_SYSTEM76_EC.
Tested on system76/lemp9.
Signed-off-by: Jeremy Soller jeremy@system76.com Change-Id: I1f693268d94b693b6764e4a3baf4c3180689f3be Reviewed-on: https://review.coreboot.org/c/coreboot/+/43612 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Reviewed-by: Michael Niewöhner --- A src/ec/system76/ec/Kconfig R src/ec/system76/ec/acpi/ac.asl R src/ec/system76/ec/acpi/battery.asl A src/ec/system76/ec/acpi/buttons.asl R src/ec/system76/ec/acpi/ec.asl R src/ec/system76/ec/acpi/ec_ram.asl R src/ec/system76/ec/acpi/hid.asl R src/ec/system76/ec/acpi/lid.asl R src/ec/system76/ec/acpi/s76.asl M src/mainboard/system76/lemp9/Kconfig D src/mainboard/system76/lemp9/acpi/buttons.asl M src/mainboard/system76/lemp9/acpi/mainboard.asl M src/mainboard/system76/lemp9/devicetree.cb M src/mainboard/system76/lemp9/dsdt.asl 14 files changed, 89 insertions(+), 40 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved Michael Niewöhner: Looks good to me, but someone else must approve
diff --git a/src/ec/system76/ec/Kconfig b/src/ec/system76/ec/Kconfig new file mode 100644 index 0000000..fea4743 --- /dev/null +++ b/src/ec/system76/ec/Kconfig @@ -0,0 +1,9 @@ +config EC_SYSTEM76_EC + bool + help + System76 EC + +config EC_SYSTEM76_EC_COLOR_KEYBOARD + depends on EC_SYSTEM76_EC + bool + default n diff --git a/src/mainboard/system76/lemp9/acpi/ac.asl b/src/ec/system76/ec/acpi/ac.asl similarity index 100% rename from src/mainboard/system76/lemp9/acpi/ac.asl rename to src/ec/system76/ec/acpi/ac.asl diff --git a/src/mainboard/system76/lemp9/acpi/battery.asl b/src/ec/system76/ec/acpi/battery.asl similarity index 100% rename from src/mainboard/system76/lemp9/acpi/battery.asl rename to src/ec/system76/ec/acpi/battery.asl diff --git a/src/ec/system76/ec/acpi/buttons.asl b/src/ec/system76/ec/acpi/buttons.asl new file mode 100644 index 0000000..ae85c67 --- /dev/null +++ b/src/ec/system76/ec/acpi/buttons.asl @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (PWRB) +{ + Name (_HID, EisaId ("PNP0C0C")) + Name (_PRW, Package () { EC_GPE_SWI, 3 }) +} + +Device (SLPB) +{ + Name (_HID, EisaId ("PNP0C0E")) + Name (_PRW, Package () { EC_GPE_SWI, 3 }) +} diff --git a/src/mainboard/system76/lemp9/acpi/ec.asl b/src/ec/system76/ec/acpi/ec.asl similarity index 89% rename from src/mainboard/system76/lemp9/acpi/ec.asl rename to src/ec/system76/ec/acpi/ec.asl index f8496ae..5989f6d 100644 --- a/src/mainboard/system76/lemp9/acpi/ec.asl +++ b/src/ec/system76/ec/acpi/ec.asl @@ -1,9 +1,18 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-Device (EC0) +Scope (_SB) { + #include "ac.asl" + #include "battery.asl" + #include "buttons.asl" + #include "hid.asl" + #include "lid.asl" + #include "s76.asl" +} + +Device (_SB.PCI0.LPCB.EC0) { Name (_HID, EisaId ("PNP0C09") /* Embedded Controller Device */) // _HID: Hardware ID - Name (_GPE, 0x50 /* GPP_E16 */) // _GPE: General Purpose Events + Name (_GPE, EC_GPE_SCI) // _GPE: General Purpose Events Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, @@ -20,7 +29,7 @@ ) })
- #include "acpi/ec_ram.asl" + #include "ec_ram.asl"
Name (ECOK, Zero) Method (_REG, 2, Serialized) // _REG: Region Availability @@ -39,6 +48,10 @@ ^^^^BAT0.UPBI() ^^^^BAT0.UPBS()
+ // Notify of changes + Notify(^^^^AC, Zero) + Notify(^^^^BAT0, Zero) + PNOT ()
// EC is now available @@ -71,8 +84,6 @@ Notify(^^^^AC, Zero) Notify(^^^^BAT0, Zero)
- Sleep (1000) - // Reset System76 Device ^^^^S76D.RSET() } @@ -116,13 +127,17 @@ Method (_Q11, 0, NotSerialized) // Brightness Down { Debug = "EC: Brightness Down" - ^^^^HIDD.HPEM (20) + if (^^^^HIDD.HRDY) { + ^^^^HIDD.HPEM (20) + } }
Method (_Q12, 0, NotSerialized) // Brightness Up { Debug = "EC: Brightness Up" - ^^^^HIDD.HPEM (19) + if (^^^^HIDD.HRDY) { + ^^^^HIDD.HPEM (19) + } }
Method (_Q13, 0, NotSerialized) // Camera Toggle @@ -133,10 +148,10 @@ Method (_Q14, 0, NotSerialized) // Airplane Mode { Debug = "EC: Airplane Mode" - // Only send HIDD message when hardware airplane mode not in use - If (ECOS == 2) { + if (^^^^HIDD.HRDY) { ^^^^HIDD.HPEM (8) } + // TODO: hardware airplane mode }
Method (_Q15, 0, NotSerialized) // Suspend Button @@ -150,13 +165,10 @@ Debug = "EC: AC Detect" ^^^^AC.ACFG = ADP Notify (AC, 0x80) // Status Change - Sleep (0x01F4) If (BAT0) { Notify (^^^^BAT0, 0x81) // Information Change - Sleep (0x32) Notify (^^^^BAT0, 0x80) // Status Change - Sleep (0x32) } }
diff --git a/src/mainboard/system76/lemp9/acpi/ec_ram.asl b/src/ec/system76/ec/acpi/ec_ram.asl similarity index 100% rename from src/mainboard/system76/lemp9/acpi/ec_ram.asl rename to src/ec/system76/ec/acpi/ec_ram.asl diff --git a/src/mainboard/system76/lemp9/acpi/hid.asl b/src/ec/system76/ec/acpi/hid.asl similarity index 100% rename from src/mainboard/system76/lemp9/acpi/hid.asl rename to src/ec/system76/ec/acpi/hid.asl diff --git a/src/mainboard/system76/lemp9/acpi/lid.asl b/src/ec/system76/ec/acpi/lid.asl similarity index 88% rename from src/mainboard/system76/lemp9/acpi/lid.asl rename to src/ec/system76/ec/acpi/lid.asl index ea382eb..45e646a 100644 --- a/src/mainboard/system76/lemp9/acpi/lid.asl +++ b/src/ec/system76/ec/acpi/lid.asl @@ -3,7 +3,7 @@ Device (LID0) { Name (_HID, EisaId ("PNP0C0D")) - Name (_PRW, Package () { 0x29 /* GPP_D9 */, 3 }) + Name (_PRW, Package () { EC_GPE_SWI, 3 })
Method (_LID, 0, NotSerialized) { DEBUG = "LID: _LID" diff --git a/src/mainboard/system76/lemp9/acpi/s76.asl b/src/ec/system76/ec/acpi/s76.asl similarity index 65% rename from src/mainboard/system76/lemp9/acpi/s76.asl rename to src/ec/system76/ec/acpi/s76.asl index 399e569..7beb50b 100644 --- a/src/mainboard/system76/lemp9/acpi/s76.asl +++ b/src/ec/system76/ec/acpi/s76.asl @@ -14,6 +14,9 @@ Debug = "S76D: RSET" SAPL(0) SKBL(0) +#if CONFIG(EC_SYSTEM76_EC_COLOR_KEYBOARD) + SKBC(0xFFFFFF) +#endif // CONFIG(EC_SYSTEM76_EC_COLOR_KEYBOARD) }
Method (INIT, 0, Serialized) { @@ -61,6 +64,32 @@ } }
+#if CONFIG(EC_SYSTEM76_EC_COLOR_KEYBOARD) + // Set KB LED Brightness + Method (SKBL, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = 6 + ^^PCI0.LPCB.EC0.FBUF = Arg0 + ^^PCI0.LPCB.EC0.FBF1 = 0 + ^^PCI0.LPCB.EC0.FBF2 = Arg0 + ^^PCI0.LPCB.EC0.FCMD = 0xCA + } + } + + // Set Keyboard Color + Method (SKBC, 1, Serialized) { + If (^^PCI0.LPCB.EC0.ECOK) { + ^^PCI0.LPCB.EC0.FDAT = 0x3 + ^^PCI0.LPCB.EC0.FBUF = (Arg0 & 0xFF) + ^^PCI0.LPCB.EC0.FBF1 = ((Arg0 >> 16) & 0xFF) + ^^PCI0.LPCB.EC0.FBF2 = ((Arg0 >> 8) & 0xFF) + ^^PCI0.LPCB.EC0.FCMD = 0xCA + Return (Arg0) + } Else { + Return (0) + } + } +#else // CONFIG(EC_SYSTEM76_EC_COLOR_KEYBOARD) // Get KB LED Method (GKBL, 0, Serialized) { Local0 = 0 @@ -81,4 +110,5 @@ ^^PCI0.LPCB.EC0.FCMD = 0xCA } } +#endif // CONFIG(EC_SYSTEM76_EC_COLOR_KEYBOARD) } diff --git a/src/mainboard/system76/lemp9/Kconfig b/src/mainboard/system76/lemp9/Kconfig index b2ce9e3..3431de1 100644 --- a/src/mainboard/system76/lemp9/Kconfig +++ b/src/mainboard/system76/lemp9/Kconfig @@ -3,7 +3,7 @@ config BOARD_SPECIFIC_OPTIONS def_bool y select BOARD_ROMSIZE_KB_16384 - select EC_ACPI + select EC_SYSTEM76_EC select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES select HAVE_SMI_HANDLER diff --git a/src/mainboard/system76/lemp9/acpi/buttons.asl b/src/mainboard/system76/lemp9/acpi/buttons.asl deleted file mode 100644 index 81e6124..0000000 --- a/src/mainboard/system76/lemp9/acpi/buttons.asl +++ /dev/null @@ -1,13 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -Device (PWRB) -{ - Name (_HID, EisaId ("PNP0C0C")) - Name (_PRW, Package () { 0x29 /* GPP_D9 */, 3 }) -} - -Device (SLPB) -{ - Name (_HID, EisaId ("PNP0C0E")) - Name (_PRW, Package () { 0x29 /* GPP_D9 */, 3 }) -} diff --git a/src/mainboard/system76/lemp9/acpi/mainboard.asl b/src/mainboard/system76/lemp9/acpi/mainboard.asl index 11735e8..4e8691d 100644 --- a/src/mainboard/system76/lemp9/acpi/mainboard.asl +++ b/src/mainboard/system76/lemp9/acpi/mainboard.asl @@ -1,12 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#define EC_GPE_SCI 0x50 /* GPP_E16 */ +#define EC_GPE_SWI 0x29 /* GPP_D9 */ +#define EC_COLOR_KEYBOARD 0 +#include <ec/system76/ec/acpi/ec.asl> + Scope (_SB) { - #include "ac.asl" - #include "battery.asl" - #include "buttons.asl" - #include "hid.asl" - #include "lid.asl" - #include "s76.asl" #include "sleep.asl" }
diff --git a/src/mainboard/system76/lemp9/devicetree.cb b/src/mainboard/system76/lemp9/devicetree.cb index 4c6c866..25ae62f 100644 --- a/src/mainboard/system76/lemp9/devicetree.cb +++ b/src/mainboard/system76/lemp9/devicetree.cb @@ -147,14 +147,14 @@
# LPC (soc/intel/cannonlake/lpc.c) # LPC configuration from lspci -s 1f.0 -xxx - # Address 0x84: Decode 0x80 - 0x8F + # Address 0x84: Decode 0x80 - 0x8F (Port 80) register "gen1_dec" = "0x000c0081" - # Address 0x88: Decode 0x68 - 0x6F + # Address 0x88: Decode 0x68 - 0x6F (PMC) register "gen2_dec" = "0x00040069" - # Address 0x8C: Decode 0x3320 - 0x332F - register "gen3_dec" = "0x000c3321" - # Address 0x90: Disabled - register "gen4_dec" = "0x00000000" + # Address 0x8C: Decode 0xE00 - 0xEFF (AP/EC command) + register "gen3_dec" = "0x00fc0E01" + # Address 0x90: Decode 0xF00 - 0xFFF (AP/EC debug) + register "gen4_dec" = "0x00fc0F01"
# PMC (soc/intel/cannonlake/pmc.c) # Enable deep Sx states diff --git a/src/mainboard/system76/lemp9/dsdt.asl b/src/mainboard/system76/lemp9/dsdt.asl index 7ae8ac4..eac27b5 100644 --- a/src/mainboard/system76/lemp9/dsdt.asl +++ b/src/mainboard/system76/lemp9/dsdt.asl @@ -25,7 +25,6 @@ Scope (_SB.PCI0.LPCB) { #include <drivers/pc80/pc/ps2_controller.asl> - #include "acpi/ec.asl" }
#include "acpi/mainboard.asl"
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 16:
@Jeremy can you upload documentation in a follow-up CL please?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 16:
It is very different from the other console methods in how it works that it does not make sense to combine it with another console method, in my opinion.
Well, it's a rather mainboard specific console, in a way like `qemu_debugcon`. They could be grouped as `board_console`, for instance. But as long as there are only two (doesn't explode yet), it's probably not worth it yet.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 16:
Patch Set 16:
@Jeremy can you upload documentation in a follow-up CL please?
What about requesting documentation *before* giving +2?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 16:
Patch Set 16:
Patch Set 16:
@Jeremy can you upload documentation in a follow-up CL please?
What about requesting documentation *before* giving +2?
^ @Phillip
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43612 )
Change subject: ec/system76_ec: add support for System76 EC ......................................................................
Patch Set 16:
I don't see any issue to do a follow-up. I also don't see it as requirement to put it in one CL. The code looks good, so have it merged is fine
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Jeremy Soller (Code Review)
-
Michael Niewöhner (Code Review)
-
Nico Huber (Code Review)
-
Philipp Deppenwiese (Code Review)