Change in coreboot[master]: tpm: Refactor TPM Kconfig dimensions

[Julius Werner (Code Review)]

Attention is currently required from: Stefan Ott, Felix Singer, Patrick Rudolph, Jonathan Zhang, Nick Vaccaro, Arthur Heymans, Andrey Petrov, Piotr Król, Jes Klinke, Nico Huber, Sean Rhodes, Michał Żygowski, Johnny Lin, Christian Walter, Werner Zeh, Alexander Couzens, Yu-Ping Wu, Tim Chu, Frans Hendriks, Tristan Corrick, Jeremy Soller, Angel Pons, Michael Niewöhner, Tim Crawford, Maxim Polyakov, Tim Wawrzynczak.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63424 )

Change subject: tpm: Refactor TPM Kconfig dimensions
......................................................................

Patch Set 13:

(4 comments)

File src/drivers/crb/chip.c:

https://review.coreboot.org/c/coreboot/+/63424/comment/8d2a09ef_a4c7fdaa 
PS13, Line 17: 	.acpi_fill_ssdt = crb_tpm_fill_ssdt,
The definitions of these two functions need to move into this file too, otherwise you have missing reference errors here when building without CONFIG_TPM.

File src/drivers/pc80/tpm/chip.c:

https://review.coreboot.org/c/coreboot/+/63424/comment/cc8c16b8_57c61344 
PS13, Line 14: 	u8 value = read8(TIS_REG(locality, TIS_REG_INT_VECTOR));
Jenkins complains that you don't have TIS_REG() in this context. Another good reason to move most of this back to tis.c.

https://review.coreboot.org/c/coreboot/+/63424/comment/e6d7a927_34546d52 
PS13, Line 194: 	if (CONFIG(TPM))
Well, in this case moving lpc_tpm_ops and everything below it into this file isn't really necessary because there's this CONFIG(TPM) check here that kills it all for the NO_TPM case anyway. In particular, some of that calls code from tis.c (like tis_open()) which then wouldn't work.

The way you did it now isn't broken, but I think it would make more sense to keep the other stuff in tis.c.

(Why this behaves differently than the CRB and I2C drivers for the NO_TPM case and doesn't register the tpm_ops with the ACPI SSDT stuff, I do not know. Most likely one of those behaviors is correct for all of them and at least one of these drivers is buggy, but I don't know which is which. Maybe some of the x86 folks on this patch can give more insight on how this should change. If we want to write an SSDT entry for NO_TPM and it needs to do actual TPM accesses to fill out those values, that would be a problem.)

File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/63424/comment/01533c67_5469834b 
PS10, Line 87: CONFIG(TPM_GOOGLE_CR50)
> Good catch again.  Actually, with the new _CR50/_TI50 split, the SPI_TPM part could be dropped here. […]

According to the comment here, it sounds like the SPI_TPM is important ("Negotiation of long interrupt pulses is only supported via SPI."). I guess maybe I2C doesn't work at all on those platforms, so it could go either way... but anyway, sounds like this code isn't "wrong".

edit: Oh right, and I didn't de-Morgan correctly when I suggested this. Thanks for paying attention!