[U-Boot] [PATCH 06/25] tpm: Move the I2C TPM code into one file

[Christophe Ricard]

Hi Simon,

On 13/08/2015 03:30, Simon Glass wrote:
> Hi Christophe,
>
> On 11 August 2015 at 15:42, christophe.ricard
> <christophe.ricard at gmail.com> wrote:
>> Hi Simon,
>>
>> I would basically disagree with this one.
>> The code from tpm.c you are merging into tpm_tis_i2c may not only be used by
>> tpm_tis_i2c as it is using data from TPM standards (e.g: Trusted computing
>> group) that should be used by other TPM drivers.
>> You can find in chapter 17 how the table tpm_protected_ordinal_duration,
>> tpm_ordinal_duration were build.
>> (https://www.trustedcomputinggroup.org/files/resource_files/E14876A3-1A4B-B294-D086297A1ED38F96/mainP2Structrev103.pdf).
>>
>> This come from a Linux port.
>>
>> As a result, we can imagine tis_sendrecv as a generic function where
>> tpm_transmit manage the way tpm commands are sent following specification
>> giving the hand to drivers thanks to the tpm_vendor_specific field
>> {send,recv} for handling the communication over a specified or proprietary
>> transport protocol.
>> As an example in tpm_tis_lpc, a 1s command duration might be too short or
>> too long for some commands and might be really hard to debug in case someone
>> decide to had a new TPM command support in u-boot.
>> Also 1s might be enought for the current commands or for evaluated TPM but
>> it may require a longer duration for some other.
>> By reading the duration TPM capability, that will be just generic.
> But this code is only used by the Infineon driver.
My idea is to standardize the way a tpm does a command transfer in u-boot.
The approach is applicable to all the existing u-boot tpm drivers.
>
>> Also tpm_tis_i2c is Infineon specific and does not fit to any TCG standard
>> for TPM1.2, i would recommand to rename this driver tpm_i2c_infineon to be
>> inline with Linux naming and not confuse a future user on what it support.
> Yes we should do that.
>
>> At the end from my delivery tentative, a Linux port as well, you may see
>> that i also rely on those functions. However I am not doing any check on the
>> command duration. This is to be improved...
>>
>> May you prefer an approach that would not lead to duplicated code ?
> I wonder if the timing of each command can be attached to the uclass
> and handled there. We might have a function like:
>
> tpm_xfer()
>
> which sends data to the TPM and receives a rseponse. This can be
> implemented by the uclass.
>
> Then the driver interface (which I currently have as xfer()) could be
> send() and receive(). The time delay measurement could happen in the
> uclass.
>
> So in summary:
>
> tpm-uclass.c:
> tpm_xfer()
>     - calls driver->send()
>     - checks timeout based on data supplied by the driver
>      -calls driver->receive()
>     - checks timeout based on data supplied by the driver
>      - returns result
>
> Then the drivers only need to implement simple send and receive functions.
I agree with this approach.

[...]

Best REgards
Christophe