Re: [PATCH v2] tpm: Factor out common startup code

2016-05-26 Thread Jarkko Sakkinen 
On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> common startup sequence. This is the sequence used by tpm_tis and
> tpm_crb.
> 
> All drivers should set this flag.
> 
> Signed-off-by: Jason Gunthorpe 
> Tested-by: Andrew Zamansky 

I assume there is coming a patch set of which part this patch will be?
In that patch set (whoever makes it) could you put the updated
description in place?

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-26 Thread Jarkko Sakkinen 
On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> common startup sequence. This is the sequence used by tpm_tis and
> tpm_crb.
> 
> All drivers should set this flag.
> 
> Signed-off-by: Jason Gunthorpe 
> Tested-by: Andrew Zamansky 

I assume there is coming a patch set of which part this patch will be?
In that patch set (whoever makes it) could you put the updated
description in place?

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-19 Thread Jarkko Sakkinen 
On Wed, May 18, 2016 at 11:13:20AM -0600, Jason Gunthorpe wrote:
> On Wed, May 18, 2016 at 12:59:24PM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> > > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > > > common startup sequence. This is the sequence used by tpm_tis and
> > > > > tpm_crb.
> > > > > 
> > > > > All drivers should set this flag.
> > > > 
> > > > The commit message should be a much much more verbose I cannot include
> > > > this without a better explanation. Please update this for the next
> > > > revision.
> > > 
> > > What more description do you want to see?
> > 
> > It is lacking a lot of relevant information:
> > 
> > * It should explain what you mean by startup sequence".
> > * It should describe the constant TPM_OPS_AUTO_STARTUP
> > * It should explain what drivers are doing at the moment (before
> >   this feature).
> > * It should explain what is the benefit for different HW drivers
> >   after applying this patch.
> > * It should explain why you call the executed sequence "automatic"
> >   and also use the word "standard". Yeah, I didn't understand this,
> >   this not me being picky. Maybe it should be DEFAULT_STARTUP??
> 
> Well, use something like this then. You can edit descriptions to your
> liking when you apply the patch, FWIW.
> 
> tpm: Factor out common startup code
> 
> The TCG standard startup sequence (get timeouts, tpm startup, etc) for
> TPM and TPM2 chips is being open coded in many drivers, move it into
> the core code.
> 
> tpm_tis and tpm_crb are used as the basis for the core code
> implementation and the easy drivers are converted. In the process
> several small drivers bugs relating to error handling this flow
> are fixed.
> 
> For now the flag TPM_OPS_AUTO_STARTUP is optional to allow a staged
> driver roll out, but ultimately all drivers should use this flow and
> the flag removed. Some drivers still do not implement the startup
> sequence at all and will need to be tested with it enabled.

This is so much better! Thank you.

I can always edit a description but I do not want completely rewrite
them :)

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-19 Thread Jarkko Sakkinen 
On Wed, May 18, 2016 at 11:13:20AM -0600, Jason Gunthorpe wrote:
> On Wed, May 18, 2016 at 12:59:24PM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> > > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > > > common startup sequence. This is the sequence used by tpm_tis and
> > > > > tpm_crb.
> > > > > 
> > > > > All drivers should set this flag.
> > > > 
> > > > The commit message should be a much much more verbose I cannot include
> > > > this without a better explanation. Please update this for the next
> > > > revision.
> > > 
> > > What more description do you want to see?
> > 
> > It is lacking a lot of relevant information:
> > 
> > * It should explain what you mean by startup sequence".
> > * It should describe the constant TPM_OPS_AUTO_STARTUP
> > * It should explain what drivers are doing at the moment (before
> >   this feature).
> > * It should explain what is the benefit for different HW drivers
> >   after applying this patch.
> > * It should explain why you call the executed sequence "automatic"
> >   and also use the word "standard". Yeah, I didn't understand this,
> >   this not me being picky. Maybe it should be DEFAULT_STARTUP??
> 
> Well, use something like this then. You can edit descriptions to your
> liking when you apply the patch, FWIW.
> 
> tpm: Factor out common startup code
> 
> The TCG standard startup sequence (get timeouts, tpm startup, etc) for
> TPM and TPM2 chips is being open coded in many drivers, move it into
> the core code.
> 
> tpm_tis and tpm_crb are used as the basis for the core code
> implementation and the easy drivers are converted. In the process
> several small drivers bugs relating to error handling this flow
> are fixed.
> 
> For now the flag TPM_OPS_AUTO_STARTUP is optional to allow a staged
> driver roll out, but ultimately all drivers should use this flow and
> the flag removed. Some drivers still do not implement the startup
> sequence at all and will need to be tested with it enabled.

This is so much better! Thank you.

I can always edit a description but I do not want completely rewrite
them :)

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-18 Thread Jason Gunthorpe 
On Wed, May 18, 2016 at 12:59:24PM +0300, Jarkko Sakkinen wrote:
> On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > > common startup sequence. This is the sequence used by tpm_tis and
> > > > tpm_crb.
> > > > 
> > > > All drivers should set this flag.
> > > 
> > > The commit message should be a much much more verbose I cannot include
> > > this without a better explanation. Please update this for the next
> > > revision.
> > 
> > What more description do you want to see?
> 
> It is lacking a lot of relevant information:
> 
> * It should explain what you mean by startup sequence".
> * It should describe the constant TPM_OPS_AUTO_STARTUP
> * It should explain what drivers are doing at the moment (before
>   this feature).
> * It should explain what is the benefit for different HW drivers
>   after applying this patch.
> * It should explain why you call the executed sequence "automatic"
>   and also use the word "standard". Yeah, I didn't understand this,
>   this not me being picky. Maybe it should be DEFAULT_STARTUP??

Well, use something like this then. You can edit descriptions to your
liking when you apply the patch, FWIW.

tpm: Factor out common startup code

The TCG standard startup sequence (get timeouts, tpm startup, etc) for
TPM and TPM2 chips is being open coded in many drivers, move it into
the core code.

tpm_tis and tpm_crb are used as the basis for the core code
implementation and the easy drivers are converted. In the process
several small drivers bugs relating to error handling this flow
are fixed.

For now the flag TPM_OPS_AUTO_STARTUP is optional to allow a staged
driver roll out, but ultimately all drivers should use this flow and
the flag removed. Some drivers still do not implement the startup
sequence at all and will need to be tested with it enabled.

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-18 Thread Jason Gunthorpe 
On Wed, May 18, 2016 at 12:59:24PM +0300, Jarkko Sakkinen wrote:
> On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > > common startup sequence. This is the sequence used by tpm_tis and
> > > > tpm_crb.
> > > > 
> > > > All drivers should set this flag.
> > > 
> > > The commit message should be a much much more verbose I cannot include
> > > this without a better explanation. Please update this for the next
> > > revision.
> > 
> > What more description do you want to see?
> 
> It is lacking a lot of relevant information:
> 
> * It should explain what you mean by startup sequence".
> * It should describe the constant TPM_OPS_AUTO_STARTUP
> * It should explain what drivers are doing at the moment (before
>   this feature).
> * It should explain what is the benefit for different HW drivers
>   after applying this patch.
> * It should explain why you call the executed sequence "automatic"
>   and also use the word "standard". Yeah, I didn't understand this,
>   this not me being picky. Maybe it should be DEFAULT_STARTUP??

Well, use something like this then. You can edit descriptions to your
liking when you apply the patch, FWIW.

tpm: Factor out common startup code

The TCG standard startup sequence (get timeouts, tpm startup, etc) for
TPM and TPM2 chips is being open coded in many drivers, move it into
the core code.

tpm_tis and tpm_crb are used as the basis for the core code
implementation and the easy drivers are converted. In the process
several small drivers bugs relating to error handling this flow
are fixed.

For now the flag TPM_OPS_AUTO_STARTUP is optional to allow a staged
driver roll out, but ultimately all drivers should use this flow and
the flag removed. Some drivers still do not implement the startup
sequence at all and will need to be tested with it enabled.

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-18 Thread Jarkko Sakkinen 
On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > common startup sequence. This is the sequence used by tpm_tis and
> > > tpm_crb.
> > > 
> > > All drivers should set this flag.
> > 
> > The commit message should be a much much more verbose I cannot include
> > this without a better explanation. Please update this for the next
> > revision.
> 
> What more description do you want to see?

It is lacking a lot of relevant information:

* It should explain what you mean by startup sequence".
* It should describe the constant TPM_OPS_AUTO_STARTUP
* It should explain what drivers are doing at the moment (before
  this feature).
* It should explain what is the benefit for different HW drivers
  after applying this patch.
* It should explain why you call the executed sequence "automatic"
  and also use the word "standard". Yeah, I didn't understand this,
  this not me being picky. Maybe it should be DEFAULT_STARTUP??

Your commit message is as good as no commit message at all. I only got
picture what the patch does by reading the code.  I see the commit
message as or sometimes more important than the code change itself.

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-18 Thread Jarkko Sakkinen 
On Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote:
> On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > > common startup sequence. This is the sequence used by tpm_tis and
> > > tpm_crb.
> > > 
> > > All drivers should set this flag.
> > 
> > The commit message should be a much much more verbose I cannot include
> > this without a better explanation. Please update this for the next
> > revision.
> 
> What more description do you want to see?

It is lacking a lot of relevant information:

* It should explain what you mean by startup sequence".
* It should describe the constant TPM_OPS_AUTO_STARTUP
* It should explain what drivers are doing at the moment (before
  this feature).
* It should explain what is the benefit for different HW drivers
  after applying this patch.
* It should explain why you call the executed sequence "automatic"
  and also use the word "standard". Yeah, I didn't understand this,
  this not me being picky. Maybe it should be DEFAULT_STARTUP??

Your commit message is as good as no commit message at all. I only got
picture what the patch does by reading the code.  I see the commit
message as or sometimes more important than the code change itself.

/Jarkko

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-17 Thread Jason Gunthorpe 
On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > common startup sequence. This is the sequence used by tpm_tis and
> > tpm_crb.
> > 
> > All drivers should set this flag.
> 
> The commit message should be a much much more verbose I cannot include
> this without a better explanation. Please update this for the next
> revision.

What more description do you want to see?

> > +   if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
> > +   rc = tpm2_probe(chip);
> > +   if (rc)
> > +   return rc;
> > +   }
> 
> Dead code.

Yes, this is used by the follow on driver updates. Andrew is going to
be sending a patch that uses it right away. I don't really care if it
gets shifted to that patch or not..

Jason

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-17 Thread Jason Gunthorpe 
On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote:
> On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> > Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> > common startup sequence. This is the sequence used by tpm_tis and
> > tpm_crb.
> > 
> > All drivers should set this flag.
> 
> The commit message should be a much much more verbose I cannot include
> this without a better explanation. Please update this for the next
> revision.

What more description do you want to see?

> > +   if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
> > +   rc = tpm2_probe(chip);
> > +   if (rc)
> > +   return rc;
> > +   }
> 
> Dead code.

Yes, this is used by the follow on driver updates. Andrew is going to
be sending a patch that uses it right away. I don't really care if it
gets shifted to that patch or not..

Jason

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-16 Thread Jarkko Sakkinen 
On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> common startup sequence. This is the sequence used by tpm_tis and
> tpm_crb.
> 
> All drivers should set this flag.

The commit message should be a much much more verbose I cannot include
this without a better explanation. Please update this for the next
revision.

> Signed-off-by: Jason Gunthorpe 
> Tested-by: Andrew Zamansky 
> ---
>  drivers/char/tpm/st33zp24/st33zp24.c |  4 +---
>  drivers/char/tpm/tpm-chip.c  | 15 ++
>  drivers/char/tpm/tpm-interface.c | 27 
>  drivers/char/tpm/tpm.h   |  2 ++
>  drivers/char/tpm/tpm2-cmd.c  | 40 
> 
>  drivers/char/tpm/tpm_crb.c   | 10 +
>  drivers/char/tpm/tpm_i2c_atmel.c |  6 +-
>  drivers/char/tpm/tpm_i2c_infineon.c  |  4 +---
>  drivers/char/tpm/tpm_i2c_nuvoton.c   |  7 +--
>  drivers/char/tpm/tpm_tis.c   | 24 +-
>  include/linux/tpm.h  |  6 ++
>  11 files changed, 96 insertions(+), 49 deletions(-)
> 
> v2 has a little typo fix From Andrew in the call to tpm2_startup
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
> b/drivers/char/tpm/st33zp24/st33zp24.c
> index 8d626784cd8d..4556c95f83cb 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -532,6 +532,7 @@ static bool st33zp24_req_canceled(struct tpm_chip *chip, 
> u8 status)
>  }
>  
>  static const struct tpm_class_ops st33zp24_tpm = {
> + .flags = TPM_OPS_AUTO_STARTUP,
>   .send = st33zp24_send,
>   .recv = st33zp24_recv,
>   .cancel = st33zp24_cancel,
> @@ -618,9 +619,6 @@ int st33zp24_probe(void *phy_id, const struct 
> st33zp24_phy_ops *ops,
>   tpm_gen_interrupt(chip);
>   }
>  
> - tpm_get_timeouts(chip);
> - tpm_do_selftest(chip);
> -
>   return tpm_chip_register(chip);
>  _tpm_clean_answer:
>   dev_info(>dev, "TPM initialization fail\n");
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 274dd0123237..9a36cedd94eb 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -223,6 +223,21 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>   int rc;
>  
> + if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
> + rc = tpm2_probe(chip);
> + if (rc)
> + return rc;
> + }

Dead code.

> +
> + if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + rc = tpm2_auto_startup(chip);
> + else
> + rc = tpm1_auto_startup(chip);
> + if (rc)
> + return rc;
> + }
> +
>   rc = tpm1_chip_register(chip);
>   if (rc)
>   return rc;
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index e2fa89c88304..4e6798ab3a90 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -842,6 +842,33 @@ int tpm_do_selftest(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm_do_selftest);
>  
> +/**
> + * tpm1_auto_startup - Perform the standard automatic TPM initialization
> + * sequence
> + * @chip: TPM chip to use
> + *
> + * Returns 0 on success, < 0 in case of fatal error.
> + */
> +int tpm1_auto_startup(struct tpm_chip *chip)
> +{
> + int rc;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto out;
> + rc = tpm_do_selftest(chip);
> + if (rc) {
> + dev_err(>dev, "TPM self test failed\n");
> + goto out;
> + }
> +
> + return rc;
> +out:
> + if (rc > 0)
> + rc = -ENODEV;
> + return rc;
> +}
> +
>  int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>  {
>   struct tpm_chip *chip;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 28b477e8da6a..a99105f1a5c4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -501,6 +501,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void 
> *cmd, int len,
>const char *desc);
>  extern int tpm_get_timeouts(struct tpm_chip *);
>  extern void tpm_gen_interrupt(struct tpm_chip *);
> +int tpm1_auto_startup(struct tpm_chip *chip);
>  extern int tpm_do_selftest(struct tpm_chip *);
>  extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
>  extern int tpm_pm_suspend(struct device *);
> @@ -539,6 +540,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>  ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
>   u32 *value, const char *desc);
>  
> +int tpm2_auto_startup(struct tpm_chip *chip);
>  extern int tpm2_startup(struct tpm_chip *chip, u16 startup_type);
>

Re: [PATCH v2] tpm: Factor out common startup code

2016-05-16 Thread Jarkko Sakkinen 
On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote:
> Provide some flags in tpm_class_ops to allow drivers to opt-in to the
> common startup sequence. This is the sequence used by tpm_tis and
> tpm_crb.
> 
> All drivers should set this flag.

The commit message should be a much much more verbose I cannot include
this without a better explanation. Please update this for the next
revision.

> Signed-off-by: Jason Gunthorpe 
> Tested-by: Andrew Zamansky 
> ---
>  drivers/char/tpm/st33zp24/st33zp24.c |  4 +---
>  drivers/char/tpm/tpm-chip.c  | 15 ++
>  drivers/char/tpm/tpm-interface.c | 27 
>  drivers/char/tpm/tpm.h   |  2 ++
>  drivers/char/tpm/tpm2-cmd.c  | 40 
> 
>  drivers/char/tpm/tpm_crb.c   | 10 +
>  drivers/char/tpm/tpm_i2c_atmel.c |  6 +-
>  drivers/char/tpm/tpm_i2c_infineon.c  |  4 +---
>  drivers/char/tpm/tpm_i2c_nuvoton.c   |  7 +--
>  drivers/char/tpm/tpm_tis.c   | 24 +-
>  include/linux/tpm.h  |  6 ++
>  11 files changed, 96 insertions(+), 49 deletions(-)
> 
> v2 has a little typo fix From Andrew in the call to tpm2_startup
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
> b/drivers/char/tpm/st33zp24/st33zp24.c
> index 8d626784cd8d..4556c95f83cb 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -532,6 +532,7 @@ static bool st33zp24_req_canceled(struct tpm_chip *chip, 
> u8 status)
>  }
>  
>  static const struct tpm_class_ops st33zp24_tpm = {
> + .flags = TPM_OPS_AUTO_STARTUP,
>   .send = st33zp24_send,
>   .recv = st33zp24_recv,
>   .cancel = st33zp24_cancel,
> @@ -618,9 +619,6 @@ int st33zp24_probe(void *phy_id, const struct 
> st33zp24_phy_ops *ops,
>   tpm_gen_interrupt(chip);
>   }
>  
> - tpm_get_timeouts(chip);
> - tpm_do_selftest(chip);
> -
>   return tpm_chip_register(chip);
>  _tpm_clean_answer:
>   dev_info(>dev, "TPM initialization fail\n");
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 274dd0123237..9a36cedd94eb 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -223,6 +223,21 @@ int tpm_chip_register(struct tpm_chip *chip)
>  {
>   int rc;
>  
> + if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
> + rc = tpm2_probe(chip);
> + if (rc)
> + return rc;
> + }

Dead code.

> +
> + if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + rc = tpm2_auto_startup(chip);
> + else
> + rc = tpm1_auto_startup(chip);
> + if (rc)
> + return rc;
> + }
> +
>   rc = tpm1_chip_register(chip);
>   if (rc)
>   return rc;
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index e2fa89c88304..4e6798ab3a90 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -842,6 +842,33 @@ int tpm_do_selftest(struct tpm_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(tpm_do_selftest);
>  
> +/**
> + * tpm1_auto_startup - Perform the standard automatic TPM initialization
> + * sequence
> + * @chip: TPM chip to use
> + *
> + * Returns 0 on success, < 0 in case of fatal error.
> + */
> +int tpm1_auto_startup(struct tpm_chip *chip)
> +{
> + int rc;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto out;
> + rc = tpm_do_selftest(chip);
> + if (rc) {
> + dev_err(>dev, "TPM self test failed\n");
> + goto out;
> + }
> +
> + return rc;
> +out:
> + if (rc > 0)
> + rc = -ENODEV;
> + return rc;
> +}
> +
>  int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>  {
>   struct tpm_chip *chip;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 28b477e8da6a..a99105f1a5c4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -501,6 +501,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void 
> *cmd, int len,
>const char *desc);
>  extern int tpm_get_timeouts(struct tpm_chip *);
>  extern void tpm_gen_interrupt(struct tpm_chip *);
> +int tpm1_auto_startup(struct tpm_chip *chip);
>  extern int tpm_do_selftest(struct tpm_chip *);
>  extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
>  extern int tpm_pm_suspend(struct device *);
> @@ -539,6 +540,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
>  ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
>   u32 *value, const char *desc);
>  
> +int tpm2_auto_startup(struct tpm_chip *chip);
>  extern int tpm2_startup(struct tpm_chip *chip, u16 startup_type);
>  extern void tpm2_shutdown(struct tpm_chip *chip, u16

[PATCH v2] tpm: Factor out common startup code

2016-05-16 Thread Jason Gunthorpe 
Provide some flags in tpm_class_ops to allow drivers to opt-in to the
common startup sequence. This is the sequence used by tpm_tis and
tpm_crb.

All drivers should set this flag.

Signed-off-by: Jason Gunthorpe 
Tested-by: Andrew Zamansky 
---
 drivers/char/tpm/st33zp24/st33zp24.c |  4 +---
 drivers/char/tpm/tpm-chip.c  | 15 ++
 drivers/char/tpm/tpm-interface.c | 27 
 drivers/char/tpm/tpm.h   |  2 ++
 drivers/char/tpm/tpm2-cmd.c  | 40 
 drivers/char/tpm/tpm_crb.c   | 10 +
 drivers/char/tpm/tpm_i2c_atmel.c |  6 +-
 drivers/char/tpm/tpm_i2c_infineon.c  |  4 +---
 drivers/char/tpm/tpm_i2c_nuvoton.c   |  7 +--
 drivers/char/tpm/tpm_tis.c   | 24 +-
 include/linux/tpm.h  |  6 ++
 11 files changed, 96 insertions(+), 49 deletions(-)

v2 has a little typo fix From Andrew in the call to tpm2_startup

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
b/drivers/char/tpm/st33zp24/st33zp24.c
index 8d626784cd8d..4556c95f83cb 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -532,6 +532,7 @@ static bool st33zp24_req_canceled(struct tpm_chip *chip, u8 
status)
 }
 
 static const struct tpm_class_ops st33zp24_tpm = {
+   .flags = TPM_OPS_AUTO_STARTUP,
.send = st33zp24_send,
.recv = st33zp24_recv,
.cancel = st33zp24_cancel,
@@ -618,9 +619,6 @@ int st33zp24_probe(void *phy_id, const struct 
st33zp24_phy_ops *ops,
tpm_gen_interrupt(chip);
}
 
-   tpm_get_timeouts(chip);
-   tpm_do_selftest(chip);
-
return tpm_chip_register(chip);
 _tpm_clean_answer:
dev_info(>dev, "TPM initialization fail\n");
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 274dd0123237..9a36cedd94eb 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -223,6 +223,21 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
int rc;
 
+   if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
+   rc = tpm2_probe(chip);
+   if (rc)
+   return rc;
+   }
+
+   if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
+   if (chip->flags & TPM_CHIP_FLAG_TPM2)
+   rc = tpm2_auto_startup(chip);
+   else
+   rc = tpm1_auto_startup(chip);
+   if (rc)
+   return rc;
+   }
+
rc = tpm1_chip_register(chip);
if (rc)
return rc;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index e2fa89c88304..4e6798ab3a90 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -842,6 +842,33 @@ int tpm_do_selftest(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm_do_selftest);
 
+/**
+ * tpm1_auto_startup - Perform the standard automatic TPM initialization
+ * sequence
+ * @chip: TPM chip to use
+ *
+ * Returns 0 on success, < 0 in case of fatal error.
+ */
+int tpm1_auto_startup(struct tpm_chip *chip)
+{
+   int rc;
+
+   rc = tpm_get_timeouts(chip);
+   if (rc)
+   goto out;
+   rc = tpm_do_selftest(chip);
+   if (rc) {
+   dev_err(>dev, "TPM self test failed\n");
+   goto out;
+   }
+
+   return rc;
+out:
+   if (rc > 0)
+   rc = -ENODEV;
+   return rc;
+}
+
 int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 {
struct tpm_chip *chip;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 28b477e8da6a..a99105f1a5c4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -501,6 +501,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, 
int len,
 const char *desc);
 extern int tpm_get_timeouts(struct tpm_chip *);
 extern void tpm_gen_interrupt(struct tpm_chip *);
+int tpm1_auto_startup(struct tpm_chip *chip);
 extern int tpm_do_selftest(struct tpm_chip *);
 extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
 extern int tpm_pm_suspend(struct device *);
@@ -539,6 +540,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
 
+int tpm2_auto_startup(struct tpm_chip *chip);
 extern int tpm2_startup(struct tpm_chip *chip, u16 startup_type);
 extern void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index b28e4da3d2cf..984190e551a1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -943,3 +943,43 @@ int tpm2_probe(struct tpm_chip *chip)
return 0;
 }

[PATCH v2] tpm: Factor out common startup code

2016-05-16 Thread Jason Gunthorpe 
Provide some flags in tpm_class_ops to allow drivers to opt-in to the
common startup sequence. This is the sequence used by tpm_tis and
tpm_crb.

All drivers should set this flag.

Signed-off-by: Jason Gunthorpe 
Tested-by: Andrew Zamansky 
---
 drivers/char/tpm/st33zp24/st33zp24.c |  4 +---
 drivers/char/tpm/tpm-chip.c  | 15 ++
 drivers/char/tpm/tpm-interface.c | 27 
 drivers/char/tpm/tpm.h   |  2 ++
 drivers/char/tpm/tpm2-cmd.c  | 40 
 drivers/char/tpm/tpm_crb.c   | 10 +
 drivers/char/tpm/tpm_i2c_atmel.c |  6 +-
 drivers/char/tpm/tpm_i2c_infineon.c  |  4 +---
 drivers/char/tpm/tpm_i2c_nuvoton.c   |  7 +--
 drivers/char/tpm/tpm_tis.c   | 24 +-
 include/linux/tpm.h  |  6 ++
 11 files changed, 96 insertions(+), 49 deletions(-)

v2 has a little typo fix From Andrew in the call to tpm2_startup

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
b/drivers/char/tpm/st33zp24/st33zp24.c
index 8d626784cd8d..4556c95f83cb 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -532,6 +532,7 @@ static bool st33zp24_req_canceled(struct tpm_chip *chip, u8 
status)
 }
 
 static const struct tpm_class_ops st33zp24_tpm = {
+   .flags = TPM_OPS_AUTO_STARTUP,
.send = st33zp24_send,
.recv = st33zp24_recv,
.cancel = st33zp24_cancel,
@@ -618,9 +619,6 @@ int st33zp24_probe(void *phy_id, const struct 
st33zp24_phy_ops *ops,
tpm_gen_interrupt(chip);
}
 
-   tpm_get_timeouts(chip);
-   tpm_do_selftest(chip);
-
return tpm_chip_register(chip);
 _tpm_clean_answer:
dev_info(>dev, "TPM initialization fail\n");
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 274dd0123237..9a36cedd94eb 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -223,6 +223,21 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
int rc;
 
+   if (chip->ops->flags & TPM_OPS_PROBE_TPM2) {
+   rc = tpm2_probe(chip);
+   if (rc)
+   return rc;
+   }
+
+   if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
+   if (chip->flags & TPM_CHIP_FLAG_TPM2)
+   rc = tpm2_auto_startup(chip);
+   else
+   rc = tpm1_auto_startup(chip);
+   if (rc)
+   return rc;
+   }
+
rc = tpm1_chip_register(chip);
if (rc)
return rc;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index e2fa89c88304..4e6798ab3a90 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -842,6 +842,33 @@ int tpm_do_selftest(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm_do_selftest);
 
+/**
+ * tpm1_auto_startup - Perform the standard automatic TPM initialization
+ * sequence
+ * @chip: TPM chip to use
+ *
+ * Returns 0 on success, < 0 in case of fatal error.
+ */
+int tpm1_auto_startup(struct tpm_chip *chip)
+{
+   int rc;
+
+   rc = tpm_get_timeouts(chip);
+   if (rc)
+   goto out;
+   rc = tpm_do_selftest(chip);
+   if (rc) {
+   dev_err(>dev, "TPM self test failed\n");
+   goto out;
+   }
+
+   return rc;
+out:
+   if (rc > 0)
+   rc = -ENODEV;
+   return rc;
+}
+
 int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 {
struct tpm_chip *chip;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 28b477e8da6a..a99105f1a5c4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -501,6 +501,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd, 
int len,
 const char *desc);
 extern int tpm_get_timeouts(struct tpm_chip *);
 extern void tpm_gen_interrupt(struct tpm_chip *);
+int tpm1_auto_startup(struct tpm_chip *chip);
 extern int tpm_do_selftest(struct tpm_chip *);
 extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
 extern int tpm_pm_suspend(struct device *);
@@ -539,6 +540,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
 
+int tpm2_auto_startup(struct tpm_chip *chip);
 extern int tpm2_startup(struct tpm_chip *chip, u16 startup_type);
 extern void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index b28e4da3d2cf..984190e551a1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -943,3 +943,43 @@ int tpm2_probe(struct tpm_chip *chip)
return 0;
 }
 EXPORT_SYMBOL_GPL(tpm2_probe);
+
+/**
+ * tpm2_auto_startup - Perform the