Dear Vadim Bendebury,
In message 20111010025327.119EB40B40@eskimo.mtv.corp.google.com you wrote:
TPM (Trusted Platform Module) is an integrated circuit and software platform that provides computer manufacturers with the core components of a subsystem used to assure authenticity, integrity and confidentiality.
This driver supports version 1.2 of the TCG (Trusted Computing Group) specifications.
The TCG specification defines several so called localities in a TPM chip, to be controlled by different software layers. When used on a typical x86 platform during the firmware phase, only locality 0 can be accessed by the CPU, so this driver even while supporting the locality concept presumes that only locality zero is used.
This implementation is loosely based on the article "Writing a TPM Device Driver" published on http://ptgmedia.pearsoncmg.com and a submission by Stefan Berger on Qemu-devel mailing list (http://lists.gnu.org/archive/html/qemu-devel).
Compiling this driver with DEBUG defined will generate trace of all accesses to TMP registers.
This driver has been tested and is being used in three different functional ChromeOS machines (Pinetrail and Sandy Bridge Intel chipsets) all using the same Infineon SLB 9635 TT 1.2 device.
A u-boot cli command allowing access to the TPM was also implemented and will be submitted separately.
Change-Id: I22a33c3e5b2e20eec9557a7621bd463b30389d73 Signed-off-by: Vadim Bendebury vbendeb@chromium.org CC: Wolfgang Denk wd@denx.de
...
As is, there are no users of this code, so it would be just dead code. Please resubmit in a patch series that also includes code to use this feature.
+++ b/drivers/tpm/Makefile @@ -0,0 +1,27 @@ +# Copyright (c) 2011 The Chromium OS Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file.
There is no LICENSE file. Please provide exact license information; for details please see bullet # 1 at http://www.denx.de/wiki/view/U-Boot/Patches#Notes
Please fix globally.
+/* #define DEBUG */
Please remove dead code like this.
+#define PREFIX "lpc_tpm: " +#define TPM_DEBUG(fmt, args...) \
- if (TPM_DEBUG_ON) { \
printf(PREFIX); \printf(fmt , ##args); \- }
Can you not use standard debug() code?
+#ifndef CONFIG_TPM_TIS_BASE_ADDRESS +/* Base TPM address standard for x86 systems */ +#define CONFIG_TPM_TIS_BASE_ADDRESS 0xfed40000 +#endif
I think this should be removed.
+#define TIS_REG(LOCALITY, REG) \
- (void *)(CONFIG_TPM_TIS_BASE_ADDRESS + (LOCALITY << 12) + REG)
We do not allow to access device registers through base address + offset. Please always use C structs instead.
...
+/* TPM access functions are carved out to make tracing easier. */ +static u32 tpm_read(int locality, u32 reg) +{
- u32 value;
- /*
* Data FIFO register must be read and written in byte access mode,* otherwise the FIFO values are returned 4 bytes at a time.*/
Please insert blank line between declarations and code. Please fix globally.
...
- /* this will have to be converted into debug printout */
- TPM_DEBUG("Found TPM %s by %s\n", device_name, vendor_name);
Is this comment still correct?
+int tis_init(void) +{
- if (tis_probe())
return TPM_DRIVER_ERR;- return 0;
+}
Or simply:
return tis_probe();
Best regards,
Wolfgang Denk