Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1481250 > unrolled thread

[PATCH v3 0/4] tpm/tpm_crb: implement power management.

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/4] tpm/tpm_crb: implement power management. Tomas Winkler <tomas.winkler@intel.com> - 2016-09-12 15:10 +0200
    [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add Tomas Winkler <tomas.winkler@intel.com> - 2016-09-12 15:10 +0200
      Re: [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-09-15 08:30 +0200
    Re: [PATCH v3 0/4] tpm/tpm_crb: implement power management. "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-09-14 08:30 +0200
      Re: [PATCH v3 0/4] tpm/tpm_crb: implement power management. Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-09-14 18:10 +0200
        Re: [PATCH v3 0/4] tpm/tpm_crb: implement power management. Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-09-14 18:10 +0200
          Re: [PATCH v3 0/4] tpm/tpm_crb: implement power management. Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2016-09-14 20:30 +0200

#1481250 — [PATCH v3 0/4] tpm/tpm_crb: implement power management.

Te overall platform ability to enter a low power state is also
conditioned on the ability of a tpm device to go to idle state.
This series should provide this feature.

Unfortunately, there is a HW bug on Intel PTT devices on Skylake,
Kabylake, and Broxton devices, where certain registers lost retention
during TPM idle state. Hence this implementation takes this into
consideration and implement the feature based only on access to
registers that retain their state. This still conforms to the spec
and should be correct also on non Intle devices.

V2: Utilize runtime_pm for driving tpm crb idle states.
V3. fix lower case corruption in the first patch

Tomas Winkler (4):
  tpm/tpm_crb: implement tpm crb idle state
  tmp/tpm_crb: fix Intel PTT hw bug during idle state
  tpm/tpm_crb: open code the crb_init into acpi_add
  tmp/tpm_crb: implment runtime pm for tpm_crb

 drivers/char/tpm/tpm-interface.c |   5 ++
 drivers/char/tpm/tpm_crb.c       | 166 +++++++++++++++++++++++++++++++++------
 2 files changed, 147 insertions(+), 24 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1481259 — [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add

This is preparation step for implementing tpm crb
runtime pm. We need to have tpm chip allocated
and populated before we access the runtime handlers.

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: new in the series
V3: resend
 drivers/char/tpm/tpm_crb.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index e945177cf2c8..b0c0e2c9022b 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -265,21 +265,6 @@ static const struct tpm_class_ops tpm_crb = {
 	.req_complete_val = CRB_DRV_STS_COMPLETE,
 };
 
-static int crb_init(struct acpi_device *device, struct crb_priv *priv)
-{
-	struct tpm_chip *chip;
-
-	chip = tpmm_chip_alloc(&device->dev, &tpm_crb);
-	if (IS_ERR(chip))
-		return PTR_ERR(chip);
-
-	dev_set_drvdata(&chip->dev, priv);
-	chip->acpi_dev_handle = device->handle;
-	chip->flags = TPM_CHIP_FLAG_TPM2;
-
-	return tpm_chip_register(chip);
-}
-
 static int crb_check_resource(struct acpi_resource *ares, void *data)
 {
 	struct resource *io_res = data;
@@ -401,6 +386,7 @@ static int crb_acpi_add(struct acpi_device *device)
 {
 	struct acpi_table_tpm2 *buf;
 	struct crb_priv *priv;
+	struct tpm_chip *chip;
 	struct device *dev = &device->dev;
 	acpi_status status;
 	u32 sm;
@@ -438,11 +424,19 @@ static int crb_acpi_add(struct acpi_device *device)
 	if (rc)
 		return rc;
 
+	chip = tpmm_chip_alloc(dev, &tpm_crb);
+	if (IS_ERR(chip))
+		return PTR_ERR(chip);
+
+	dev_set_drvdata(&chip->dev, priv);
+	chip->acpi_dev_handle = device->handle;
+	chip->flags = TPM_CHIP_FLAG_TPM2;
+
 	rc  = crb_cmd_ready(dev, priv);
 	if (rc)
 		return rc;
 
-	rc = crb_init(device, priv);
+	rc = tpm_chip_register(chip);
 	if (rc)
 		crb_go_idle(dev, priv);
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1483867 — Re: [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add

On Mon, Sep 12, 2016 at 04:04:20PM +0300, Tomas Winkler wrote:
> This is preparation step for implementing tpm crb
> runtime pm. We need to have tpm chip allocated
> and populated before we access the runtime handlers.
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>

Use pm_runtime_put().

Tested-by: Jarkko Sakkinen <jarkko.sakkinn@linux.intel.com>

> ---
> V2: new in the series
> V3: resend
>  drivers/char/tpm/tpm_crb.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index e945177cf2c8..b0c0e2c9022b 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -265,21 +265,6 @@ static const struct tpm_class_ops tpm_crb = {
>  	.req_complete_val = CRB_DRV_STS_COMPLETE,
>  };
>  
> -static int crb_init(struct acpi_device *device, struct crb_priv *priv)
> -{
> -	struct tpm_chip *chip;
> -
> -	chip = tpmm_chip_alloc(&device->dev, &tpm_crb);
> -	if (IS_ERR(chip))
> -		return PTR_ERR(chip);
> -
> -	dev_set_drvdata(&chip->dev, priv);
> -	chip->acpi_dev_handle = device->handle;
> -	chip->flags = TPM_CHIP_FLAG_TPM2;
> -
> -	return tpm_chip_register(chip);
> -}
> -
>  static int crb_check_resource(struct acpi_resource *ares, void *data)
>  {
>  	struct resource *io_res = data;
> @@ -401,6 +386,7 @@ static int crb_acpi_add(struct acpi_device *device)
>  {
>  	struct acpi_table_tpm2 *buf;
>  	struct crb_priv *priv;
> +	struct tpm_chip *chip;
>  	struct device *dev = &device->dev;
>  	acpi_status status;
>  	u32 sm;
> @@ -438,11 +424,19 @@ static int crb_acpi_add(struct acpi_device *device)
>  	if (rc)
>  		return rc;
>  
> +	chip = tpmm_chip_alloc(dev, &tpm_crb);
> +	if (IS_ERR(chip))
> +		return PTR_ERR(chip);
> +
> +	dev_set_drvdata(&chip->dev, priv);
> +	chip->acpi_dev_handle = device->handle;
> +	chip->flags = TPM_CHIP_FLAG_TPM2;
> +
>  	rc  = crb_cmd_ready(dev, priv);
>  	if (rc)
>  		return rc;
>  
> -	rc = crb_init(device, priv);
> +	rc = tpm_chip_register(chip);
>  	if (rc)
>  		crb_go_idle(dev, priv);
>  
> -- 
> 2.7.4
> 

[toc] | [prev] | [next] | [standalone]

#1482954

On Mon, 2016-09-12 at 16:04 +0300, Tomas Winkler wrote:
> Te overall platform ability to enter a low power state is also
> conditioned on the ability of a tpm device to go to idle state.
> This series should provide this feature.
> 
> Unfortunately, there is a HW bug on Intel PTT devices on Skylake,
> Kabylake, and Broxton devices, where certain registers lost retention
> during TPM idle state. Hence this implementation takes this into
> consideration and implement the feature based only on access to
> registers that retain their state. This still conforms to the spec
> and should be correct also on non Intle devices.
> 
> V2: Utilize runtime_pm for driving tpm crb idle states.
> V3. fix lower case corruption in the first patch
> 
Jarkko, had you chance to test v3 series one on your side, is this okay
to go?

Thanks
Tomas

[toc] | [prev] | [next] | [standalone]

#1483451

On Wed, Sep 14, 2016 at 06:28:03AM +0000, Winkler, Tomas wrote:
> On Mon, 2016-09-12 at 16:04 +0300, Tomas Winkler wrote:
> > Te overall platform ability to enter a low power state is also
> > conditioned on the ability of a tpm device to go to idle state.
> > This series should provide this feature.
> > 
> > Unfortunately, there is a HW bug on Intel PTT devices on Skylake,
> > Kabylake, and Broxton devices, where certain registers lost retention
> > during TPM idle state. Hence this implementation takes this into
> > consideration and implement the feature based only on access to
> > registers that retain their state. This still conforms to the spec
> > and should be correct also on non Intle devices.
> > 
> > V2: Utilize runtime_pm for driving tpm crb idle states.
> > V3. fix lower case corruption in the first patch
> > 
> Jarkko, had you chance to test v3 series one on your side, is this okay
> to go?

Opens for me are:

- pm_runtime_put_sync() is still used 
- callback names
- wait_for_tpm_stat

> Thanks
> Tomas

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1483452

On Wed, Sep 14, 2016 at 07:06:02PM +0300, Jarkko Sakkinen wrote:
> On Wed, Sep 14, 2016 at 06:28:03AM +0000, Winkler, Tomas wrote:
> > On Mon, 2016-09-12 at 16:04 +0300, Tomas Winkler wrote:
> > > Te overall platform ability to enter a low power state is also
> > > conditioned on the ability of a tpm device to go to idle state.
> > > This series should provide this feature.
> > > 
> > > Unfortunately, there is a HW bug on Intel PTT devices on Skylake,
> > > Kabylake, and Broxton devices, where certain registers lost retention
> > > during TPM idle state. Hence this implementation takes this into
> > > consideration and implement the feature based only on access to
> > > registers that retain their state. This still conforms to the spec
> > > and should be correct also on non Intle devices.
> > > 
> > > V2: Utilize runtime_pm for driving tpm crb idle states.
> > > V3. fix lower case corruption in the first patch
> > > 
> > Jarkko, had you chance to test v3 series one on your side, is this okay
> > to go?
> 
> Opens for me are:
> 
> - pm_runtime_put_sync() is still used 
> - callback names
> - wait_for_tpm_stat

I also tested v3 and it works for me.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1483530

On Wed, Sep 14, 2016 at 07:06:52PM +0300, Jarkko Sakkinen wrote:
> On Wed, Sep 14, 2016 at 07:06:02PM +0300, Jarkko Sakkinen wrote:
> > On Wed, Sep 14, 2016 at 06:28:03AM +0000, Winkler, Tomas wrote:
> > > On Mon, 2016-09-12 at 16:04 +0300, Tomas Winkler wrote:
> > > > Te overall platform ability to enter a low power state is also
> > > > conditioned on the ability of a tpm device to go to idle state.
> > > > This series should provide this feature.
> > > > 
> > > > Unfortunately, there is a HW bug on Intel PTT devices on Skylake,
> > > > Kabylake, and Broxton devices, where certain registers lost retention
> > > > during TPM idle state. Hence this implementation takes this into
> > > > consideration and implement the feature based only on access to
> > > > registers that retain their state. This still conforms to the spec
> > > > and should be correct also on non Intle devices.
> > > > 
> > > > V2: Utilize runtime_pm for driving tpm crb idle states.
> > > > V3. fix lower case corruption in the first patch
> > > > 
> > > Jarkko, had you chance to test v3 series one on your side, is this okay
> > > to go?
> > 
> > Opens for me are:
> > 
> > - pm_runtime_put_sync() is still used 
> > - callback names
> > - wait_for_tpm_stat

I think I got you wit with_for_tpm_stat. It's CRB specific internal
thing and thus using wait_for_tpm_stat would be abusing that function
and making it dependent on CRB driver internals.

Could you add a note on this to the commit message and I would be
find with your implementation.

I started to think about this after I wrote about callback names.

/Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web