Hello Jes Klinke,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/47049
to review the following change.
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controlled base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M 3rdparty/amd_blobs M src/mainboard/google/volteer/mainboard.c 2 files changed, 13 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47049/1
diff --git a/3rdparty/amd_blobs b/3rdparty/amd_blobs index 8c668ab..e393a88 160000 --- a/3rdparty/amd_blobs +++ b/3rdparty/amd_blobs @@ -1 +1 @@ -Subproject commit 8c668ab552a02724a07f8c6e7285a5f21a61569b +Subproject commit e393a885c89f8ee3f05242a9e42578c60931b49d diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index b4d6676..480a3f6 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -45,13 +45,24 @@ void mainboard_update_soc_chip_config(struct soc_intel_tigerlake_config *cfg) { int ret; + if (!CONFIG(MAINBOARD_HAS_SPI_TPM_CR50)) { + /* + * Negotiation of long interrupt pulses is only supported via + * SPI. I2C is only used on reworked prototypes on which the + * TPM is replaced with Dauntless under development, it will + * use long pulses by default, or use the interrupt line in a + * different way altogether. + */ + return; + } + ret = tlcl_lib_init(); if (ret != VB2_SUCCESS) { printk(BIOS_ERR, "tlcl_lib_init() failed: 0x%x\n", ret); return; }
- if (CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) && cr50_is_long_interrupt_pulse_enabled()) { + if (cr50_is_long_interrupt_pulse_enabled()) { printk(BIOS_INFO, "Enabling S0i3.4\n"); } else { /*
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47049/1/src/mainboard/google/voltee... PS1, Line 58: trailing whitespace
Hello build bot (Jenkins), Jes Klinke,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47049
to look at the new patch set (#2).
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controlled base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M 3rdparty/amd_blobs M src/mainboard/google/volteer/mainboard.c 2 files changed, 12 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47049/2
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47049
to look at the new patch set (#4).
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controlled base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47049/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/4//COMMIT_MSG@17 PS4, Line 17: controlled nit: controlle*r* (or did you mean the controlled I2C device?)
Hello build bot (Jenkins), Patrick Georgi, Furquan Shaikh, Jes Klinke, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47049
to look at the new patch set (#6).
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controller base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47049/6
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/4//COMMIT_MSG@17 PS4, Line 17: controlled
nit: controlle*r* (or did you mean the controlled I2C device?)
Done
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
All the actual Volteer variants will have CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) enabled, and in that case, it is obvious to see that this CL is a no-op.
Only the reworked Volteer prototypes, hooked up with external Dauntless development board, will use I2C instead of SPI for TPM communication, and could be affected by the logic change here.
Furquan, could you please +2 this.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7: Code-Review+2
Patch Set 7:
All the actual Volteer variants will have CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) enabled, and in that case, it is obvious to see that this CL is a no-op.
Only the reworked Volteer prototypes, hooked up with external Dauntless development board, will use I2C instead of SPI for TPM communication, and could be affected by the logic change here.
Furquan, could you please +2 this.
I didn't +2 since I'm not a Googler, but I'm fine with this change.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
Patch Set 7: Code-Review+2
Patch Set 7:
All the actual Volteer variants will have CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) enabled, and in that case, it is obvious to see that this CL is a no-op.
Only the reworked Volteer prototypes, hooked up with external Dauntless development board, will use I2C instead of SPI for TPM communication, and could be affected by the logic change here.
Furquan, could you please +2 this.
I didn't +2 since I'm not a Googler, but I'm fine with this change.
Uh, I didn't explain myself very well. Since I'm not actively keeping track of Google development in coreboot, I usually don't know if changes to Google-specific code are OK or not, so I wait for a Googler to ack the idea before giving out a +2. However, given that this change is well-reasoned, I feel confident enough to +2 it.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7: Code-Review+2
Patch Set 7:
All the actual Volteer variants will have CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) enabled, and in that case, it is obvious to see that this CL is a no-op.
Only the reworked Volteer prototypes, hooked up with external Dauntless development board, will use I2C instead of SPI for TPM communication, and could be affected by the logic change here.
Furquan, could you please +2 this.
I didn't +2 since I'm not a Googler, but I'm fine with this change.
Uh, I didn't explain myself very well. Since I'm not actively keeping track of Google development in coreboot, I usually don't know if changes to Google-specific code are OK or not, so I wait for a Googler to ack the idea before giving out a +2. However, given that this change is well-reasoned, I feel confident enough to +2 it.
Thank you, your explanation makes sense. (And so does you general stance on Google specific code.)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@14 PS7, Line 14: The I2C drivers do not support : being accessed early in ramstage So, shouldn't we fix the I2C drivers in that case?
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@16 PS7, Line 16: I2C : controller base address It is because the I2C driver seems to be assuming that the BAR is allocated before I2C initialization. However, for the TPM access case, that assumption doesn't hold and hence the driver behavior needs to be fixed.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts Is that the default for Dauntless firmware i.e. will it always support longer interrupts without an additional register write? Or is the new interrupt signalling mode supposed to guarantee that 100us+ pulse would be seen on the interrupt line without any additional configuration from the host?
In my opinion that should be the default. But, is this already captured somewhere and implemented in that way?
https://review.coreboot.org/c/coreboot/+/47049/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47049/7/src/mainboard/google/voltee... PS7, Line 103: it will use long pulses by default, or use the interrupt line in : * a different way altogether Which one is it? Is it tracked in some bug?
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
(3 comments)
I understand your concerns Furquan. It would be nice to clean up the I2C driver code of its shortcomings, but question whether there is much benefit to doing so.
Right now, I am trying to enable other Dauntless developers to compile firmware for Volteer, for our reworked setup. And this workaround code, which was relevant only for Cr50, and will not be for Ti50, is getting in the way, so I hoped to be able to disable it.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@14 PS7, Line 14: The I2C drivers do not support : being accessed early in ramstage
So, shouldn't we fix the I2C drivers in that case?
That may depend on whether we have any real use case for it. The SPI drivers also had an issue here, specifically, the SPI driver did not like being accessed both before and after chip init, due to caching.
My point is that the only reason we accessed TPM early in RAM stage was this ugly workaround to handle Cr50 firmware possibly not supporting the long interrupt pulses. At some time in the future, all our Cr50 chips will support this from the factory, and we will no longer need to access Cr50 early in ramstage.
Ti50/Dauntless is newly developed, and we will make sure that it supports whatever interrupt requirements from the first firmware version, so we will not need the early ramstage detection for Ti50, (and I do not know of plans of putting Cr50 on I2C.)
My stance is that we can fix this when we need it, and we will likely never need it.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@16 PS7, Line 16: I2C : controller base address
It is because the I2C driver seems to be assuming that the BAR is allocated before I2C initializatio […]
Again, the I2C driver appears to branch based on which stage pre-ramstage vs. ramstage, which has worked so far, and as I argue above that we have no plans of accessing I2C in early ramstage, I think that though it is a shortcoming of the driver, we do not strictly need to fix it.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts
Is that the default for Dauntless firmware i.e. […]
Yes, by default, Dauntless will guarantee 100us interrupt pulse width and spacing.
Due to the nature of I2C, and specifically "clock stretching", it is possible to modify the protocol and drivers to not even need a ready signal to be asserted before the host initiates each bus transaction. And that is very much our plan.
Furthermore, Andrey told me that the TPM standard specification actually calls for an interrupt line with different more high level semantics, probably something about the completion of long-running operations, rather than each bus transaction, so we may re-purpose the signal to fit that bill on Ti50. In any case, the Ti50 on I2C will definitely not have a register to request 100us interrupt pulses.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts
Yes, by default, Dauntless will guarantee 100us interrupt pulse width and spacing. […]
But what about when the next SoC generation comes out and has totally different requirements? If we're redesigning things, couldn't be allow for a programmable-length IRQ pulse? What about other platforms like AMD & ARM? I'm sure their controllers have different requirements.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts
But what about when the next SoC generation comes out and has totally different requirements? If we' […]
Actually, if we are thinking long term. Rather than having the GSC generate a fixed length pulse, I would rather that it asserts the interrupt line, and keeps it asserted indefinitely, when it has something to say. Then when the AP starts using the SPI/I2C bus to hear what the GSC wanted, that is when the interrupt signal should go de-asserted.
If done that way, no matter what future requirement comes up for the length of interrupt pulses, it will automatically be satisfied with no active effort.
As we are starting from a clean slate with Dauntless/Ti50, and are looking at following the TPM standard for interrupt signals, I hope that we can arrive at such a state. I do not yet know exactly what that would look like, but I am certain that there should be no need for communication in early ramstage, to set up the AP chipset depending on GSC firmware version.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 7:
(2 comments)
Patch Set 7:
(3 comments)
I understand your concerns Furquan. It would be nice to clean up the I2C driver code of its shortcomings, but question whether there is much benefit to doing so.
Right now, I am trying to enable other Dauntless developers to compile firmware for Volteer, for our reworked setup. And this workaround code, which was relevant only for Cr50, and will not be for Ti50, is getting in the way, so I hoped to be able to disable it.
With respect to fixing the I2C driver, I think it's okay. We can do that in parallel. I am more concerned about ensuring that we capture the requirements/expectations correctly so that what is being implemented on the Ti50 firmware does not lead to more problems down the line.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@16 PS7, Line 16: I2C : controller base address
Again, the I2C driver appears to branch based on which stage pre-ramstage vs. […]
Yes, it is generally true that we wouldn't access I2C or SPI devices early on i.e. before resource allocation is complete. But, I think ensuring we have a way to handle it if required isn't bad. At minimum, I think there should be a bug to track it so that it can be fixed.
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts
Yes, by default, Dauntless will guarantee 100us interrupt pulse width and spacing. […]
Are these all points captured in some place? I want to make sure that as we are doing the validation we capture the expectations/assumptions correctly. Again, there should at least be a bug/doc to track this behavior so that we don't run into surprises with different hardware requirements.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Tim Wawrzynczak, Jes Klinke, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47049
to look at the new patch set (#8).
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controller base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865, b:172509545 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/47049/8
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47049/7//COMMIT_MSG@19 PS7, Line 19: can be made to support : longer interrupts
Are these all points captured in some place? I want to make sure that as we are doing the validation […]
I have created b/172509545 to keep track of what we decide to do with the ready signal on the Ti50.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47049/7/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47049/7/src/mainboard/google/voltee... PS7, Line 103: it will use long pulses by default, or use the interrupt line in : * a different way altogether
Which one is it? Is it tracked in some bug?
We have not decided yet. (I have not taken the time to study these particular requirements of the I2C TPM standard.) But any way we decide, we will not have old GSC firmware that need special handling in early ramstage, as we do with Cr50.
I have created b/172509545 to track this.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
Patch Set 8: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47049 )
Change subject: mb/google/volteer: Skip TPM detection except on SPI ......................................................................
mb/google/volteer: Skip TPM detection except on SPI
Production Volteer devices have Cr50 TPM connected via SPI, depending on Cr50 firmware version it may or may not support long enough interrupt pulses for the SoC to safely be able to enable lowest power mode.
Some reworked Volteer devices have had the Cr50 (Haven) TPM replaced with Dauntless, communicating via I2C. The I2C drivers do not support being accessed early in ramstage, before chip init and memory mapping, (tlcl_lib_init() will halt with an error finding the I2C controller base address.)
Since the Dauntless device under development can be made to support longer interrupts, or a completely new interrupt signalling mode, there is no need to try to go through the same discovery as is done via SPI. This CL will skip the discovery, enabling the S0i3.4 sleep mode in all cases, on the reworked test devices.
BUG=b:169526865, b:172509545 TEST=abuild -t GOOGLE_VOLTEER2 -c max -x
Change-Id: I08a533cede30a3c0ab943938961dc7e4b572d4ce Signed-off-by: Jes Bodi Klinke jbk@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/47049 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/mainboard.c 1 file changed, 11 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index acba972..0850e74 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -96,13 +96,23 @@ void mainboard_update_soc_chip_config(struct soc_intel_tigerlake_config *cfg) { int ret; + if (!CONFIG(MAINBOARD_HAS_SPI_TPM_CR50)) { + /* + * Negotiation of long interrupt pulses is only supported via SPI. I2C is only + * used on reworked prototypes on which the TPM is replaced with Dauntless under + * development, it will use long pulses by default, or use the interrupt line in + * a different way altogether. + */ + return; + } + ret = tlcl_lib_init(); if (ret != VB2_SUCCESS) { printk(BIOS_ERR, "tlcl_lib_init() failed: 0x%x\n", ret); return; }
- if (CONFIG(MAINBOARD_HAS_SPI_TPM_CR50) && cr50_is_long_interrupt_pulse_enabled()) { + if (cr50_is_long_interrupt_pulse_enabled()) { printk(BIOS_INFO, "Enabling S0i3.4\n"); } else { /*
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
Jes Klinke (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)