Re: [PATCH resend] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Wed, Nov 21, 2012 at 08:59:24AM +0000, Peter.Huewe@infineon.com wrote:

> I just gave the new version a run on my beagleboard with our Infineon SLB9635 TT 1.2 Soft I2C TPM
> and it seems to work as expected. (Tested with and without previous startup).
> 
> Tested-by: Peter Huewe <peter.huewe@infineon.com>

Great, I'll send an updated version, thanks!
 
> >+++ b/drivers/char/tpm/tpm.c
> >+#define TPM_ORD_STARTUP cpu_to_be32(153)
> >+#define TPM_ST_CLEAR cpu_to_be16(1)
> >+#define TPM_ST_STATE cpu_to_be16(2)
> >+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
> >+static const struct tpm_input_header tpm_startup_header = {
> >+	.tag = TPM_TAG_RQU_COMMAND,
> >+	.length = cpu_to_be32(12),
> >+	.ordinal = TPM_ORD_STARTUP
> >+};
> >+
> > ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> > 		   const char *desc)
> > {

> Purely cosmetic question, but why did you define this before the
> tpm_getcap and not tpm_startup?  All the other definitions are made
> before they are used - so this should perhaps better be moved
> directly before tpm_startup.  (Maybe we should move out these
> definitions to a common location? Header?)

Hmm, When I first read through this I thought all these definitions were
being grouped together, easy to move.

> >+struct tpm_startup_in {
> >+	__be16	startup_type;
> >+} __packed;
> 
> 
> All the other user
> __attribute__((packed));
> Care to change to be consistent?

I used to __packed to avoid a checkpatch warning, they should probably
all be changed?

Jason