Hi Simon,
On 13/08/2015 03:30, Simon Glass wrote:
Hi Christophe,
On 11 August 2015 at 15:42, christophe.ricard christophe.ricard@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-B29...).
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