Hello Alexander Couzens, Patrick Rudolph, Nathaniel Roach, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/30905
to review the following change.
Change subject: Revert "lenovo/h8,thinkpads: Re-do USB Always On" ......................................................................
Revert "lenovo/h8,thinkpads: Re-do USB Always On"
This reverts commit 4f4322dd68575402e099e2dfda057687388f064e.
Reason for revert: According to bug #171 on coreboot's bug tracker: "It seems like commit 4f4322dd68575402e099e2dfda057687388f064e (which actually mentions this issue) was not enough to resolve this issue for me."
Change-Id: I05604102d12a9b3a8b4cbfafae3cf57dd66eb27c --- M src/ec/lenovo/h8/Makefile.inc M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h A src/ec/lenovo/h8/smm.c M src/mainboard/lenovo/l520/smihandler.c M src/mainboard/lenovo/t420/smihandler.c M src/mainboard/lenovo/t420s/smihandler.c M src/mainboard/lenovo/t430/smihandler.c M src/mainboard/lenovo/t430s/smihandler.c M src/mainboard/lenovo/t520/smihandler.c M src/mainboard/lenovo/t530/smihandler.c M src/mainboard/lenovo/x201/cmos.layout M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/lenovo/x220/cmos.layout M src/mainboard/lenovo/x220/smihandler.c M src/mainboard/lenovo/x230/smihandler.c 16 files changed, 72 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/30905/1
diff --git a/src/ec/lenovo/h8/Makefile.inc b/src/ec/lenovo/h8/Makefile.inc index ebf6d7d..e4408f1 100644 --- a/src/ec/lenovo/h8/Makefile.inc +++ b/src/ec/lenovo/h8/Makefile.inc @@ -9,5 +9,6 @@ ramstage-y += bluetooth.c ramstage-y += wwan.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += ssdt.c +smm-y += smm.c
endif diff --git a/src/ec/lenovo/h8/h8.c b/src/ec/lenovo/h8/h8.c index da216ec..1f1655d 100644 --- a/src/ec/lenovo/h8/h8.c +++ b/src/ec/lenovo/h8/h8.c @@ -128,34 +128,6 @@
}
-void h8_usb_always_on_enable(enum usb_always_on on) -{ - u8 val; - - switch (on) { - case UAO_OFF: - val = ec_read(H8_USB_ALWAYS_ON); - // Clear bits 0,2,3 - val &= ~(H8_USB_ALWAYS_ON_ENABLE | H8_USB_ALWAYS_ON_AC_ONLY); - ec_write(H8_USB_ALWAYS_ON, val); - break; - - case UAO_AC_AND_BATTERY: - val = ec_read(H8_USB_ALWAYS_ON); - val |= H8_USB_ALWAYS_ON_ENABLE; // Set bit 0 - val &= ~H8_USB_ALWAYS_ON_AC_ONLY; // Clear bits 2 and 3 - ec_write(H8_USB_ALWAYS_ON, val); - break; - - case UAO_AC_ONLY: - val = ec_read(H8_USB_ALWAYS_ON); - // Set bits 0,2,3 - val |= (H8_USB_ALWAYS_ON_ENABLE | H8_USB_ALWAYS_ON_AC_ONLY); - ec_write(H8_USB_ALWAYS_ON, val); - break; - } -} - void h8_usb_power_enable(int onoff) { if (onoff) @@ -298,10 +270,8 @@ ec_write(0x1f, conf->eventf_enable);
ec_write(H8_FAN_CONTROL, H8_FAN_CONTROL_AUTO); - - if (get_option(&val, "usb_always_on") != CB_SUCCESS) - val = 0; - h8_usb_always_on_enable(val); + ec_write(H8_USB_ALWAYS_ON, ec_read(H8_USB_ALWAYS_ON) & + ~H8_USB_ALWAYS_ON_ENABLE);
if (get_option(&val, "wlan") != CB_SUCCESS) val = 1; diff --git a/src/ec/lenovo/h8/h8.h b/src/ec/lenovo/h8/h8.h index a46ba1f..4ac395a 100644 --- a/src/ec/lenovo/h8/h8.h +++ b/src/ec/lenovo/h8/h8.h @@ -19,16 +19,9 @@ #include <stdint.h> #include <device/device.h>
-enum usb_always_on { - UAO_OFF = 0, - UAO_AC_AND_BATTERY = 1, - UAO_AC_ONLY = 2 -}; - void h8_trackpoint_enable(int on); void h8_wlan_enable(int on); void h8_set_audio_mute(int on); -void h8_usb_always_on_enable(enum usb_always_on on); void h8_usb_power_enable(int on); void h8_enable_event(int event); void h8_disable_event(int event); diff --git a/src/ec/lenovo/h8/smm.c b/src/ec/lenovo/h8/smm.c new file mode 100644 index 0000000..6005c31 --- /dev/null +++ b/src/ec/lenovo/h8/smm.c @@ -0,0 +1,47 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2016 Nicola Corna nicola@corna.info + * + * 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 <ec/acpi/ec.h> +#include <pc80/mc146818rtc.h> + +#include "h8.h" + +enum usb_always_on { + UAO_OFF = 0, + UAO_AC_AND_BATTERY, + UAO_AC_ONLY +}; + +void h8_usb_always_on(void) +{ + enum usb_always_on val; + u8 reg; + + if (get_option(&val, "usb_always_on") != CB_SUCCESS) + val = UAO_OFF; + + if (val == UAO_AC_AND_BATTERY) { + reg = ec_read(H8_USB_ALWAYS_ON); + reg &= ~H8_USB_ALWAYS_ON_AC_ONLY; + reg |= H8_USB_ALWAYS_ON_ENABLE; + ec_write(H8_USB_ALWAYS_ON, reg); + } else if (val == UAO_AC_ONLY) { + reg = ec_read(H8_USB_ALWAYS_ON); + reg |= H8_USB_ALWAYS_ON_AC_ONLY; + reg |= H8_USB_ALWAYS_ON_ENABLE; + ec_write(H8_USB_ALWAYS_ON, reg); + ec_set_bit(0x2, 3); + } +} diff --git a/src/mainboard/lenovo/l520/smihandler.c b/src/mainboard/lenovo/l520/smihandler.c index 982233d..fa038ed 100644 --- a/src/mainboard/lenovo/l520/smihandler.c +++ b/src/mainboard/lenovo/l520/smihandler.c @@ -73,6 +73,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t420/smihandler.c b/src/mainboard/lenovo/t420/smihandler.c index bc92cf1..dd29232 100644 --- a/src/mainboard/lenovo/t420/smihandler.c +++ b/src/mainboard/lenovo/t420/smihandler.c @@ -72,6 +72,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t420s/smihandler.c b/src/mainboard/lenovo/t420s/smihandler.c index 0e8e6d0..578aee8 100644 --- a/src/mainboard/lenovo/t420s/smihandler.c +++ b/src/mainboard/lenovo/t420s/smihandler.c @@ -102,6 +102,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t430/smihandler.c b/src/mainboard/lenovo/t430/smihandler.c index 9a567ab..910fc1a 100644 --- a/src/mainboard/lenovo/t430/smihandler.c +++ b/src/mainboard/lenovo/t430/smihandler.c @@ -72,6 +72,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t430s/smihandler.c b/src/mainboard/lenovo/t430s/smihandler.c index 8831e9d..79679dc 100644 --- a/src/mainboard/lenovo/t430s/smihandler.c +++ b/src/mainboard/lenovo/t430s/smihandler.c @@ -102,6 +102,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t520/smihandler.c b/src/mainboard/lenovo/t520/smihandler.c index f8400dd..2c50e23 100644 --- a/src/mainboard/lenovo/t520/smihandler.c +++ b/src/mainboard/lenovo/t520/smihandler.c @@ -102,6 +102,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/t530/smihandler.c b/src/mainboard/lenovo/t530/smihandler.c index 150232f..c967e40 100644 --- a/src/mainboard/lenovo/t530/smihandler.c +++ b/src/mainboard/lenovo/t530/smihandler.c @@ -102,6 +102,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x201/cmos.layout b/src/mainboard/lenovo/x201/cmos.layout index 0cda679..d8d8794 100644 --- a/src/mainboard/lenovo/x201/cmos.layout +++ b/src/mainboard/lenovo/x201/cmos.layout @@ -70,7 +70,7 @@ 419 1 e 1 power_management_beeps 420 1 e 1 low_battery_beep 421 1 e 9 sata_mode -422 2 e 11 usb_always_on +422 1 e 11 usb_always_on #423 1 r 1 unused
# coreboot config options: northbridge diff --git a/src/mainboard/lenovo/x201/smihandler.c b/src/mainboard/lenovo/x201/smihandler.c index f1e2c3f..10ca4d4 100644 --- a/src/mainboard/lenovo/x201/smihandler.c +++ b/src/mainboard/lenovo/x201/smihandler.c @@ -178,6 +178,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x220/cmos.layout b/src/mainboard/lenovo/x220/cmos.layout index d4a4ed3..c6e270a 100644 --- a/src/mainboard/lenovo/x220/cmos.layout +++ b/src/mainboard/lenovo/x220/cmos.layout @@ -69,7 +69,7 @@ 418 1 e 1 sticky_fn 419 1 e 1 power_management_beeps 421 1 e 9 sata_mode -422 2 e 12 usb_always_on +422 1 e 12 usb_always_on #423 1 r 1 unused
# coreboot config options: cpu diff --git a/src/mainboard/lenovo/x220/smihandler.c b/src/mainboard/lenovo/x220/smihandler.c index 150232f..c967e40 100644 --- a/src/mainboard/lenovo/x220/smihandler.c +++ b/src/mainboard/lenovo/x220/smihandler.c @@ -102,6 +102,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */ diff --git a/src/mainboard/lenovo/x230/smihandler.c b/src/mainboard/lenovo/x230/smihandler.c index 2425927..a69b78f 100644 --- a/src/mainboard/lenovo/x230/smihandler.c +++ b/src/mainboard/lenovo/x230/smihandler.c @@ -72,6 +72,8 @@
void mainboard_smi_sleep(u8 slp_typ) { + h8_usb_always_on(); + if (slp_typ == 3) { u8 ec_wake = ec_read(0x32); /* If EC wake events are enabled, enable wake on EC WAKE GPE. */
Hello Alexander Couzens, Patrick Rudolph, Nathaniel Roach, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30905
to look at the new patch set (#2).
Change subject: Revert "lenovo/h8,thinkpads: Re-do USB Always On" ......................................................................
Revert "lenovo/h8,thinkpads: Re-do USB Always On"
This reverts commit 4f4322dd68575402e099e2dfda057687388f064e.
Reason for revert: According to bug #171 on coreboot's bug tracker: "It seems like commit 4f4322dd68575402e099e2dfda057687388f064e (which actually mentions this issue) was not enough to resolve this issue for me."
Change-Id: I05604102d12a9b3a8b4cbfafae3cf57dd66eb27c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/ec/lenovo/h8/Makefile.inc M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h A src/ec/lenovo/h8/smm.c M src/mainboard/lenovo/l520/smihandler.c M src/mainboard/lenovo/t420/smihandler.c M src/mainboard/lenovo/t420s/smihandler.c M src/mainboard/lenovo/t430/smihandler.c M src/mainboard/lenovo/t430s/smihandler.c M src/mainboard/lenovo/t520/smihandler.c M src/mainboard/lenovo/t530/smihandler.c M src/mainboard/lenovo/x201/cmos.layout M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/lenovo/x220/cmos.layout M src/mainboard/lenovo/x220/smihandler.c M src/mainboard/lenovo/x230/smihandler.c 16 files changed, 72 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/30905/2
Hello Alexander Couzens, Patrick Rudolph, Nathaniel Roach, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30905
to look at the new patch set (#4).
Change subject: Revert "lenovo/h8,thinkpads: Re-do USB Always On" ......................................................................
Revert "lenovo/h8,thinkpads: Re-do USB Always On"
This reverts commit 4f4322dd68575402e099e2dfda057687388f064e.
Reason for revert: According to bug #171 on coreboot's bug tracker: "It seems like commit 4f4322dd68575402e099e2dfda057687388f064e (which actually mentions this issue) was not enough to resolve this issue for me."
Change-Id: I05604102d12a9b3a8b4cbfafae3cf57dd66eb27c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/ec/lenovo/h8/Makefile.inc M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h A src/ec/lenovo/h8/smm.c M src/mainboard/lenovo/l520/smihandler.c M src/mainboard/lenovo/t420/smihandler.c M src/mainboard/lenovo/t420s/smihandler.c M src/mainboard/lenovo/t430/smihandler.c M src/mainboard/lenovo/t430s/smihandler.c M src/mainboard/lenovo/t520/smihandler.c M src/mainboard/lenovo/t530/smihandler.c M src/mainboard/lenovo/x201/cmos.layout M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/lenovo/x220/cmos.layout M src/mainboard/lenovo/x220/smihandler.c M src/mainboard/lenovo/x230/smihandler.c 16 files changed, 72 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/30905/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30905 )
Change subject: Revert "lenovo/h8,thinkpads: Re-do USB Always On" ......................................................................
Patch Set 4: Code-Review-1
The reverted commit was tested on real hardware and seem to work. -1 until tests on real hardware show that the original commit wasn't tested or is broken.
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30905 )
Change subject: Revert "lenovo/h8,thinkpads: Re-do USB Always On" ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)