From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christophe Ricard Date: Thu, 13 Aug 2015 22:26:11 +0200 Subject: [U-Boot] [PATCH 06/25] tpm: Move the I2C TPM code into one file In-Reply-To: References: <1439304497-10081-1-git-send-email-sjg@chromium.org> <1439304497-10081-7-git-send-email-sjg@chromium.org> <55CA6C38.7020104@gmail.com> Message-ID: <55CCFD63.1000808@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 13/08/2015 03:30, Simon Glass wrote: > Hi Christophe, > > On 11 August 2015 at 15:42, christophe.ricard > 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