Change in coreboot[master]: ec/lenovo/h8: Add a config to indicate whether dock init is needed
Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
ec/lenovo/h8: Add a config to indicate whether dock init is needed
Discussed in CB:36093, in the past many lenovo boards need to declare empty h8_mainboard_init_dock() to satisfy h8.c. Since H8_DOCK_EARLY_INIT has a different meaning, it had better add another config to indicate whether dock init is needed, thus eliminating the necessity to declare empty h8_mainboard_init_dock() in the future.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/Kconfig M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Kconfig M src/mainboard/lenovo/x200/Makefile.inc D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/Kconfig M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 21 files changed, 16 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/1
diff --git a/src/ec/lenovo/h8/Kconfig b/src/ec/lenovo/h8/Kconfig index f3df56a..5cc26d6 100644 --- a/src/ec/lenovo/h8/Kconfig +++ b/src/ec/lenovo/h8/Kconfig @@ -44,7 +44,14 @@ bool default n
-endif +config H8_NEED_DOCK_INIT + bool "Dock init during boot is necessary" + +if H8_NEED_DOCK_INIT
config H8_DOCK_EARLY_INIT bool + +endif # H8_NEED_DOCK_INIT + +endif # EC_LENOVO_H8 diff --git a/src/ec/lenovo/h8/h8.c b/src/ec/lenovo/h8/h8.c index 3a99b52..679b96c 100644 --- a/src/ec/lenovo/h8/h8.c +++ b/src/ec/lenovo/h8/h8.c @@ -354,7 +354,7 @@
h8_set_audio_mute(0);
-#if !CONFIG(H8_DOCK_EARLY_INIT) +#if CONFIG(H8_NEED_DOCK_INIT) && !CONFIG(H8_DOCK_EARLY_INIT) h8_mainboard_init_dock(); #endif } diff --git a/src/mainboard/lenovo/l520/mainboard.c b/src/mainboard/lenovo/l520/mainboard.c index db695b1..88c7884 100644 --- a/src/mainboard/lenovo/l520/mainboard.c +++ b/src/mainboard/lenovo/l520/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t400/Kconfig b/src/mainboard/lenovo/t400/Kconfig index 2caf1d3..171cb29 100644 --- a/src/mainboard/lenovo/t400/Kconfig +++ b/src/mainboard/lenovo/t400/Kconfig @@ -10,6 +10,7 @@ select EC_LENOVO_PMH7 select EC_LENOVO_H8 select H8_HAS_BAT_TRESHOLDS_IMPL + select H8_NEED_DOCK_INIT select H8_DOCK_EARLY_INIT select BOARD_ROMSIZE_KB_8192 if !BOARD_LENOVO_R500 select BOARD_ROMSIZE_KB_4096 if BOARD_LENOVO_R500 diff --git a/src/mainboard/lenovo/t410/Kconfig b/src/mainboard/lenovo/t410/Kconfig index 329d08d..600b8a6 100644 --- a/src/mainboard/lenovo/t410/Kconfig +++ b/src/mainboard/lenovo/t410/Kconfig @@ -21,6 +21,7 @@ select INTEL_GMA_HAVE_VBT select MAINBOARD_USES_IFD_GBE_REGION select H8_HAS_BAT_TRESHOLDS_IMPL + select H8_NEED_DOCK_INIT select MAINBOARD_HAS_LIBGFXINIT
config VBOOT diff --git a/src/mainboard/lenovo/t420/mainboard.c b/src/mainboard/lenovo/t420/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t420/mainboard.c +++ b/src/mainboard/lenovo/t420/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t420s/mainboard.c b/src/mainboard/lenovo/t420s/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t420s/mainboard.c +++ b/src/mainboard/lenovo/t420s/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t430/mainboard.c b/src/mainboard/lenovo/t430/mainboard.c index 99a5bd8..a86a90e 100644 --- a/src/mainboard/lenovo/t430/mainboard.c +++ b/src/mainboard/lenovo/t430/mainboard.c @@ -17,10 +17,6 @@ #include <drivers/intel/gma/int15.h> #include <ec/lenovo/h8/h8.h>
-void h8_mainboard_init_dock (void) -{ -} - static void mainboard_enable(struct device *dev) { install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, diff --git a/src/mainboard/lenovo/t430s/mainboard.c b/src/mainboard/lenovo/t430s/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t430s/mainboard.c +++ b/src/mainboard/lenovo/t430s/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t520/mainboard.c b/src/mainboard/lenovo/t520/mainboard.c index bd4292a..6825b6b 100644 --- a/src/mainboard/lenovo/t520/mainboard.c +++ b/src/mainboard/lenovo/t520/mainboard.c @@ -28,10 +28,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t530/mainboard.c b/src/mainboard/lenovo/t530/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/t530/mainboard.c +++ b/src/mainboard/lenovo/t530/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t60/Kconfig b/src/mainboard/lenovo/t60/Kconfig index bc8fa29..86351c8 100644 --- a/src/mainboard/lenovo/t60/Kconfig +++ b/src/mainboard/lenovo/t60/Kconfig @@ -19,6 +19,7 @@ select BOARD_ROMSIZE_KB_2048 select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME + select H8_NEED_DOCK_INIT select H8_DOCK_EARLY_INIT select HAVE_CMOS_DEFAULT select I945_LVDS diff --git a/src/mainboard/lenovo/x131e/mainboard.c b/src/mainboard/lenovo/x131e/mainboard.c index 1342aca..50b4de1 100644 --- a/src/mainboard/lenovo/x131e/mainboard.c +++ b/src/mainboard/lenovo/x131e/mainboard.c @@ -24,10 +24,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c b/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c index 64cc15b..bc6dcb1 100644 --- a/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c +++ b/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c @@ -29,8 +29,3 @@ struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; - -/* TODO: this device doesnt have a dock */ -void h8_mainboard_init_dock (void) -{ -} diff --git a/src/mainboard/lenovo/x200/Kconfig b/src/mainboard/lenovo/x200/Kconfig index fab8a88..8cab040 100644 --- a/src/mainboard/lenovo/x200/Kconfig +++ b/src/mainboard/lenovo/x200/Kconfig @@ -23,6 +23,7 @@ select DRIVERS_LENOVO_WACOM select MAINBOARD_HAS_LIBGFXINIT select MAINBOARD_USES_IFD_GBE_REGION + select H8_NEED_DOCK_INIT if BOARD_LENOVO_X200
config MAINBOARD_DIR string diff --git a/src/mainboard/lenovo/x200/Makefile.inc b/src/mainboard/lenovo/x200/Makefile.inc index f6c2c0c..7e38a78 100644 --- a/src/mainboard/lenovo/x200/Makefile.inc +++ b/src/mainboard/lenovo/x200/Makefile.inc @@ -13,7 +13,7 @@ ## GNU General Public License for more details. ##
-ramstage-y += variants/$(VARIANT_DIR)/dock.c +ramstage-$(CONFIG_BOARD_LENOVO_X200) += variants/$(VARIANT_DIR)/dock.c ramstage-y += cstates.c ramstage-y += blc.c romstage-y += variants/$(VARIANT_DIR)/gpio.c diff --git a/src/mainboard/lenovo/x200/variants/x301/dock.c b/src/mainboard/lenovo/x200/variants/x301/dock.c deleted file mode 100644 index f8a2dc4..0000000 --- a/src/mainboard/lenovo/x200/variants/x301/dock.c +++ /dev/null @@ -1,22 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2011 Sven Schnelle svens@stackframe.org - * Copyright (C) 2013 Vladimir Serbinenko phcoder@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 <ec/lenovo/h8/h8.h> - -void h8_mainboard_init_dock(void) -{ -} diff --git a/src/mainboard/lenovo/x201/Kconfig b/src/mainboard/lenovo/x201/Kconfig index e40c0d3..21edec79 100644 --- a/src/mainboard/lenovo/x201/Kconfig +++ b/src/mainboard/lenovo/x201/Kconfig @@ -22,6 +22,7 @@ select INTEL_GMA_HAVE_VBT select MAINBOARD_USES_IFD_GBE_REGION select H8_HAS_BAT_TRESHOLDS_IMPL + select H8_NEED_DOCK_INIT
config VBOOT select VBOOT_VBNV_CMOS diff --git a/src/mainboard/lenovo/x220/mainboard.c b/src/mainboard/lenovo/x220/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/x220/mainboard.c +++ b/src/mainboard/lenovo/x220/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x230/mainboard.c b/src/mainboard/lenovo/x230/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/x230/mainboard.c +++ b/src/mainboard/lenovo/x230/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x60/Kconfig b/src/mainboard/lenovo/x60/Kconfig index e3aabf3..27c953b 100644 --- a/src/mainboard/lenovo/x60/Kconfig +++ b/src/mainboard/lenovo/x60/Kconfig @@ -22,6 +22,7 @@ select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME select USE_OPTION_TABLE + select H8_NEED_DOCK_INIT select H8_DOCK_EARLY_INIT select DRIVERS_LENOVO_WACOM select I945_LVDS
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig@47 PS1, Line 47: H8_NEED_DOCK_INIT Maybe it's not worth to overcomplicate things. What about H8_DOCK_INIT and H8_EARLY_DOCK_INIT and make them mutually exclusive in Kconfig (depend !...)
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig@48 PS1, Line 48: "Dock init during boot is necessary" This is a property of the board. Please don't make it user selectable.
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#2).
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
ec/lenovo/h8: Add a config to indicate whether dock init is needed
Discussed in CB:36093, in the past many lenovo boards need to declare empty h8_mainboard_init_dock() to satisfy h8.c. Since H8_DOCK_EARLY_INIT has a different meaning, it had better add another config to indicate whether dock init is needed, thus eliminating the necessity to declare empty h8_mainboard_init_dock() in the future.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t410/Kconfig M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Kconfig M src/mainboard/lenovo/x200/Makefile.inc D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/Kconfig M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c 18 files changed, 10 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/2
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig@47 PS1, Line 47: H8_NEED_DOCK_INIT
Maybe it's not worth to overcomplicate things. […]
Done
https://review.coreboot.org/c/coreboot/+/36385/1/src/ec/lenovo/h8/Kconfig@48 PS1, Line 48: "Dock init during boot is necessary"
This is a property of the board. Please don't make it user selectable.
Done
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT) I'd recommand to remove this code and just call h8_mainboard_init_dock in the mainboard chip_ops .enable_dev function instead. That way there's no need for a new Kconfig.
That needs test on all boards that use h8_mainboard_init_dock, as the previous code might need to be run first.
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
I'd recommand to remove this code and just call h8_mainboard_init_dock in the mainboard chip_ops . […]
I would rather keep this scheme, as I have no condition to test all boards that use h8_mainboard_init_dock(), and the current scheme is indifferent with the old one for those using h8_mainboard_init_dock() at runtime, so it certainly works.
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
I would rather keep this scheme, as I have no condition to test all boards that use h8_mainboard_ini […]
The H8_DOCK_INIT could be treated as transitional, eliminating empty h8_mainboard_init_dock() declarations first, and migrate to your scheme board by board, just as C_ENVIRONMENT_BOOTBLOCK.
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#3).
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
ec/lenovo/h8: Add a config to indicate whether dock init is needed
Discussed in CB:36093, in the past many lenovo boards need to declare empty h8_mainboard_init_dock() to satisfy h8.c. Since H8_DOCK_EARLY_INIT has a different meaning, it had better add another config to indicate whether dock init is needed, thus eliminating the necessity to declare empty h8_mainboard_init_dock() in the future.
The introduction of H8_DOCK_INIT may be transitional. Boards needing h8_mainboard_init_dock() may call it in the mainboard chip_ops.enable_dev function in future commits, while the timing between h8_mainboard_init_dock() and h8_enable() should be considered.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t410/Kconfig M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Kconfig M src/mainboard/lenovo/x200/Makefile.inc D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/Kconfig M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c 18 files changed, 10 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG@18 PS3, Line 18: between h8_mainboard_init_dock() and h8_enable() should be considered. Mainboard ops precede all others, so that won't work.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@47 PS3, Line 47: config H8_DOCK_INIT This name seems too generic. Not selecting it makes it look like there is no dock init at all. Maybe call it `H8_DOCK_LATE_INIT`?
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@50 PS3, Line 50: config H8_DOCK_EARLY_INIT This is not used anymore.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c@357 PS3, Line 357: #if CONFIG(H8_DOCK_INIT) Please use a C `if (CONFIG(H8_DOCK_INIT))`. Garbage collection takes care of it before linking, so there will be no error.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Add a config to indicate whether dock init is needed ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG@18 PS3, Line 18: between h8_mainboard_init_dock() and h8_enable() should be considered.
Mainboard ops precede all others, so that won't work.
What would be the issue in doing so?
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
The H8_DOCK_INIT could be treated as transitional, eliminating empty h8_mainboard_init_dock() declar […]
Looking at boards that need dock init here (x200, x201, t410) I don't think it matters if it's done in before h8_enable. After all doing it early on some boards where dock init is even more involving (LPC to LPC bridge) works, so...
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#4).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/mainboard.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 21 files changed, 7 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36385/4/src/mainboard/lenovo/x200/m... File src/mainboard/lenovo/x200/mainboard.c:
https://review.coreboot.org/c/coreboot/+/36385/4/src/mainboard/lenovo/x200/m... PS4, Line 32: if(CONFIG(BOARD_LENOVO_X200)) space required before the open parenthesis '('
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#5).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/mainboard.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 21 files changed, 7 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/5
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36385/3//COMMIT_MSG@18 PS3, Line 18: between h8_mainboard_init_dock() and h8_enable() should be considered.
Mainboard ops precede all others, so that won't work. […]
Refactored.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@47 PS3, Line 47: config H8_DOCK_INIT
This name seems too generic. Not selecting it makes it look like there is […]
It may be better to retire the confusing H8_DOCK_EARLY_INIT and apply Patrick's idea, if Arther's statement is true.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/Kconfig@50 PS3, Line 50: config H8_DOCK_EARLY_INIT
This is not used anymore.
Done
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
Looking at boards that need dock init here (x200, x201, t410) I don't think it matters if it's done […]
Okay. I am going to implement the idea of Patrick Rudolph directly if the timing between h8_mainboard_init_dock() and h8_enable() does not matter.
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/3/src/ec/lenovo/h8/h8.c@357 PS3, Line 357: #if CONFIG(H8_DOCK_INIT)
Please use a C `if (CONFIG(H8_DOCK_INIT))`. […]
Refactored
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c File src/ec/lenovo/h8/h8.c:
https://review.coreboot.org/c/coreboot/+/36385/2/src/ec/lenovo/h8/h8.c@357 PS2, Line 357: #if CONFIG(H8_DOCK_INIT)
Okay. […]
Could you guys help me to test it e.g. on x201 and t410?
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#6).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/dock.c M src/mainboard/lenovo/t410/dock.h M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/dock.h M src/mainboard/lenovo/x200/mainboard.c M src/mainboard/lenovo/x200/variants/x200/dock.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/dock.c M src/mainboard/lenovo/x201/dock.h M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 28 files changed, 15 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/6
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#7).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/dock.c M src/mainboard/lenovo/t410/dock.h M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/dock.h M src/mainboard/lenovo/x200/mainboard.c M src/mainboard/lenovo/x200/variants/x200/dock.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/dock.c M src/mainboard/lenovo/x201/dock.h M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 28 files changed, 15 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/7
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#8).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Tested on X200. Testing on other affected targets may be necessary.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/dock.c M src/mainboard/lenovo/t410/dock.h M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/dock.h M src/mainboard/lenovo/x200/mainboard.c M src/mainboard/lenovo/x200/variants/x200/dock.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/dock.c M src/mainboard/lenovo/x201/dock.h M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 28 files changed, 15 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/8
Hello Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36385
to look at the new patch set (#9).
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Tested on X200. Testing on other affected targets may be necessary.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/dock.c M src/mainboard/lenovo/t410/dock.h M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t440p/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/dock.h M src/mainboard/lenovo/x200/mainboard.c M src/mainboard/lenovo/x200/variants/x200/dock.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/dock.c M src/mainboard/lenovo/x201/dock.h M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 29 files changed, 15 insertions(+), 91 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/36385/9
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 9: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific
Discussed in CB:36093, in the past many lenovo boards need to declare an empty h8_mainboard_init_dock() to satisfy h8.c.
Now the confusing H8_DOCK_EARLY_INIT might be retired, and if a mainboard needs dock init (done with h8_mainboard_init_dock() in the past) in ramstage, (discussed in CB:4294 where H8_DOCK_EARLY_INIT is introduced) it can just do it in its own chip_ops.enable_dev function.
Tested on X200. Testing on other affected targets may be necessary.
Change-Id: I5737406d1f6cb6e91b2e2fa349a206a3dba988d1 Signed-off-by: Bill XIE persmule@hardenedlinux.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/36385 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/h8.c M src/ec/lenovo/h8/h8.h M src/mainboard/lenovo/l520/mainboard.c M src/mainboard/lenovo/t400/Kconfig M src/mainboard/lenovo/t410/dock.c M src/mainboard/lenovo/t410/dock.h M src/mainboard/lenovo/t410/mainboard.c M src/mainboard/lenovo/t420/mainboard.c M src/mainboard/lenovo/t420s/mainboard.c M src/mainboard/lenovo/t430/mainboard.c M src/mainboard/lenovo/t430s/mainboard.c M src/mainboard/lenovo/t440p/mainboard.c M src/mainboard/lenovo/t520/mainboard.c M src/mainboard/lenovo/t530/mainboard.c M src/mainboard/lenovo/t60/Kconfig M src/mainboard/lenovo/x131e/mainboard.c M src/mainboard/lenovo/x1_carbon_gen1/mainboard.c M src/mainboard/lenovo/x200/Makefile.inc M src/mainboard/lenovo/x200/dock.h M src/mainboard/lenovo/x200/mainboard.c M src/mainboard/lenovo/x200/variants/x200/dock.c D src/mainboard/lenovo/x200/variants/x301/dock.c M src/mainboard/lenovo/x201/dock.c M src/mainboard/lenovo/x201/dock.h M src/mainboard/lenovo/x201/mainboard.c M src/mainboard/lenovo/x220/mainboard.c M src/mainboard/lenovo/x230/mainboard.c M src/mainboard/lenovo/x60/Kconfig 29 files changed, 15 insertions(+), 91 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/ec/lenovo/h8/Kconfig b/src/ec/lenovo/h8/Kconfig index f3df56a..6bd290f 100644 --- a/src/ec/lenovo/h8/Kconfig +++ b/src/ec/lenovo/h8/Kconfig @@ -44,7 +44,4 @@ bool default n
-endif - -config H8_DOCK_EARLY_INIT - bool +endif # EC_LENOVO_H8 diff --git a/src/ec/lenovo/h8/h8.c b/src/ec/lenovo/h8/h8.c index 3a99b52..93a771c 100644 --- a/src/ec/lenovo/h8/h8.c +++ b/src/ec/lenovo/h8/h8.c @@ -353,10 +353,6 @@ h8_charge_priority(val);
h8_set_audio_mute(0); - -#if !CONFIG(H8_DOCK_EARLY_INIT) - h8_mainboard_init_dock(); -#endif }
struct chip_operations ec_lenovo_h8_ops = { diff --git a/src/ec/lenovo/h8/h8.h b/src/ec/lenovo/h8/h8.h index 14948c5..6dad288 100644 --- a/src/ec/lenovo/h8/h8.h +++ b/src/ec/lenovo/h8/h8.h @@ -36,8 +36,6 @@ u8 h8_build_id_and_function_spec_version(char *buf, u8 buf_len); void h8_usb_always_on(void);
-void h8_mainboard_init_dock (void); - int h8_get_fn_key(void); int h8_get_sense_ready(void);
diff --git a/src/mainboard/lenovo/l520/mainboard.c b/src/mainboard/lenovo/l520/mainboard.c index db695b1..88c7884 100644 --- a/src/mainboard/lenovo/l520/mainboard.c +++ b/src/mainboard/lenovo/l520/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t400/Kconfig b/src/mainboard/lenovo/t400/Kconfig index 8d286c4..a68d1fa 100644 --- a/src/mainboard/lenovo/t400/Kconfig +++ b/src/mainboard/lenovo/t400/Kconfig @@ -10,7 +10,6 @@ select EC_LENOVO_PMH7 select EC_LENOVO_H8 select H8_HAS_BAT_TRESHOLDS_IMPL - select H8_DOCK_EARLY_INIT select BOARD_ROMSIZE_KB_8192 if !BOARD_LENOVO_R500 select BOARD_ROMSIZE_KB_4096 if BOARD_LENOVO_R500 select DRIVERS_GENERIC_IOAPIC diff --git a/src/mainboard/lenovo/t410/dock.c b/src/mainboard/lenovo/t410/dock.c index 317fb05..1575aa1 100644 --- a/src/mainboard/lenovo/t410/dock.c +++ b/src/mainboard/lenovo/t410/dock.c @@ -22,7 +22,7 @@ #include <ec/lenovo/h8/h8.h> #include <ec/acpi/ec.h>
-void h8_mainboard_init_dock(void) +void init_dock(void) { if (dock_present()) { printk(BIOS_DEBUG, "dock is connected\n"); diff --git a/src/mainboard/lenovo/t410/dock.h b/src/mainboard/lenovo/t410/dock.h index 4cd8c85..6a08d81 100644 --- a/src/mainboard/lenovo/t410/dock.h +++ b/src/mainboard/lenovo/t410/dock.h @@ -15,7 +15,7 @@
#ifndef THINKPAD_X201_DOCK_H #define THINKPAD_X201_DOCK_H - +void init_dock(void); void dock_connect(void); void dock_disconnect(void); int dock_present(void); diff --git a/src/mainboard/lenovo/t410/mainboard.c b/src/mainboard/lenovo/t410/mainboard.c index 23b68fa..8b6a737 100644 --- a/src/mainboard/lenovo/t410/mainboard.c +++ b/src/mainboard/lenovo/t410/mainboard.c @@ -19,12 +19,14 @@ #include <device/device.h> #include <arch/acpi.h> #include <drivers/intel/gma/int15.h> +#include "dock.h"
static void mainboard_enable(struct device *dev) { install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, GMA_INT15_PANEL_FIT_DEFAULT, GMA_INT15_BOOT_DISPLAY_LFP, 2); + init_dock(); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/lenovo/t420/mainboard.c b/src/mainboard/lenovo/t420/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t420/mainboard.c +++ b/src/mainboard/lenovo/t420/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t420s/mainboard.c b/src/mainboard/lenovo/t420s/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t420s/mainboard.c +++ b/src/mainboard/lenovo/t420s/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t430/mainboard.c b/src/mainboard/lenovo/t430/mainboard.c index 99a5bd8..a86a90e 100644 --- a/src/mainboard/lenovo/t430/mainboard.c +++ b/src/mainboard/lenovo/t430/mainboard.c @@ -17,10 +17,6 @@ #include <drivers/intel/gma/int15.h> #include <ec/lenovo/h8/h8.h>
-void h8_mainboard_init_dock (void) -{ -} - static void mainboard_enable(struct device *dev) { install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, diff --git a/src/mainboard/lenovo/t430s/mainboard.c b/src/mainboard/lenovo/t430s/mainboard.c index 6c85aba..bc6dcb1 100644 --- a/src/mainboard/lenovo/t430s/mainboard.c +++ b/src/mainboard/lenovo/t430s/mainboard.c @@ -26,10 +26,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t440p/mainboard.c b/src/mainboard/lenovo/t440p/mainboard.c index dcfd503..0881c2d 100644 --- a/src/mainboard/lenovo/t440p/mainboard.c +++ b/src/mainboard/lenovo/t440p/mainboard.c @@ -25,10 +25,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t520/mainboard.c b/src/mainboard/lenovo/t520/mainboard.c index bd4292a..6825b6b 100644 --- a/src/mainboard/lenovo/t520/mainboard.c +++ b/src/mainboard/lenovo/t520/mainboard.c @@ -28,10 +28,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t530/mainboard.c b/src/mainboard/lenovo/t530/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/t530/mainboard.c +++ b/src/mainboard/lenovo/t530/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/t60/Kconfig b/src/mainboard/lenovo/t60/Kconfig index bc8fa29..436b035 100644 --- a/src/mainboard/lenovo/t60/Kconfig +++ b/src/mainboard/lenovo/t60/Kconfig @@ -19,7 +19,6 @@ select BOARD_ROMSIZE_KB_2048 select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME - select H8_DOCK_EARLY_INIT select HAVE_CMOS_DEFAULT select I945_LVDS select INTEL_GMA_HAVE_VBT diff --git a/src/mainboard/lenovo/x131e/mainboard.c b/src/mainboard/lenovo/x131e/mainboard.c index 1342aca..50b4de1 100644 --- a/src/mainboard/lenovo/x131e/mainboard.c +++ b/src/mainboard/lenovo/x131e/mainboard.c @@ -24,10 +24,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock(void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c b/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c index 64cc15b..bc6dcb1 100644 --- a/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c +++ b/src/mainboard/lenovo/x1_carbon_gen1/mainboard.c @@ -29,8 +29,3 @@ struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; - -/* TODO: this device doesnt have a dock */ -void h8_mainboard_init_dock (void) -{ -} diff --git a/src/mainboard/lenovo/x200/Makefile.inc b/src/mainboard/lenovo/x200/Makefile.inc index f6c2c0c..7e38a78 100644 --- a/src/mainboard/lenovo/x200/Makefile.inc +++ b/src/mainboard/lenovo/x200/Makefile.inc @@ -13,7 +13,7 @@ ## GNU General Public License for more details. ##
-ramstage-y += variants/$(VARIANT_DIR)/dock.c +ramstage-$(CONFIG_BOARD_LENOVO_X200) += variants/$(VARIANT_DIR)/dock.c ramstage-y += cstates.c ramstage-y += blc.c romstage-y += variants/$(VARIANT_DIR)/gpio.c diff --git a/src/mainboard/lenovo/x200/dock.h b/src/mainboard/lenovo/x200/dock.h index 56f3fe0..a129cd0 100644 --- a/src/mainboard/lenovo/x200/dock.h +++ b/src/mainboard/lenovo/x200/dock.h @@ -15,7 +15,7 @@
#ifndef THINKPAD_X200_DOCK_H #define THINKPAD_X200_DOCK_H - +void init_dock(void); void dock_connect(void); void dock_disconnect(void); int dock_present(void); diff --git a/src/mainboard/lenovo/x200/mainboard.c b/src/mainboard/lenovo/x200/mainboard.c index 1510ab7..37fe865 100644 --- a/src/mainboard/lenovo/x200/mainboard.c +++ b/src/mainboard/lenovo/x200/mainboard.c @@ -16,6 +16,7 @@ #include <device/device.h> #include <drivers/intel/gma/int15.h> #include <drivers/lenovo/lenovo.h> +#include "dock.h"
static void fill_ssdt(struct device *device) { @@ -29,6 +30,8 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 2);
dev->ops->acpi_fill_ssdt_generator = fill_ssdt; + if (CONFIG(BOARD_LENOVO_X200)) + init_dock(); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/lenovo/x200/variants/x200/dock.c b/src/mainboard/lenovo/x200/variants/x200/dock.c index bdd65a5..8aa39bb 100644 --- a/src/mainboard/lenovo/x200/variants/x200/dock.c +++ b/src/mainboard/lenovo/x200/variants/x200/dock.c @@ -23,7 +23,7 @@
#include "../../dock.h"
-void h8_mainboard_init_dock(void) +void init_dock(void) { if (dock_present()) { printk(BIOS_DEBUG, "dock is connected\n"); diff --git a/src/mainboard/lenovo/x200/variants/x301/dock.c b/src/mainboard/lenovo/x200/variants/x301/dock.c deleted file mode 100644 index f8a2dc4..0000000 --- a/src/mainboard/lenovo/x200/variants/x301/dock.c +++ /dev/null @@ -1,22 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2011 Sven Schnelle svens@stackframe.org - * Copyright (C) 2013 Vladimir Serbinenko phcoder@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 <ec/lenovo/h8/h8.h> - -void h8_mainboard_init_dock(void) -{ -} diff --git a/src/mainboard/lenovo/x201/dock.c b/src/mainboard/lenovo/x201/dock.c index 652a144..58510ce 100644 --- a/src/mainboard/lenovo/x201/dock.c +++ b/src/mainboard/lenovo/x201/dock.c @@ -22,7 +22,7 @@ #include <ec/lenovo/h8/h8.h> #include <ec/acpi/ec.h>
-void h8_mainboard_init_dock(void) +void init_dock(void) { if (dock_present()) { printk(BIOS_DEBUG, "dock is connected\n"); diff --git a/src/mainboard/lenovo/x201/dock.h b/src/mainboard/lenovo/x201/dock.h index 4cd8c85..6a08d81 100644 --- a/src/mainboard/lenovo/x201/dock.h +++ b/src/mainboard/lenovo/x201/dock.h @@ -15,7 +15,7 @@
#ifndef THINKPAD_X201_DOCK_H #define THINKPAD_X201_DOCK_H - +void init_dock(void); void dock_connect(void); void dock_disconnect(void); int dock_present(void); diff --git a/src/mainboard/lenovo/x201/mainboard.c b/src/mainboard/lenovo/x201/mainboard.c index c021db1..a403237 100644 --- a/src/mainboard/lenovo/x201/mainboard.c +++ b/src/mainboard/lenovo/x201/mainboard.c @@ -42,6 +42,8 @@ install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS, GMA_INT15_PANEL_FIT_DEFAULT, GMA_INT15_BOOT_DISPLAY_LFP, 2); + + init_dock(); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/lenovo/x220/mainboard.c b/src/mainboard/lenovo/x220/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/x220/mainboard.c +++ b/src/mainboard/lenovo/x220/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x230/mainboard.c b/src/mainboard/lenovo/x230/mainboard.c index 29c15e0..144fde7 100644 --- a/src/mainboard/lenovo/x230/mainboard.c +++ b/src/mainboard/lenovo/x230/mainboard.c @@ -29,10 +29,6 @@ GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); }
-void h8_mainboard_init_dock (void) -{ -} - struct chip_operations mainboard_ops = { .enable_dev = mainboard_enable, }; diff --git a/src/mainboard/lenovo/x60/Kconfig b/src/mainboard/lenovo/x60/Kconfig index e3aabf3..a1a5fec 100644 --- a/src/mainboard/lenovo/x60/Kconfig +++ b/src/mainboard/lenovo/x60/Kconfig @@ -22,7 +22,6 @@ select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME select USE_OPTION_TABLE - select H8_DOCK_EARLY_INIT select DRIVERS_LENOVO_WACOM select I945_LVDS select INTEL_GMA_HAVE_VBT
Stefan Ott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 10:
This breaks docking on the X201: The dock is no longer detected after resume or on power-up, docking only works when the machine is running (see https://ticket.coreboot.org/issues/256)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 10:
This commit caused a regression on the Lenovo X201.
https://ticket.coreboot.org/issues/256
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 10:
Patch Set 10:
This commit caused a regression on the Lenovo X201.
What is the cause of this? Is init_dock() called before h8 is enabled?
Stefan Ott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 10:
Patch Set 10:
Patch Set 10:
This commit caused a regression on the Lenovo X201.
What is the cause of this? Is init_dock() called before h8 is enabled?
Probably. If I understand things right, mainboard_enable is called quite early during wakeup, whereas the h8 stuff happens much later (assuming that the order in the device tree is relevant and also assuming that I'm reading it correctly).
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36385 )
Change subject: ec/lenovo/h8: Make dock init in ramstage fully mainboard-specific ......................................................................
Patch Set 10:
Patch Set 10:
Patch Set 10:
Patch Set 10:
This commit caused a regression on the Lenovo X201.
What is the cause of this? Is init_dock() called before h8 is enabled?
Probably. If I understand things right, mainboard_enable is called quite early during wakeup, whereas the h8 stuff happens much later (assuming that the order in the device tree is relevant and also assuming that I'm reading it correctly).
You could try CB:39708 after it passes basic build tests.
-
Arthur Heymans (Code Review) -
Bill XIE (Code Review)
-
build bot (Jenkins) (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Stefan Ott (Code Review)