On Mon, Apr 22, 2013 at 05:55:50PM +0200, Mathias leblanc wrote:
From: Mathias Leblanc mathias.leblanc@st.com
- STMicroelectronics version 1.2.0, Copyright (C) 2013
- This is free software, and you are welcome to redistribute it
- under certain conditions.
Just leave these lines out of the commit message.
This is the driver for TPM chip from ST Microelectronics.
If you have a TPM security chip from STMicroelectronics working with an I2C, read the README file and add the correct defines regarding the tpm in the configuration file of your board. This file is located in include/configs/your_board.h
The driver will be accessible from within uboot terminal.
Please see http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions as this is the 3rd or 4th posting of these changes.
[snip]
diff --git a/common/cmd_tpm.c b/common/cmd_tpm.c index 0970a6f..2b7bf35 100644 --- a/common/cmd_tpm.c +++ b/common/cmd_tpm.c @@ -145,10 +145,177 @@ static int do_tpm_many(cmd_tbl_t *cmdtp, int flag, return rv; }
+static int do_tpm_hash(cmd_tbl_t *cmdtp, int flag, int argc, +char * const argv[])
checkpatch warning there, please run tools/checkpatch.pl and fix all errors.
- u8 startup[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x0c,0x00, 0x00, 0x00, 0x99,0x00, 0x01- };
- u8 selftestfull[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x0a,0x00, 0x00, 0x00, 0x50- };
- u8 readpcr17[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x0e,0x00, 0x00, 0x00, 0x15,0x00, 0x00, 0x00, 0x11- };
Comment what these are and/or where they come from.
[snip]
- if (!
- tis_sendrecv(readpcr17, sizeof(readpcr17), &response, &read_size)) {
Funny place to break the line, split it on the tis_sendrecv params. And fix anything else like this too.
[snip]
- u8 startup[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x0c,0x00, 0x00, 0x00, 0x99,0x00, 0x01- };
- u8 selftestfull[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x0a,0x00, 0x00, 0x00, 0x50- };
- u8 getpermflags[] = {
0x00, 0xc1,0x00, 0x00, 0x00, 0x16,0x00, 0x00, 0x00, 0x65,0x00, 0x00, 0x00, 0x04,0x00, 0x00, 0x00, 0x04,0x00, 0x00, 0x01, 0x08- };
startup and selftestfull are the same as before? Shouldn't duplicate them then.
- CHECK(tis_init());
Too much whitespace around the line.
[snip]
+/* extended error numbers from linux (see errno.h) */ +#define ECANCELED 125 /* Operation Canceled */
'#define<space>' please.
+#define msleep(t) udelay((t)*1000)
We already have a few msleep compatibility defines. Can you please do a separate prepatch that adds msleep to <common.h> after the extern for udelay?
+/* Timer frequency. Corresponds to msec timer resolution*/ +#define HZ 1000
Please use CONFIG_SYS_HZ
+++ b/drivers/tpm/tis_i2c.c
[snip]
- [backport from https://github.com/theopolis/u-boot-sboot/
blob/master/drivers/tpm/tis_i2c.c]
You're giving the original author credit in the file already, yes? If not, please do, and drop these lines.
+/* Define in board config */ +#ifndef CONFIG_TPM_I2C_BUS
- #define CONFIG_TPM_I2C_BUS 0
- #define CONFIG_TPM_I2C_ADDR 0
+#endif
No extra indentation.
[snip]
- if (i2c_probe(tpm.slave_addr) && i2c_probe(tpm.slave_addr)) {
debug("%s: fail to probe i2c addr 0x%x\n", __func__, tpm.slave_addr);
Split after __func__ instead.
diff --git a/drivers/tpm/tpm.c b/drivers/tpm/tpm.c
[snip]
We have this as drivers/tpm/slb9635_i2c/tpm.c now, so I think we need to be renaming the existing one, same with tpm.h
[snip]
diff --git a/drivers/tpm/tpm_i2c_st.c b/drivers/tpm/tpm_i2c_st.c
[snip]
+/*
- write8_reg
- Send byte to the TIS register according to the ST33ZP24 I2C protocol.
- @param: tpm_register, the tpm tis register where the data should be written
- @param: tpm_data, the tpm_data to write inside the tpm_register
- @param: tpm_size, The length of the data
- @return: Returns zero in case of success else the negative error code.
- */
+static int write8_reg(u8 addr, u8 tpm_register,
u8 *tpm_data, u16 tpm_size)+{
- u8 data;
- data = tpm_register;
- memcpy(&(tpm_dev.buf[0]), &data, sizeof(data));
- memcpy(&(tpm_dev.buf[0])+1, tpm_data, tpm_size);
- return i2c_write(addr, 0, 0, &tpm_dev.buf[0],
tpm_size + 1);+} /* write8_reg() */
Since this is kerneldoc/etc style commenting, it should be /** at the start, yes? And we don't do comments like that at the end of a function. And since it is kerneldoc style, can you do a template for them? See doc/DocBook/ Thanks!