Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
[WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM
Board devicetrees requests for drivers7pc80/tpm but that was not included for the build.
Change-Id: Ia959ae9072e3fc709b779b1fc778eb82e10c2ee3 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/intel/cannonlake_rvp/Kconfig M src/mainboard/intel/coffeelake_rvp/Kconfig 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35103/1
diff --git a/src/mainboard/intel/cannonlake_rvp/Kconfig b/src/mainboard/intel/cannonlake_rvp/Kconfig index d86e485..6d686c3 100644 --- a/src/mainboard/intel/cannonlake_rvp/Kconfig +++ b/src/mainboard/intel/cannonlake_rvp/Kconfig @@ -13,6 +13,7 @@ select SOC_INTEL_CANNONLAKE select MAINBOARD_USES_IFD_EC_REGION select INTEL_LPSS_UART_FOR_CONSOLE + select MAINBOARD_HAS_LPC_TPM
config MAINBOARD_DIR string diff --git a/src/mainboard/intel/coffeelake_rvp/Kconfig b/src/mainboard/intel/coffeelake_rvp/Kconfig index 13e55b3..8bd2881 100644 --- a/src/mainboard/intel/coffeelake_rvp/Kconfig +++ b/src/mainboard/intel/coffeelake_rvp/Kconfig @@ -16,6 +16,7 @@ select SOC_INTEL_COMMON_BLOCK_HDA if BOARD_INTEL_WHISKEYLAKE_RVP || BOARD_INTEL_COMETLAKE_RVP select MAINBOARD_USES_IFD_EC_REGION select MAINBOARD_USES_IFD_GBE_REGION if BOARD_INTEL_COFFEELAKE_RVP11 || BOARD_INTEL_COFFEELAKE_RVP8 + select MAINBOARD_HAS_LPC_TPM
config MAINBOARD_DIR string
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
I don't know if boards have LPC connector for the purpose.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG@9 PS1, Line 9: 7 should be a slash instead
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
(1 comment)
I am waiting for Subrata to tell if these boards have LPC signals routed such that LPC_TPM would ever be possible. Error might be in devicetree.cb instead.
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG@9 PS1, Line 9: 7
should be a slash instead
oh shift!
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
yes, we could connect LPC TPM om this boards but it also need change in descriptor, just CB changes won't be enough. More over on RVPs ppl prefers to use Mock TPM to avoid the additional HW to boot the platform. For this reason we don't enable this Kconfig by default.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
What kind of error you faced when this wasnt added in? So far we didnt see any issue without this config.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
It has not been decided if CB:35086 will be merged, but I can say that the weak declarations util/sconfig autogenerates in static.c are problematic. CB:35105 CB:35106 and some more similar cases I have not fixed yet.
https://qa.coreboot.org/job/coreboot-gerrit/102175/testReport/junit/board/ch...
If there are additional requirements for LPC_TPM to work, besides the Kconfig change, I would remove pc80/tpm from devicetree.cb files instead. If you initiate commit with the required descriptor change, you get to put it back. The option to have pc80/tpm in devicetree and still have LPC_TPM=n will be solved somehow. I would probably rename 'MAINBOARD_HAS_LPC_TPM' to 'DRIVERS_PC80_TPM' and have 'LPC_TPM' to only control if device is enabled instead of using it as guard to pc80/tpm/tis.c.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 1:
Please advice if one should be allowed to list chips in devicetree.cb, for which the associated driver will not be compiled. In my opinion no, chip_ops->enable_dev can be made to evaluate if the driver should be disabled.
See related CB:35086, we caught some cases where we pick up the empty weak chip_operations generated by util/sconfig.
Hello Aaron Durbin, Amol N Sukerkar, Patrick Rudolph, Subrata Banik, Angel Pons, Sachin Agrawal, Lean Sheng Tan, Boon Tiong Teo, build bot (Jenkins), Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35103
to look at the new patch set (#4).
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
[WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM
Board devicetrees requests for drivers/pc80/tpm but that was not included for the build.
Change-Id: Ia959ae9072e3fc709b779b1fc778eb82e10c2ee3 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/intel/cannonlake_rvp/Kconfig M src/mainboard/intel/coffeelake_rvp/Kconfig 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35103/4
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 4:
Seems to me that MAINBOARD_HAS_LPC_TPM currently is alias of LPC_TPM ?
Can we consider some cleanup here to support cases where TPM is an add-on on LPC connector and not a fixed part of the configuration? We would need to keep chip entry in devicetree.cb for LPC routing purposes at least.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 5:
Patrick (R.) can you have a look at the LPC enablement?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 5:
Patch Set 5:
Patrick (R.) can you have a look at the LPC enablement?
It looks like there's no additional LPC setup required as the PCH routes I/O below 0x1000 to LPC (if subtractive decode is enabled) and memory in the range 0xfed40000-0xfed44fff also to LPC.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Abandoned
Insufficient vendor and/or TPM feedback for me to solve this.
Kyösti Mälkki has restored this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Restored
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35103/1//COMMIT_MSG@9 PS1, Line 9: 7
oh shift!
Done 😄
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 6:
Looking at older comments, Subrata implied that nobody is using these boards with an LPC TPM. So how about removing the devicetree entries instead?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: [WIP] mb/intel/cannonlake_rvp,coffeelake_rvp: Add LPC_TPM ......................................................................
Patch Set 7:
I don't mind either approach.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Boon Tiong Teo, Angel Pons, Subrata Banik, Sachin Agrawal, Lean Sheng Tan, Patrick Rudolph, Aaron Durbin, Amol N Sukerkar,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35103
to look at the new patch set (#9).
Change subject: mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM ......................................................................
mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM
Board devicetrees requests for drivers/pc80/tpm but that was not included for the build. Note that there is actually no on-board TPM, just an LPC header connector on these boards.
Change-Id: Ia959ae9072e3fc709b779b1fc778eb82e10c2ee3 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/intel/cannonlake_rvp/Kconfig M src/mainboard/intel/coffeelake_rvp/Kconfig 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/35103/9
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM ......................................................................
Patch Set 9:
See related CB:41872 and CB:41873.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM ......................................................................
Patch Set 9: Code-Review+1
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM ......................................................................
Patch Set 9: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35103 )
Change subject: mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM ......................................................................
mb/intel/cannonlake_rvp,coffeelake_rvp: Add MAINBOARD_HAS_LPC_TPM
Board devicetrees requests for drivers/pc80/tpm but that was not included for the build. Note that there is actually no on-board TPM, just an LPC header connector on these boards.
Change-Id: Ia959ae9072e3fc709b779b1fc778eb82e10c2ee3 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35103 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/intel/cannonlake_rvp/Kconfig M src/mainboard/intel/coffeelake_rvp/Kconfig 2 files changed, 2 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Felix Held: Looks good to me, approved
diff --git a/src/mainboard/intel/cannonlake_rvp/Kconfig b/src/mainboard/intel/cannonlake_rvp/Kconfig index 133fe47..e4885a8 100644 --- a/src/mainboard/intel/cannonlake_rvp/Kconfig +++ b/src/mainboard/intel/cannonlake_rvp/Kconfig @@ -13,6 +13,7 @@ select SOC_INTEL_CANNONLAKE select MAINBOARD_USES_IFD_EC_REGION select INTEL_LPSS_UART_FOR_CONSOLE + select MAINBOARD_HAS_LPC_TPM
config MAINBOARD_DIR string diff --git a/src/mainboard/intel/coffeelake_rvp/Kconfig b/src/mainboard/intel/coffeelake_rvp/Kconfig index da2cd2e..1b1a85c 100644 --- a/src/mainboard/intel/coffeelake_rvp/Kconfig +++ b/src/mainboard/intel/coffeelake_rvp/Kconfig @@ -16,6 +16,7 @@ select SOC_INTEL_COMMON_BLOCK_HDA if BOARD_INTEL_WHISKEYLAKE_RVP || BOARD_INTEL_COMETLAKE_RVPU select MAINBOARD_USES_IFD_EC_REGION select MAINBOARD_USES_IFD_GBE_REGION if BOARD_INTEL_COFFEELAKE_RVP11 || BOARD_INTEL_COFFEELAKE_RVP8 + select MAINBOARD_HAS_LPC_TPM
config MAINBOARD_DIR string
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Kyösti Mälkki (Code Review)
-
Lean Sheng Tan (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)