[coreboot-gerrit] Change in coreboot[master]: google/oak: support cr50 over i2c on rowan

[Daniel Kurtz (Code Review)]

Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/19364 )

Change subject: google/oak: support cr50 over i2c on rowan
......................................................................


Patch Set 1:

(8 comments)

https://review.coreboot.org/#/c/19364/1//COMMIT_MSG
Commit Message:

PS1, Line 22: deptcharge
> depthcharge
Done


https://review.coreboot.org/#/c/19364/1/src/mainboard/google/oak/Kconfig
File src/mainboard/google/oak/Kconfig:

Line 22: 	bool "Build for a board revision with a SPI TPM 2.0"
> The equivalent for Gru needed a string because we had Kevins of both flavor
Done


Line 41: 	select MAINBOARD_HAS_TPM2 if OAK_HAS_TPM2
> These selects can go up into OAK_HAS_TPM2. They will be selected if OAK_HAS
Done


PS1, Line 66: 	default 0x20 if !OAK_HAS_TPM2
            : 	default 0x50 if OAK_HAS_TPM2
> I’d welcome a note in the commit message, why the address has to be changed
Done


https://review.coreboot.org/#/c/19364/1/src/mainboard/google/oak/bootblock.c
File src/mainboard/google/oak/bootblock.c:

PS1, Line 92: MTK_EINT_EDGE_RISING
> I thought cr50's interrupt was active low? Why is it triggering on the risi
This is what Vadim used and afaict, it appears to work.


https://review.coreboot.org/#/c/19364/1/src/mainboard/google/oak/chromeos.c
File src/mainboard/google/oak/chromeos.c:

PS1, Line 47: Cr50
> Let's use "TPM interrupt" which already has precedent on Gru.
Done


Line 59: int tis_plat_irq_status(void)
> edit: Also, when you do that, it's easy to guard that file by Kconfig in th
Done


https://review.coreboot.org/#/c/19364/1/src/mainboard/google/oak/gpio.h
File src/mainboard/google/oak/gpio.h:

Line 45: #if IS_ENABLED(CONFIG_OAK_HAS_TPM2)
> No need to guard a constant.
Done


-- 
To view, visit https://review.coreboot.org/19364
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If6cdd0e39e4ac86538f27f322c55c329179ee084
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Daniel Kurtz <djkurtz at google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Daniel Kurtz <djkurtz at chromium.org>
Gerrit-Reviewer: Daniel Kurtz <djkurtz at google.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Vadim Bendebury <vbendeb at chromium.org>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes