Hello Jes Klinke,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to review the following change.
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org Bug: b:154333137 --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 3 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/1
diff --git a/src/mainboard/google/volteer/Kconfig b/src/mainboard/google/volteer/Kconfig index 82edb82..8eedca1 100644 --- a/src/mainboard/google/volteer/Kconfig +++ b/src/mainboard/google/volteer/Kconfig @@ -58,6 +58,10 @@ config DRIVER_TPM_SPI_BUS default 0x1
+config CR50_USE_LONG_INTERRUPT_PULSES + bool + default y + config MAINBOARD_DIR string default "google/volteer" diff --git a/src/mainboard/google/volteer/chromeos.c b/src/mainboard/google/volteer/chromeos.c index abd50c5..8f43071 100644 --- a/src/mainboard/google/volteer/chromeos.c +++ b/src/mainboard/google/volteer/chromeos.c @@ -2,7 +2,10 @@
#include <baseboard/variants.h> #include <boot/coreboot_tables.h> +#include <drivers/spi/tpm/tpm.h> #include <gpio.h> +#include <security/tpm/tss.h> +#include <soc/ramstage.h> #include <variant/gpio.h> #include <vendorcode/google/chromeos/chromeos.h>
@@ -32,3 +35,18 @@ gpios = variant_cros_gpios(&num); chromeos_acpi_gpio_generate(gpios, num); } + +void mainboard_silicon_init_params(FSP_S_CONFIG *params) +{ + tlcl_lib_init(); + if (get_cr50_board_cfg() & 0x01) { + printk(BIOS_INFO, "Enabling S0i3.4\n"); + } else { + /* Disable S03.4, preventing the GPIO block from switching to + * slow clock. */ + printk(BIOS_INFO, "Not enabling S0i3.4\n"); + params->LpmStateEnableMask &= ~0x80; + } +} + + diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 53bbe5a..6004c1d 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -203,6 +203,7 @@
# Enable S0ix register "s0ix_enable" = "1" + register "LpmStateEnableMask" = "0xFF"
# Enable DPTF register "dptf_enable" = "1"
Hello build bot (Jenkins), Jes Klinke,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#2).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org Bug: b:154333137 --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 3 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/2
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 2:
Factored out from CB:43741
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 2:
Wouldn't it make more sense to put all this with the SoC code (guarded by TPM_CR50)?
Hello build bot (Jenkins), Patrick Georgi, Jes Klinke, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#3).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org Bug: b:154333137 --- M src/drivers/spi/tpm/tpm.c M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 4 files changed, 25 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/3
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#4).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org Bug: b:154333137 --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 3 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/4
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
Wouldn't it make more sense to put all this with the SoC code (guarded by TPM_CR50)?
Possibly. I assume that you mean to put some conditionals into soc/intel/tigerlake/Kconfig, such that if TPM_CR50 is enabled, then it will also enable CR50_USE_LONG_INTERRUPT_PULSES. Besides, there will be some C code to set LpmStateEnableMask depending on the Cr50 response.
Among Tiger Lake devices, I can see three categories: 1) Devices without Cr50 (likely not Chromebooks) 2) Devices on which all other interrupt sources beside Cr50 guarantee 100us pulses 3) Devices on which at least one other interrupt source generates pulses shorter than 100us
Putting some code in soc/intel/tigerlake guarded by TPM_CR50 will nicely separate category 1 from the other two. However, we need some mainboard declarations in order to distinguish between categories 2 and 3.
I am not familiar with the exact precedence of Kconfig, but assuming that mainboard/ defaults take precedence over soc/, and soc/ takes precendence over drivers/, then I suppose that category 3 could be handled by the mainboard declarations setting CR50_USE_LONG_INTERRUPT_PULSES to n (overriding the conditional logic in soc/tigerlake), and also claring bit7 in LpmStateEnableMask.
In this proposal, the C code in soc/intel/tigerlake should be guarded by CR50_USE_LONG_INTERRUPT_PULSES, rather than TPM_CR50.
Do you think that is preferrable to the current patchset?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
Putting some code in soc/intel/tigerlake guarded by TPM_CR50 will nicely separate category 1 from the other two. However, we need some mainboard declarations in order to distinguish between categories 2 and 3.
Okay, fair enough, that's a reasonable argument. I'll let the x86 guys chime in on where they think this should be. Either option sounds reasonable to me.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
(7 comments)
I don't think we need to worry about category 3 for now. All peripherals are expected to meet the 100us requirement. Considering that, I think we can directly compare TPM_CR50 and CR50_USE_LONG_INTERRUPT_PULSES to decide if LpmStateEnableMask should be updated.
https://review.coreboot.org/c/coreboot/+/44359/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/4//COMMIT_MSG@20 PS4, Line 20: Bug: b:154333137 Move before Change-Id above. Also, BUG=
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 61: config CR50_USE_LONG_INTERRUPT_PULSES : bool : default y You can just do a "select CR50_USE_LONG_INTERRUPT_PULSES" under BOARD_GOOGLE_BASEBOARD_VOLTEER
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 12: #define MIN_INTERRUPT_PULSE_LENGTH_US_FOR_S0i3_4 100 In my opinion, we don't really need this. Instead using a bool should be sufficient for checking if cr50 supports long pulses.
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 43: tlcl_lib_init(); I think this should be added to the function `get_cr50_ready_pulse_length_us()` or whatever the name is if you change it to bool return type. It is a must when making the call to cr50. So, instead of having each mainboard/SoC do it, I think it is better to have the TPM driver code do this.
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 45: >= MIN_INTERRUPT_PULSE_LENGTH_US_FOR_S0i3_4) { I believe this can go on the previous line within the 96-column limit.
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 48: S03.4 S0i3.4
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 206: register "LpmStateEnableMask" = "0xFF" Add a comment indicating all S0ix substates are supported.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 41: mainboard_silicon_init_params I would recommend moving this to mainboard_chip_init() in mainboard.c: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
Basically add a helper function variant_update_devtree() and then use something like:
``` struct soc_intel_tigerlake_config *cfg = config_of_soc(); tlcl_lib_init(); if (cr50_supports_long_pulses()) cfg->LpmStateDisableMask = LPM_S0i3_4; ```
This ensures that the soc config gets updated by mainboard before SoC sets the FSP UPD.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 61: config CR50_USE_LONG_INTERRUPT_PULSES : bool : default y
You can just do a "select CR50_USE_LONG_INTERRUPT_PULSES" under BOARD_GOOGLE_BASEBOARD_VOLTEER
Actually, I am starting to think that this Kconfig setting belongs in soc/tigerlake. That is, if the SoC is Tiger Lake, and TPM_CR50 is selected, then we should autumatically also select CR50_USE_LONG_INTERRUPT_PULSES. I have seen config clauses declaring that the default should depend on some other setting, but I do not know if such a default in the soc directory would properly override the default "n" from drivers.
You may say that it is odd to have the Kconfig in soc/tigerlake, but the other logic in mainboard, but I see it as this version-detection logic is only for a "transition" phase, until our Cr50 from the factory have new enough firmware, new Tiger Lake designs coming out after that will only need CR50_USE_LONG_INTERRUPT_PULSES to be selected, no additional code in mainboard. (It may well be that we stop making Tiger Lake designs before we update the factory image of Cr50, but even then, we will have new similar "lakes" in the future, which could benefit from copying a CR50_USE_LONG_INTERRUPT_PULSES setting from soc/tigerlake.)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 48: S03.4
S0i3. […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 61: config CR50_USE_LONG_INTERRUPT_PULSES : bool : default y
Actually, I am starting to think that this Kconfig setting belongs in soc/tigerlake. That is, if the SoC is Tiger Lake, and TPM_CR50 is selected, then we should autumatically also select CR50_USE_LONG_INTERRUPT_PULSES.
That sounds fine. You can do it under "config CHROMEOS" in tigerlake.
I have seen config clauses declaring that the default should depend on some other setting, but I do not know if such a default in the soc directory would properly override the default "n" from drivers.
That should work fine. soc/*/Kconfig gets sourced before the drivers. So, it should do the right thing.
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#5).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/5
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 5:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44359/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/4//COMMIT_MSG@20 PS4, Line 20: Bug: b:154333137
Move before Change-Id above. […]
Done
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 61: config CR50_USE_LONG_INTERRUPT_PULSES : bool : default y
Actually, I am starting to think that this Kconfig setting belongs in soc/tigerlake. […]
Done
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 12: #define MIN_INTERRUPT_PULSE_LENGTH_US_FOR_S0i3_4 100
In my opinion, we don't really need this. […]
Done
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 41: mainboard_silicon_init_params
I would recommend moving this to mainboard_chip_init() in mainboard.c: https://source.chromium. […]
Done
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 43: tlcl_lib_init();
I think this should be added to the function `get_cr50_ready_pulse_length_us()` or whatever the name […]
I think we had a discussion elsewhere. The call to tlcl_lib_init() cannot be put into get_cr50_ready_pulse_length_us(), but we could in theory make a new helper function, somewhere under a /google/ directory, but shared between mainboards, which would do tlcl_lib_init() followed by get_cr50_ready_pulse_length_us() , or maybe even better, a function that does all the logic that I am currently adding here, to be shared between future Intel mainboards.
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 45: >= MIN_INTERRUPT_PULSE_LENGTH_US_FOR_S0i3_4) {
I believe this can go on the previous line within the 96-column limit.
Ack
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 206: register "LpmStateEnableMask" = "0xFF"
Add a comment indicating all S0ix substates are supported.
With the new convention of a bit being 1 meaning that the corresponding substate is DISabled, there is no need to add anything to this file, for Volteer or future Tiger Lake devices.
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#6).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/chromeos.c M src/mainboard/google/volteer/mainboard.c M src/soc/intel/tigerlake/Kconfig 4 files changed, 24 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/6
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#7).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/Kconfig M src/mainboard/google/volteer/mainboard.c M src/soc/intel/tigerlake/Kconfig 3 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/7
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#8).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/soc/intel/tigerlake/Kconfig 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/8
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... PS8, Line 40: tlcl_lib_init(); : if (get_cr50_long_interrupt_pulses()) { : printk(BIOS_INFO, "Enabling S0i3.4\n"); : } else { : /* Disable S03.4, preventing the GPIO block from switching to : * slow clock. */ : printk(BIOS_INFO, "Not enabling S0i3.4\n"); : cfg->LpmStateDisableMask = LPM_S0i3_4; : } nit: Put this in a helper function just to logically keep all changes together. mainboard_update_s0ix_disable_mask().
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
PS8: Can you please push this in a separate patch? We typically separate out SoC changes from mainboard changes.
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... PS8, Line 173: config A comment here would be very helpful as to why CR50_USE_LONG_INTERRUPT_PULSES is being selected.
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#9).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/9
Hello build bot (Jenkins), Patrick Georgi, Christian Walter, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#10).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/10
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 43: tlcl_lib_init();
I think we had a discussion elsewhere. […]
Ack
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/8/src/mainboard/google/voltee... PS8, Line 40: tlcl_lib_init(); : if (get_cr50_long_interrupt_pulses()) { : printk(BIOS_INFO, "Enabling S0i3.4\n"); : } else { : /* Disable S03.4, preventing the GPIO block from switching to : * slow clock. */ : printk(BIOS_INFO, "Not enabling S0i3.4\n"); : cfg->LpmStateDisableMask = LPM_S0i3_4; : }
nit: Put this in a helper function just to logically keep all changes together. […]
Done.
In time, I think that this mainboard_update_s0ix_disable_mask() should be moved into some common place, from which other board using Tiger Lake, or future Intel designs, can call it. (Until such time when our cr50 chips ship with new enough firmware that we can count on the long pulse support to always be there, at which point we no longer need to runtime branching here.)
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
PS8:
Can you please push this in a separate patch? We typically separate out SoC changes from mainboard c […]
I have created: https://review.coreboot.org/c/coreboot/+/44626
https://review.coreboot.org/c/coreboot/+/44359/8/src/soc/intel/tigerlake/Kco... PS8, Line 173: config
A comment here would be very helpful as to why CR50_USE_LONG_INTERRUPT_PULSES is being selected.
Done (in the separate changelist, linked above).
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... PS10, Line 37: cfg->LpmStateDisableMask = LPM_S0i3_4; I think you will also have to handle gpio_pm_config here:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
i.e. by default those should be set to no override i.e. gpio_override_pm = "0" and here along with disabling of LPM_S0i3_4, you will also need to set gpio_override_pm = 1 and gpio_pm[0..4] to 0.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 10:
(1 comment)
Subrata, please let us know if we need to do more than clearing bit7 of the Lpm enable register, in order to allow Tiger Lake to recognize 4us interrupt pulses.
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... PS10, Line 37: cfg->LpmStateDisableMask = LPM_S0i3_4;
I think you will also have to handle gpio_pm_config here: […]
I have not heard the Intel engineers mention anything besides the Lpm bit7 being cleared, as necessary for Tiger Lake to recognize interrupt pulses as short as 4us.
Subrata, can you please confirm?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... PS10, Line 37: cfg->LpmStateDisableMask = LPM_S0i3_4;
I have not heard the Intel engineers mention anything besides the Lpm bit7 being cleared, as necessa […]
Ideally if u have implemented long TPM IRQ width then you can keep those GPIO advance PM configuration enable and don't need any override as Furquan has mentioned else better u set override_pm=1 and make bit4:0 all 0
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#11).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/11
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#12).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/12
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/10/src/mainboard/google/volte... PS10, Line 37: cfg->LpmStateDisableMask = LPM_S0i3_4;
Ideally if u have implemented long TPM IRQ width then you can keep those GPIO advance PM configurati […]
This piece of code deals with the case where the external cr50 chip is not able to emit 100us pulses, and will emit 4us pulses instead, until it is upgraded.
I take what you are saying to mean that setting bit 7 of the LPM control register to zero, thereby preventing the SoC from entering S0i3.4, is not enough to guarantee that the Tiger Lake SoC will recognize a 4us pulse on a GPIO line configured to interrupt on edge. And that additionally, I need to set override_pm=1 and make bit4:0 in gpio_pm all zero.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask There is a callback used in the `fsp_params.c` file specifically for when a mainboard needs to override FSP-S UPDs, `mainboard_silicon_init_params(FSP_S_CONFIG *params)`. I think the intention may be more clear by updating the UPDs directly. Also otherwise you're relying on the mainboard chip_init happening before the SoC chip init , because FSP-S is called from the SoC chip init.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
I think the intention may be more clear by updating the UPDs directly.
We have generally avoided overwriting the FSP UPDs directly in mainboard and instead used the SoC chip configs so that the SoC alone can update the UPDs. (I honestly think that we should drop the callback `mainboard_silicon_init_params` from SoC).
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 46: gpio_override_pm You will have to drop this from devicetree.cb now:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 48: cfg->gpio_pm[i] = 0 memset to 0?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
I think the intention may be more clear by updating the UPDs directly. […]
That's fine, what about a callback like in octopus, 'mainboard_devtree_update` or similar. Using config_of_soc() all over the place is just reaching for global state
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
That's fine, what about a callback like in octopus, 'mainboard_devtree_update` or similar. […]
I have no experience in this area to base an opinion on.
Furquan do you agree with Tim's latest suggestion, or do you think the current way is better? I hope that the two of you can come to an agreement.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
what about a callback like in octopus, 'mainboard_devtree_update` or similar.
Do you want to make the callback from SoC and pass chip config as input to the call? That should be fine. It would still expose the chip config, but updating the devtree configs at runtime requires access to chip config. APL/GLK boards use a callback from SoC, whereas others simply implement a variant_devtree_update() directly from mainboard chip init.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
what about a callback like in octopus, 'mainboard_devtree_update` or similar. […]
Yes, I was thinking of a `void mainboard_update_devtree(struct soc_intel_tigerlake *config)` in the flow before `parse_devicetree` is called for any last-minute programmatic changes required for devicetree entries. WDYT?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
Yes, I was thinking of a `void mainboard_update_devtree(struct soc_intel_tigerlake *config)` in the […]
SGTM.
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#13).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/13
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#14).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/14
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#15).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 21 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/15
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 35: mainboard_update_s0ix_disable_mask
SGTM.
I have created the below CL to introduce a weak method for mainboard to override. https://review.coreboot.org/c/coreboot/+/44914
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 46: gpio_override_pm
You will have to drop this from devicetree.cb now: […]
Done
https://review.coreboot.org/c/coreboot/+/44359/12/src/mainboard/google/volte... PS12, Line 48: cfg->gpio_pm[i] = 0
memset to 0?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 15: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/44359/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/15//COMMIT_MSG@24 PS15, Line 24: It would be good to also mention the change in gpio_pm_override.
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 10: #include <halt.h> Is this required?
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 11: #include <intelblocks/cse.h> Is this required?
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 40: 5 * sizeof(cfg->gpio_pm[0]) nit: just `sizeof(cfg->gpio_pm)` should work.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 9: #include <ec/google/chromeec/ec.h> I don't think this is needed either
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 35: S03.4 /* * Multi-line comments should look * like this. */
Also S0i3.4
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#16).
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
mainboard/google/volteer: Enable long cr50 ready pulses
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the Si03.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/16
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 16:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 9: #include <ec/google/chromeec/ec.h>
I don't think this is needed either
I am not sure where this line came from. Removed
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 10: #include <halt.h>
Is this required?
Done
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 11: #include <intelblocks/cse.h>
Is this required?
Done
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 35: S03.4
/* […]
Done
https://review.coreboot.org/c/coreboot/+/44359/15/src/mainboard/google/volte... PS15, Line 40: 5 * sizeof(cfg->gpio_pm[0])
nit: just `sizeof(cfg->gpio_pm)` should work.
I was not aware that the gpio_pm array did not have more than the 5 elements mentioned as needing setting to zero. If we are filling the entire array, then definitely your suggestion makes sense.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44359/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/16//COMMIT_MSG@7 PS16, Line 7: Enable long cr50 ready pulses nit: With all the changes that have happened in the review process, this change is not really enabling long pulses. It is basically enabling S0i3.4 and GPIO PM config only if long pulses are enabled
https://review.coreboot.org/c/coreboot/+/44359/16/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/16/src/mainboard/google/volte... PS16, Line 26: mainboard_config_update mainboard_update_soc_chip_config
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#17).
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the S0i3.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/soc/intel/tigerlake/chip.h 3 files changed, 23 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/17
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#18).
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case disable the S0i3.4 substate. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/18
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#19).
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case explicitly disable the S0i3.4 substate as well as setting gpio_pm_override to all zeroes. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
(Prior to this change, the gpio_pm_override was hardcoded to zero for Volteer, but the S0i3.4 substate was not disabled. According to my conversations with Intel engineers, that was not enough to guarantee detection pulses shorter than 100us. But it is entirely possible that we have just been "lucky" that the SoC has not gone into low power mode during the boot process, where most of the cr50 communication happens.)
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/19
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 19:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44359/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/15//COMMIT_MSG@24 PS15, Line 24:
It would be good to also mention the change in gpio_pm_override.
Done
https://review.coreboot.org/c/coreboot/+/44359/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/16//COMMIT_MSG@7 PS16, Line 7: Enable long cr50 ready pulses
nit: With all the changes that have happened in the review process, this change is not really enabli […]
Done
https://review.coreboot.org/c/coreboot/+/44359/16/src/mainboard/google/volte... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/44359/16/src/mainboard/google/volte... PS16, Line 26: mainboard_config_update
mainboard_update_soc_chip_config
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 19: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 19: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG@29 PS19, Line 29: But it is entirely possible that : we have just been "lucky" that the SoC has not gone into low power : mode during the boot process, where most of the cr50 communication : happens. FYI, it's not necessarily luck, but likely the fact that most of the Cr50 communication happens in verstage, whereas the GPIO PM doesn't get enabled until FSP-S runs in ramstage.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Christian Walter, Jes Klinke, Subrata Banik, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44359
to look at the new patch set (#20).
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case explicitly disable the S0i3.4 substate as well as setting gpio_pm_override to all zeroes. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
(Prior to this change, the gpio_pm_override was hardcoded to zero for Volteer, but the S0i3.4 substate was not disabled. According to my conversations with Intel engineers, that was not enough to guarantee detection pulses shorter than 100us. But it is entirely possible that we have just been "lucky" that the SoC has not gone into low power mode during the boot process, where most of the cr50 communication happens.)
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 20 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/44359/20
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 20:
(1 comment)
Rebased to resolve merge conflict, PTAL.
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG@29 PS19, Line 29: But it is entirely possible that : we have just been "lucky" that the SoC has not gone into low power : mode during the boot process, where most of the cr50 communication : happens.
FYI, it's not necessarily luck, but likely the fact that most of the Cr50 communication happens in v […]
True, but we do still have cr50 communication when the user logs in, to mount the home directory, when new users are added, etc. All of which are after PM having been enabled.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 20: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44359/19//COMMIT_MSG@29 PS19, Line 29: But it is entirely possible that : we have just been "lucky" that the SoC has not gone into low power : mode during the boot process, where most of the cr50 communication : happens.
True, but we do still have cr50 communication when the user logs in, to mount the home directory, wh […]
IIRC, there is a timeout in the kernel driver, so if the AP doesn't receive the IRQ after a certain time, it will go and read from it to check the status.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate.
A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register.
This CL adds code to detect the case when Cr50 is unable to generate longer pulses, and in that case explicitly disable the S0i3.4 substate as well as setting gpio_pm_override to all zeroes. This will increase power usage slightly, but guarantee that the GPIO block in the SoC does not switch to a slower sampling clock. In practice, this case will only be encountered in the factory, before the Cr50 chip is updated to a new RW image.
(Prior to this change, the gpio_pm_override was hardcoded to zero for Volteer, but the S0i3.4 substate was not disabled. According to my conversations with Intel engineers, that was not enough to guarantee detection pulses shorter than 100us. But it is entirely possible that we have just been "lucky" that the SoC has not gone into low power mode during the boot process, where most of the cr50 communication happens.)
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: Idef1fffd410a345678da4b3c8aea46ac74a01470 Signed-off-by: Jes Bodi Klinke jbk@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/44359 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 2 files changed, 20 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index ead187e..849869a 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -4,9 +4,12 @@ #include <acpi/acpi.h> #include <baseboard/variants.h> #include <device/device.h> +#include <drivers/spi/tpm/tpm.h> #include <ec/ec.h> #include <fw_config.h> +#include <security/tpm/tss.h> #include <soc/gpio.h> +#include <soc/ramstage.h> #include <vendorcode/google/chromeos/chromeos.h> #include <variant/gpio.h>
@@ -38,6 +41,23 @@ dev->ops->get_smbios_strings = mainboard_smbios_strings; }
+void mainboard_update_soc_chip_config(struct soc_intel_tigerlake_config *cfg) +{ + tlcl_lib_init(); + if (cr50_is_long_interrupt_pulse_enabled()) { + printk(BIOS_INFO, "Enabling S0i3.4\n"); + } else { + /* + * Disable S0i3.4, preventing the GPIO block from switching to + * slow clock. + */ + printk(BIOS_INFO, "Not enabling S0i3.4\n"); + cfg->LpmStateDisableMask |= LPM_S0i3_4; + cfg->gpio_override_pm = 1; + memset(cfg->gpio_pm, 0, sizeof(cfg->gpio_pm)); + } +} + static void mainboard_chip_init(void *chip_info) { const struct pad_config *base_pads; diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 3e9e02c..1b8b0d6 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -187,14 +187,6 @@ register "DdiPort3Ddc" = "0" register "DdiPort4Ddc" = "0"
- # Disable PM to allow for shorter irq pulses - register "gpio_override_pm" = "1" - register "gpio_pm[0]" = "0" - register "gpio_pm[1]" = "0" - register "gpio_pm[2]" = "0" - register "gpio_pm[3]" = "0" - register "gpio_pm[4]" = "0" - # Enable "Intel Speed Shift Technology" register "speed_shift_enable" = "1"
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Disable S0i3.4 if cr50 firmware is too old ......................................................................
Patch Set 21:
Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/18045 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18044 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/18043 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18042 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/18041 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/18048 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/18047 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/18046
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Furquan Shaikh (Code Review)
-
Jes Klinke (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)