Change in ...coreboot[master]: mb/lenovo/x131e: function key support
James has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31067 Change subject: mb/lenovo/x131e: function key support ...................................................................... mb/lenovo/x131e: function key support Enables function keys for X131e. The IT8518e EC of this board uses some different ACPI methods compared to the regular Lenovo H8. Add an option to use the alternative set of methods. Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Signed-off-by: James Ye <jye836@gmail.com> --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x131e/Kconfig M src/mainboard/lenovo/x131e/Makefile.inc M src/mainboard/lenovo/x131e/devicetree.cb A src/mainboard/lenovo/x131e/ec.h M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x131e/romstage.c A src/mainboard/lenovo/x131e/smihandler.c 9 files changed, 146 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31067/1 diff --git a/src/ec/lenovo/h8/Kconfig b/src/ec/lenovo/h8/Kconfig index f9b0b14..3dab615 100644 --- a/src/ec/lenovo/h8/Kconfig +++ b/src/ec/lenovo/h8/Kconfig @@ -28,6 +28,12 @@ Disable BDC detection and assume bluetooth is installed. Required for bluetooth on wifi cards, as it's not possible to detect it in coreboot. +config H8_ALT_EC_QUERY + bool "Alternate ACPI EC query methods" + default n + help + Use alternative EC query methods for X131e + endif config H8_DOCK_EARLY_INIT diff --git a/src/ec/lenovo/h8/acpi/ec.asl b/src/ec/lenovo/h8/acpi/ec.asl index 327a2cf..7c96504 100644 --- a/src/ec/lenovo/h8/acpi/ec.asl +++ b/src/ec/lenovo/h8/acpi/ec.asl @@ -140,7 +140,11 @@ /* Sleep Button pressed */ Method(_Q13, 0, NotSerialized) { +#ifndef CONFIG_H8_ALT_EC_QUERY Notify(^SLPB, 0x80) +#else + ^HKEY.RHK(0x04) +#endif } /* Brightness up GPE */ @@ -217,9 +221,20 @@ Method (_Q17, 0, NotSerialized) { +#ifndef CONFIG_H8_ALT_EC_QUERY ^HKEY.RHK (0x08) +#else + BRIGHTNESS_DOWN() +#endif } +#ifdef CONFIG_H8_ALT_EC_QUERY + Method (_Q18, 0, NotSerialized) + { + BRIGHTNESS_UP() + } +#endif + Method (_Q66, 0, NotSerialized) { ^HKEY.RHK (0x0A) diff --git a/src/mainboard/lenovo/x131e/Kconfig b/src/mainboard/lenovo/x131e/Kconfig index 946b945..ee7a83f 100644 --- a/src/mainboard/lenovo/x131e/Kconfig +++ b/src/mainboard/lenovo/x131e/Kconfig @@ -8,6 +8,7 @@ select USE_NATIVE_RAMINIT select SOUTHBRIDGE_INTEL_C216 select EC_LENOVO_H8 + select H8_ALT_EC_QUERY select NO_UART_ON_SUPERIO select BOARD_ROMSIZE_KB_12288 select HAVE_ACPI_TABLES diff --git a/src/mainboard/lenovo/x131e/Makefile.inc b/src/mainboard/lenovo/x131e/Makefile.inc index 7a00cce..2dab950 100644 --- a/src/mainboard/lenovo/x131e/Makefile.inc +++ b/src/mainboard/lenovo/x131e/Makefile.inc @@ -13,6 +13,7 @@ ## GNU General Public License for more details. ## +smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c romstage-y += gpio.c ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-mainboard.ads diff --git a/src/mainboard/lenovo/x131e/devicetree.cb b/src/mainboard/lenovo/x131e/devicetree.cb index 3d0148f..0a73010 100644 --- a/src/mainboard/lenovo/x131e/devicetree.cb +++ b/src/mainboard/lenovo/x131e/devicetree.cb @@ -47,7 +47,8 @@ chip southbridge/intel/bd82x6x # GPI routing - register "alt_gp_smi_en" = "0x0000" + register "alt_gp_smi_en" = "0x0002" + register "gpi1_routing" = "1" register "gpi6_routing" = "2" register "gpi13_routing" = "2" @@ -115,22 +116,10 @@ register "beepmask1" = "0x87" register "has_power_management_beeps" = "0" - register "event0_enable" = "0xff" - register "event1_enable" = "0xff" - register "event2_enable" = "0xff" - register "event3_enable" = "0xff" - register "event4_enable" = "0xff" - register "event5_enable" = "0xff" - register "event6_enable" = "0xff" - register "event7_enable" = "0xff" - register "event8_enable" = "0xff" - register "event9_enable" = "0xff" - register "eventa_enable" = "0xff" - register "eventb_enable" = "0xff" - register "eventc_enable" = "0xff" - register "eventd_enable" = "0xff" - register "evente_enable" = "0xff" - register "eventf_enable" = "0xff" + register "event2_enable" = "0xc8" # sleep, monitor, brightness down + register "event3_enable" = "0x01" # brightness up + register "eventc_enable" = "0x30" # airplane, camera + register "eventd_enable" = "0x04" # mic register "has_bdc_detection" = "0" diff --git a/src/mainboard/lenovo/x131e/ec.h b/src/mainboard/lenovo/x131e/ec.h new file mode 100644 index 0000000..4a731dc --- /dev/null +++ b/src/mainboard/lenovo/x131e/ec.h @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2011 The Chromium OS Authors. All rights reserved. + * Copyright (C) 2019 James Ye <jye836@gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef X131E_EC_H +#define X131E_EC_H + +#define GPE_EC_SCI 6 +#define GPE_EC_WAKE 13 + +#define EC_CMD_NOTIFY_ACPI_ENTER 0x86 +#define EC_CMD_NOTIFY_ACPI_EXIT 0x87 + +#endif // X131E_EC_H diff --git a/src/mainboard/lenovo/x131e/mainboard.c b/src/mainboard/lenovo/x131e/mainboard.c index 1342aca..9e6a7dd 100644 --- a/src/mainboard/lenovo/x131e/mainboard.c +++ b/src/mainboard/lenovo/x131e/mainboard.c @@ -13,9 +13,17 @@ * GNU General Public License for more details. */ +#include <arch/acpi.h> #include <device/device.h> #include <drivers/intel/gma/int15.h> +#include <ec/acpi/ec.h> #include <ec/lenovo/h8/h8.h> +#include "ec.h" + +void mainboard_suspend_resume(void) +{ + send_ec_command(EC_CMD_NOTIFY_ACPI_ENTER); +} static void mainboard_enable(struct device *dev) { diff --git a/src/mainboard/lenovo/x131e/romstage.c b/src/mainboard/lenovo/x131e/romstage.c index 43e0bd7..30df36f 100644 --- a/src/mainboard/lenovo/x131e/romstage.c +++ b/src/mainboard/lenovo/x131e/romstage.c @@ -26,11 +26,6 @@ /* Enable TPM, EC, PS/2 Keyboard/Mouse */ pci_write_config16(PCH_LPC_DEV, LPC_EN, CNF2_LPC_EN | CNF1_LPC_EN | MC_LPC_EN | KBC_LPC_EN); - - pci_write_config32(PCH_LPC_DEV, LPC_GEN1_DEC, 0x007c1611); - pci_write_config32(PCH_LPC_DEV, LPC_GEN2_DEC, 0x00040069); - pci_write_config32(PCH_LPC_DEV, LPC_GEN3_DEC, 0x000c0701); - pci_write_config32(PCH_LPC_DEV, LPC_GEN4_DEC, 0x000c06a1); } void mainboard_rcba_config(void) diff --git a/src/mainboard/lenovo/x131e/smihandler.c b/src/mainboard/lenovo/x131e/smihandler.c new file mode 100644 index 0000000..a634359 --- /dev/null +++ b/src/mainboard/lenovo/x131e/smihandler.c @@ -0,0 +1,83 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2008-2009 coresystems GmbH + * Copyright (C) 2014 Vladimir Serbinenko + * Copyright (C) 2019 James Ye <jye836@gmail.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2 of + * the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <arch/io.h> +#include <console/console.h> +#include <cpu/x86/smm.h> +#include <ec/acpi/ec.h> +#include <ec/lenovo/h8/h8.h> +#include <southbridge/intel/bd82x6x/pch.h> +#include <southbridge/intel/common/pmutil.h> +#include "ec.h" + +static void mainboard_smi_handle_ec_sci(void) +{ + u8 status = ec_status(); + if (!(status & EC_SCI_EVT)) + return; + + u8 event = ec_query(); + printk(BIOS_DEBUG, "EC event %02x\n", event); +} + +void mainboard_smi_gpi(u32 gpi_sts) +{ + if (gpi_sts & (1 << GPE_EC_SCI)) + mainboard_smi_handle_ec_sci(); +} + +int mainboard_smi_apmc(u8 data) +{ + switch (data) { + case APM_CNT_ACPI_ENABLE: + send_ec_command(EC_CMD_NOTIFY_ACPI_ENTER); + /* use 0x1600/0x1604 to prevent races with userspace */ + ec_set_ports(0x1604, 0x1600); + /* route EC_SCI to SCI */ + gpi_route_interrupt(GPE_EC_SCI, GPI_IS_SCI); + /* discard all events, and enable attention */ + ec_write(0x80, 0x01); + break; + case APM_CNT_ACPI_DISABLE: + send_ec_command(EC_CMD_NOTIFY_ACPI_EXIT); + /* we have to use port 0x62/0x66, as 0x1600/0x1604 doesn't + provide a EC query function */ + ec_set_ports(EC_SC, EC_DATA); + /* route EC_SCI to SMI */ + gpi_route_interrupt(GPE_EC_SCI, GPI_IS_SMI); + /* discard all events, and enable attention */ + ec_write(0x80, 0x01); + break; + } + return 0; +} + +void mainboard_smi_sleep(u8 slp_typ) +{ + if (slp_typ == 3) { + u8 ec_wake = ec_read(0x32); + /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ + if (ec_wake & 0x14) { + /* Redirect EC WAKE GPE to SCI. */ + gpi_route_interrupt(GPE_EC_WAKE, GPI_IS_SCI); + } + } + + //outb(0xe9, 0x64); // EC_KBD_CMD_MUTE + //ec_set_bit(0xbf, 4); // EC_WAKE_SRC_ENABLE, EC_LID_WAKE_ENABLE +} -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James <jye836@gmail.com> Gerrit-MessageType: newchange
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (5 comments) What’s the difference to `src/ec/quanta/it8518/`? https://review.coreboot.org/#/c/31067/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/31067/1//COMMIT_MSG@7 PS1, Line 7: mb/lenovo/x131e: function key support Please use a statement:
Add function key support
Support function key
https://review.coreboot.org/#/c/31067/1//COMMIT_MSG@11 PS1, Line 11: The IT8518e EC of this board uses some different ACPI methods compared to the : regular Lenovo H8. Did you have documentation for that, or did you reverse engineer this? https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig: https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig@35 PS1, Line 35: Use alternative EC query methods for X131e Please also mention the EC.
… using an IT8518e EC.
https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/ec.h File src/mainboard/lenovo/x131e/ec.h: PS1: Shouldn’t this go under `src/ec/…/it…/` like with `src/ec/quanta/it8518/ec.h`? https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/mainboard... File src/mainboard/lenovo/x131e/mainboard.c: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/mainboard... PS1, Line 25: send_ec_command(EC_CMD_NOTIFY_ACPI_ENTER); Please add a comment, why this is needed. -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James Ye <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: James Ye <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 24 Jan 2019 11:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
James Ye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (2 comments)
(5 comments)
What’s the difference to `src/ec/quanta/it8518/`?
quanta/it8518 is used in google/stout The EC is an IT8518 but (from what I can tell) is compatible with Lenovo interfaces. While quanta/it8518 could be used, lenovo/h8 has support for ThinkPad ACPI interfaces that are used or useful on this board (function keys, thinkpad_acpi). X131e Chromebook (google/stout) doesn't have function keys, so doesn't have any of the relevant ACPI methods. So H8 is the most appropriate EC code to use. However, there are a few differences like this that need to be taken into account. https://review.coreboot.org/#/c/31067/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/31067/1//COMMIT_MSG@11 PS1, Line 11: The IT8518e EC of this board uses some different ACPI methods compared to the : regular Lenovo H8.
Did you have documentation for that, or did you reverse engineer this? The ACPI code for the vendor BIOS differs from the coreboot H8 code. I experimentally verified this. So a bit of both?
https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/ec.h File src/mainboard/lenovo/x131e/ec.h: PS1:
Shouldn’t this go under `src/ec/…/it…/` like with `src/ec/quanta/it8518/ec. […] I'm not sure. Is it more appropriate for it to be `src/ec/lenovo/it8518/ec.h`?
-- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James Ye <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: James Ye <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 24 Jan 2019 11:55:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/devicetre... File src/mainboard/lenovo/x131e/devicetree.cb: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/devicetre... PS1, Line 118: event0 and event1 are critical on other lenovo devices, are they zero on x131e ? https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/romstage.... File src/mainboard/lenovo/x131e/romstage.c: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/romstage.... PS1, Line 30: unrelated https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/smihandle... File src/mainboard/lenovo/x131e/smihandler.c: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/smihandle... PS1, Line 81: //outb(0xe9, 0x64); // EC_KBD_CMD_MUTE don't include dead code -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James Ye <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James Ye <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 24 Jan 2019 14:22:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/devicetre... File src/mainboard/lenovo/x131e/devicetree.cb: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/devicetre... PS1, Line 118:
event0 and event1 are critical on other lenovo devices, are they zero on x131e ? What are they used for and how can I test it?
I notice that no board sets event{0,1}_enable. https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/smihandle... File src/mainboard/lenovo/x131e/smihandler.c: https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/smihandle... PS1, Line 81: //outb(0xe9, 0x64); // EC_KBD_CMD_MUTE
don't include dead code Whoops, that wasn't supposed to be there.
-- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 24 Jan 2019 22:33:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: comment
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig: https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig@35 PS1, Line 35: Use alternative EC query methods for X131e why is it called EC query methods ?? https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/acpi/ec.asl File src/ec/lenovo/h8/acpi/ec.asl: https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/acpi/ec.asl@a141 PS1, Line 141: I'd prefer to split Method(_Q* into a new file and include those depending on the real ec in use. -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 25 Jan 2019 09:08:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: function key support ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig: https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig@35 PS1, Line 35: Use alternative EC query methods for X131e
why is it called EC query methods ?? The methods being modified (_Qxx) are handlers for EC queries.
https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/acpi/ec.asl File src/ec/lenovo/h8/acpi/ec.asl: https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/acpi/ec.asl@a141 PS1, Line 141:
I'd prefer to split Method(_Q* into a new file and include those depending on the real ec in use. Would the _Qxx methods that are common between standard H8 and H8-compatible IT8518 be duplicated between the files in this case?
-- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 1 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 25 Jan 2019 10:58:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: comment
Hello Alexander Couzens, Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31067 to look at the new patch set (#2). Change subject: mb/lenovo/x131e: add function key support ...................................................................... mb/lenovo/x131e: add function key support Enables function keys for X131e. SMI handler based on google/stout. ACPI code and event masks are reverse-engineered. The IT8518e EC of this board uses some different ACPI methods compared to the regular Lenovo H8. Add an option to use the alternative set of methods. Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Signed-off-by: James Ye <jye836@gmail.com> --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x131e/Kconfig M src/mainboard/lenovo/x131e/Makefile.inc M src/mainboard/lenovo/x131e/devicetree.cb M src/mainboard/lenovo/x131e/mainboard.c A src/mainboard/lenovo/x131e/smihandler.c 7 files changed, 123 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31067/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 2 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: add function key support ...................................................................... Patch Set 2: Code-Review-1 Ideally, instead of defining EC_CMD_NOTIFY_ACPI_* twice, <quanta/it8518/ec.h> should be included. However, quanta/it8518/ec.h has conflicting definitions with acpi/ec.h As the only use of quanta/it8518 is google/stout, which is almost identical to x131e, it8518 should be modified to work with the generic ACPI EC code. -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 2 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 12 Feb 2019 11:37:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello Alexander Couzens, Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31067 to look at the new patch set (#3). Change subject: mb/lenovo/x131e: add function key support ...................................................................... mb/lenovo/x131e: add function key support Enables function keys for X131e. SMI handler based on google/stout. ACPI code and event masks are reverse-engineered. The IT8518e EC of this board uses some different ACPI methods compared to the regular Lenovo H8. Add an option to use the alternative set of methods. Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Signed-off-by: James Ye <jye836@gmail.com> --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x131e/Kconfig M src/mainboard/lenovo/x131e/Makefile.inc M src/mainboard/lenovo/x131e/devicetree.cb M src/mainboard/lenovo/x131e/mainboard.c A src/mainboard/lenovo/x131e/smihandler.c 7 files changed, 123 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31067/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 3 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello Alexander Couzens, Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31067 to look at the new patch set (#4). Change subject: mb/lenovo/x131e: add function key support ...................................................................... mb/lenovo/x131e: add function key support Enables function keys for X131e. SMI handler based on google/stout. ACPI code and event masks are reverse-engineered. The IT8518e EC of this board uses some different ACPI methods compared to the regular Lenovo H8. Add an option to use the alternative set of methods. Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Signed-off-by: James Ye <jye836@gmail.com> --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x131e/Kconfig M src/mainboard/lenovo/x131e/Makefile.inc M src/mainboard/lenovo/x131e/devicetree.cb M src/mainboard/lenovo/x131e/mainboard.c A src/mainboard/lenovo/x131e/smihandler.c 7 files changed, 123 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31067/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 4 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: add function key support ...................................................................... Patch Set 4: Code-Review-1 -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 4 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 12 Feb 2019 12:38:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
James has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 ) Change subject: mb/lenovo/x131e: add function key support ...................................................................... Patch Set 5: This change is ready for review. -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 5 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nicola Corna <nicola@corna.info> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 22 Nov 2019 11:57:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello Alexander Couzens, Patrick Rudolph, Felix Held, Matt DeVillier, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/31067 to look at the new patch set (#7). Change subject: mb/lenovo/x131e: add function key support ...................................................................... mb/lenovo/x131e: add function key support This enables function keys for X131e. The SMI handler code is based on google/stout. The ACPI code and event masks are reverse-engineered. The ACPI interface of the Quanta IT8518e differs somewhat from the Lenovo H8. Add a special-case for this mainboard. Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Signed-off-by: James Ye <jye836@gmail.com> --- M src/ec/lenovo/h8/acpi/ec.asl M src/mainboard/lenovo/x131e/Kconfig M src/mainboard/lenovo/x131e/Makefile.inc M src/mainboard/lenovo/x131e/devicetree.cb M src/mainboard/lenovo/x131e/mainboard.c A src/mainboard/lenovo/x131e/smihandler.c 6 files changed, 98 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31067/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/31067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 7 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin Roth <martinroth@google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nicola Corna <nicola@corna.info> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31067?usp=email ) Change subject: mb/lenovo/x131e: add function key support ...................................................................... Abandoned This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author. -- To view, visit https://review.coreboot.org/c/coreboot/+/31067?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3a01f37a8b54889b55e92c501c9350e6c68bd57 Gerrit-Change-Number: 31067 Gerrit-PatchSet: 7 Gerrit-Owner: James <jye836@gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis@fe80.eu> Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de> Gerrit-Reviewer: James <jye836@gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless@gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nicola Corna <nicola@corna.info> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
participants (5)
-
James (Code Review) -
James Ye (Code Review)
-
Martin L Roth (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)