On Mon, Apr 22, 2013 at 05:55:50PM +0200, Mathias leblanc wrote: > From: Mathias Leblanc <mathias.lebl...@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! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
