[PATCH v3] tpm: do not suspend/resume if power stays on

[PATCH v3] tpm: do not suspend/resume if power stays on

[Enric Balletbo i Serra]

From: Sonny Rao <sonnyrao@chromium.org>

The suspend/resume behavior of the TPM can be controlled by setting
"powered-while-suspended" in the DTS. This is useful for the cases
when hardware does not power-off the TPM.

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v2:
 Jarkko Sakkinen
  - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
  - Remove a trailing newline.
Changes since v1:
 Jason Gunthorpe :
  - Move the code to handle suspend/resume in the common chip code.

 Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
 drivers/char/tpm/tpm-interface.c                           | 3 +++
 drivers/char/tpm/tpm.h                                     | 1 +
 drivers/char/tpm/tpm_of.c                                  | 3 +++
 4 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
index 8cb638b..85c8216 100644
--- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
@@ -8,6 +8,12 @@ Required properties:
                    the firmware event log
 - linux,sml-size : size of the memory allocated for the firmware event log
 
+Optional properties:
+
+- powered-while-suspended: present when the TPM is left powered on between
+                           suspend and resume (makes the suspend/resume
+                           callbacks do nothing).
+
 Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
 ----------------------------------------------------------
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a2688ac..6869093 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -944,6 +944,9 @@ int tpm_pm_suspend(struct device *dev)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
+		return 0;
+
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		tpm2_shutdown(chip, TPM2_SU_STATE);
 		return 0;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..0ce379c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -146,6 +146,7 @@ enum tpm_chip_flags {
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
 	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
 	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
+	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
 };
 
 struct tpm_chip_seqops {
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 7dee42d7..e89276c 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
 	else
 		return -ENODEV;
 
+	if (of_property_read_bool(np, "powered-while-suspended"))
+		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
+
 	sizep = of_get_property(np, "linux,sml-size", NULL);
 	basep = of_get_property(np, "linux,sml-base", NULL);
 	if (sizep == NULL && basep == NULL)
-- 
2.9.3

[PATCH v3] tpm: do not suspend/resume if power stays on

[Enric Balletbo i Serra]

From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

The suspend/resume behavior of the TPM can be controlled by setting
"powered-while-suspended" in the DTS. This is useful for the cases
when hardware does not power-off the TPM.

Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
Changes since v2:
 Jarkko Sakkinen
  - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
  - Remove a trailing newline.
Changes since v1:
 Jason Gunthorpe :
  - Move the code to handle suspend/resume in the common chip code.

 Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
 drivers/char/tpm/tpm-interface.c                           | 3 +++
 drivers/char/tpm/tpm.h                                     | 1 +
 drivers/char/tpm/tpm_of.c                                  | 3 +++
 4 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
index 8cb638b..85c8216 100644
--- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
+++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
@@ -8,6 +8,12 @@ Required properties:
                    the firmware event log
 - linux,sml-size : size of the memory allocated for the firmware event log
 
+Optional properties:
+
+- powered-while-suspended: present when the TPM is left powered on between
+                           suspend and resume (makes the suspend/resume
+                           callbacks do nothing).
+
 Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
 ----------------------------------------------------------
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a2688ac..6869093 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -944,6 +944,9 @@ int tpm_pm_suspend(struct device *dev)
 	if (chip == NULL)
 		return -ENODEV;
 
+	if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
+		return 0;
+
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		tpm2_shutdown(chip, TPM2_SU_STATE);
 		return 0;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..0ce379c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -146,6 +146,7 @@ enum tpm_chip_flags {
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
 	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
 	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
+	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
 };
 
 struct tpm_chip_seqops {
diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 7dee42d7..e89276c 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
 	else
 		return -ENODEV;
 
+	if (of_property_read_bool(np, "powered-while-suspended"))
+		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
+
 	sizep = of_get_property(np, "linux,sml-size", NULL);
 	basep = of_get_property(np, "linux,sml-base", NULL);
 	if (sizep == NULL && basep == NULL)
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jason Gunthorpe]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao@chromium.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

> +++ b/drivers/char/tpm/tpm_of.c
> @@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
>  	else
>  		return -ENODEV;
>  
> +	if (of_property_read_bool(np, "powered-while-suspended"))
> +		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +

It is really weird that this is in a function called tpm_read_log_of,
but it makes sense to use the existing conditional infrastructure for
OF as well.

We should try to tidy this later...

Jason

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jason Gunthorpe]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> +++ b/drivers/char/tpm/tpm_of.c
> @@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
>  	else
>  		return -ENODEV;
>  
> +	if (of_property_read_bool(np, "powered-while-suspended"))
> +		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +

It is really weird that this is in a function called tpm_read_log_of,
but it makes sense to use the existing conditional infrastructure for
OF as well.

We should try to tidy this later...

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao@chromium.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks. Looks good to me.

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> index 8cb638b..85c8216 100644
> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> @@ -8,6 +8,12 @@ Required properties:
>                     the firmware event log
>  - linux,sml-size : size of the memory allocated for the firmware event log
>  
> +Optional properties:
> +
> +- powered-while-suspended: present when the TPM is left powered on between
> +                           suspend and resume (makes the suspend/resume
> +                           callbacks do nothing).
> +
>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>  ----------------------------------------------------------
>  
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a2688ac..6869093 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -944,6 +944,9 @@ int tpm_pm_suspend(struct device *dev)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> +	if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> +		return 0;
> +
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>  		tpm2_shutdown(chip, TPM2_SU_STATE);
>  		return 0;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1ae9768..0ce379c 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -146,6 +146,7 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_IRQ		= BIT(2),
>  	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
>  	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
> +	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
>  };
>  
>  struct tpm_chip_seqops {
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 7dee42d7..e89276c 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
>  	else
>  		return -ENODEV;
>  
> +	if (of_property_read_bool(np, "powered-while-suspended"))
> +		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +
>  	sizep = of_get_property(np, "linux,sml-size", NULL);
>  	basep = of_get_property(np, "linux,sml-base", NULL);
>  	if (sizep == NULL && basep == NULL)
> -- 
> 2.9.3
> 

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Thanks. Looks good to me.

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> index 8cb638b..85c8216 100644
> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> @@ -8,6 +8,12 @@ Required properties:
>                     the firmware event log
>  - linux,sml-size : size of the memory allocated for the firmware event log
>  
> +Optional properties:
> +
> +- powered-while-suspended: present when the TPM is left powered on between
> +                           suspend and resume (makes the suspend/resume
> +                           callbacks do nothing).
> +
>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>  ----------------------------------------------------------
>  
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index a2688ac..6869093 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -944,6 +944,9 @@ int tpm_pm_suspend(struct device *dev)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> +	if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> +		return 0;
> +
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>  		tpm2_shutdown(chip, TPM2_SU_STATE);
>  		return 0;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 1ae9768..0ce379c 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -146,6 +146,7 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_IRQ		= BIT(2),
>  	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
>  	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
> +	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
>  };
>  
>  struct tpm_chip_seqops {
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 7dee42d7..e89276c 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -34,6 +34,9 @@ int tpm_read_log_of(struct tpm_chip *chip)
>  	else
>  		return -ENODEV;
>  
> +	if (of_property_read_bool(np, "powered-while-suspended"))
> +		chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +
>  	sizep = of_get_property(np, "linux,sml-size", NULL);
>  	basep = of_get_property(np, "linux,sml-base", NULL);
>  	if (sizep == NULL && basep == NULL)
> -- 
> 2.9.3
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao@chromium.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> index 8cb638b..85c8216 100644
> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> @@ -8,6 +8,12 @@ Required properties:
>                     the firmware event log
>  - linux,sml-size : size of the memory allocated for the firmware event log
>  
> +Optional properties:
> +
> +- powered-while-suspended: present when the TPM is left powered on between
> +                           suspend and resume (makes the suspend/resume
> +                           callbacks do nothing).
> +
>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>  ----------------------------------------------------------

Hey, just noticed something. Shouldn't this be a separate commit?
I'm also wondering whether this can be submitted through my tree
upper maintainers.

Does not change my reviewed-by for the actual code change but you
would have to split this into a patch set if this is the case.

/Jarkko

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> index 8cb638b..85c8216 100644
> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> @@ -8,6 +8,12 @@ Required properties:
>                     the firmware event log
>  - linux,sml-size : size of the memory allocated for the firmware event log
>  
> +Optional properties:
> +
> +- powered-while-suspended: present when the TPM is left powered on between
> +                           suspend and resume (makes the suspend/resume
> +                           callbacks do nothing).
> +
>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>  ----------------------------------------------------------

Hey, just noticed something. Shouldn't this be a separate commit?
I'm also wondering whether this can be submitted through my tree
upper maintainers.

Does not change my reviewed-by for the actual code change but you
would have to split this into a patch set if this is the case.

/Jarkko

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Enric Balletbo i Serra]

Hi Jarkko,

On 06/03/17 21:59, Jarkko Sakkinen wrote:
> On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
>> From: Sonny Rao <sonnyrao@chromium.org>
>>
>> The suspend/resume behavior of the TPM can be controlled by setting
>> "powered-while-suspended" in the DTS. This is useful for the cases
>> when hardware does not power-off the TPM.
>>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>> Changes since v2:
>>  Jarkko Sakkinen
>>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>>   - Remove a trailing newline.
>> Changes since v1:
>>  Jason Gunthorpe :
>>   - Move the code to handle suspend/resume in the common chip code.
>>
>>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>>  drivers/char/tpm/tpm.h                                     | 1 +
>>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>>  4 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> index 8cb638b..85c8216 100644
>> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> @@ -8,6 +8,12 @@ Required properties:
>>                     the firmware event log
>>  - linux,sml-size : size of the memory allocated for the firmware event log
>>  
>> +Optional properties:
>> +
>> +- powered-while-suspended: present when the TPM is left powered on between
>> +                           suspend and resume (makes the suspend/resume
>> +                           callbacks do nothing).
>> +
>>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>>  ----------------------------------------------------------
> 
> Hey, just noticed something. Shouldn't this be a separate commit?

During my life submitting patches I saw the both options, sometimes the maintainer asked me to join the DT patch and the driver and sometimes he asked me to do in different patches, so I think this is more a maintainer option. Maybe Rob Herring or Mark Rutland can share their preferences?

I'll do what you want I do, TBH I don't have a strong opinion about this.

> I'm also wondering whether this can be submitted through my tree
> upper maintainers.
> 
> Does not change my reviewed-by for the actual code change but you
> would have to split this into a patch set if this is the case.
> 
> /Jarkko
> 

Cheers,
 Enric

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Enric Balletbo i Serra]

Hi Jarkko,

On 06/03/17 21:59, Jarkko Sakkinen wrote:
> On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
>> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> The suspend/resume behavior of the TPM can be controlled by setting
>> "powered-while-suspended" in the DTS. This is useful for the cases
>> when hardware does not power-off the TPM.
>>
>> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
>> ---
>> Changes since v2:
>>  Jarkko Sakkinen
>>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>>   - Remove a trailing newline.
>> Changes since v1:
>>  Jason Gunthorpe :
>>   - Move the code to handle suspend/resume in the common chip code.
>>
>>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
>>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>>  drivers/char/tpm/tpm.h                                     | 1 +
>>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>>  4 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> index 8cb638b..85c8216 100644
>> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
>> @@ -8,6 +8,12 @@ Required properties:
>>                     the firmware event log
>>  - linux,sml-size : size of the memory allocated for the firmware event log
>>  
>> +Optional properties:
>> +
>> +- powered-while-suspended: present when the TPM is left powered on between
>> +                           suspend and resume (makes the suspend/resume
>> +                           callbacks do nothing).
>> +
>>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
>>  ----------------------------------------------------------
> 
> Hey, just noticed something. Shouldn't this be a separate commit?

During my life submitting patches I saw the both options, sometimes the maintainer asked me to join the DT patch and the driver and sometimes he asked me to do in different patches, so I think this is more a maintainer option. Maybe Rob Herring or Mark Rutland can share their preferences?

I'll do what you want I do, TBH I don't have a strong opinion about this.

> I'm also wondering whether this can be submitted through my tree
> upper maintainers.
> 
> Does not change my reviewed-by for the actual code change but you
> would have to split this into a patch set if this is the case.
> 
> /Jarkko
> 

Cheers,
 Enric

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Tue, Mar 07, 2017 at 09:44:55AM +0100, Enric Balletbo i Serra wrote:
> Hi Jarkko,
> 
> On 06/03/17 21:59, Jarkko Sakkinen wrote:
> > On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> >> From: Sonny Rao <sonnyrao@chromium.org>
> >>
> >> The suspend/resume behavior of the TPM can be controlled by setting
> >> "powered-while-suspended" in the DTS. This is useful for the cases
> >> when hardware does not power-off the TPM.
> >>
> >> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >> Changes since v2:
> >>  Jarkko Sakkinen
> >>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
> >>   - Remove a trailing newline.
> >> Changes since v1:
> >>  Jason Gunthorpe :
> >>   - Move the code to handle suspend/resume in the common chip code.
> >>
> >>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
> >>  drivers/char/tpm/tpm-interface.c                           | 3 +++
> >>  drivers/char/tpm/tpm.h                                     | 1 +
> >>  drivers/char/tpm/tpm_of.c                                  | 3 +++
> >>  4 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> index 8cb638b..85c8216 100644
> >> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> @@ -8,6 +8,12 @@ Required properties:
> >>                     the firmware event log
> >>  - linux,sml-size : size of the memory allocated for the firmware event log
> >>  
> >> +Optional properties:
> >> +
> >> +- powered-while-suspended: present when the TPM is left powered on between
> >> +                           suspend and resume (makes the suspend/resume
> >> +                           callbacks do nothing).
> >> +
> >>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
> >>  ----------------------------------------------------------
> > 
> > Hey, just noticed something. Shouldn't this be a separate commit?
> 
> During my life submitting patches I saw the both options, sometimes
> the maintainer asked me to join the DT patch and the driver and
> sometimes he asked me to do in different patches, so I think this is
> more a maintainer option. Maybe Rob Herring or Mark Rutland can share
> their preferences?
> 
> I'll do what you want I do, TBH I don't have a strong opinion about this.

I'll add this to my tree and put it to my next branch so that
it will flow to linux-next since it only touch TPM specific
file in bindings.

I do not have perfect answer for this so I'll use common sense
(or try to).

Rob, is this OK?

/Jarkko

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Jarkko Sakkinen]

On Tue, Mar 07, 2017 at 09:44:55AM +0100, Enric Balletbo i Serra wrote:
> Hi Jarkko,
> 
> On 06/03/17 21:59, Jarkko Sakkinen wrote:
> > On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> >> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >>
> >> The suspend/resume behavior of the TPM can be controlled by setting
> >> "powered-while-suspended" in the DTS. This is useful for the cases
> >> when hardware does not power-off the TPM.
> >>
> >> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> >> ---
> >> Changes since v2:
> >>  Jarkko Sakkinen
> >>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
> >>   - Remove a trailing newline.
> >> Changes since v1:
> >>  Jason Gunthorpe :
> >>   - Move the code to handle suspend/resume in the common chip code.
> >>
> >>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
> >>  drivers/char/tpm/tpm-interface.c                           | 3 +++
> >>  drivers/char/tpm/tpm.h                                     | 1 +
> >>  drivers/char/tpm/tpm_of.c                                  | 3 +++
> >>  4 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> index 8cb638b..85c8216 100644
> >> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> @@ -8,6 +8,12 @@ Required properties:
> >>                     the firmware event log
> >>  - linux,sml-size : size of the memory allocated for the firmware event log
> >>  
> >> +Optional properties:
> >> +
> >> +- powered-while-suspended: present when the TPM is left powered on between
> >> +                           suspend and resume (makes the suspend/resume
> >> +                           callbacks do nothing).
> >> +
> >>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
> >>  ----------------------------------------------------------
> > 
> > Hey, just noticed something. Shouldn't this be a separate commit?
> 
> During my life submitting patches I saw the both options, sometimes
> the maintainer asked me to join the DT patch and the driver and
> sometimes he asked me to do in different patches, so I think this is
> more a maintainer option. Maybe Rob Herring or Mark Rutland can share
> their preferences?
> 
> I'll do what you want I do, TBH I don't have a strong opinion about this.

I'll add this to my tree and put it to my next branch so that
it will flow to linux-next since it only touch TPM specific
file in bindings.

I do not have perfect answer for this so I'll use common sense
(or try to).

Rob, is this OK?

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Rob Herring]

On Tue, Mar 07, 2017 at 09:44:55AM +0100, Enric Balletbo i Serra wrote:
> Hi Jarkko,
> 
> On 06/03/17 21:59, Jarkko Sakkinen wrote:
> > On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> >> From: Sonny Rao <sonnyrao@chromium.org>
> >>
> >> The suspend/resume behavior of the TPM can be controlled by setting
> >> "powered-while-suspended" in the DTS. This is useful for the cases
> >> when hardware does not power-off the TPM.
> >>
> >> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >> Changes since v2:
> >>  Jarkko Sakkinen
> >>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
> >>   - Remove a trailing newline.
> >> Changes since v1:
> >>  Jason Gunthorpe :
> >>   - Move the code to handle suspend/resume in the common chip code.
> >>
> >>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
> >>  drivers/char/tpm/tpm-interface.c                           | 3 +++
> >>  drivers/char/tpm/tpm.h                                     | 1 +
> >>  drivers/char/tpm/tpm_of.c                                  | 3 +++
> >>  4 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> index 8cb638b..85c8216 100644
> >> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> @@ -8,6 +8,12 @@ Required properties:
> >>                     the firmware event log
> >>  - linux,sml-size : size of the memory allocated for the firmware event log
> >>  
> >> +Optional properties:
> >> +
> >> +- powered-while-suspended: present when the TPM is left powered on between
> >> +                           suspend and resume (makes the suspend/resume
> >> +                           callbacks do nothing).
> >> +
> >>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
> >>  ----------------------------------------------------------
> > 
> > Hey, just noticed something. Shouldn't this be a separate commit?
> 
> During my life submitting patches I saw the both options, sometimes the maintainer asked me to join the DT patch and the driver and sometimes he asked me to do in different patches, so I think this is more a maintainer option. Maybe Rob Herring or Mark Rutland can share their preferences?
> 

Need to fix your mailer to wrap lines...

We prefer DT bindings to be separate commits, but don't enforce it. I 
don't ask for on small changes or if there's no other reason to respin 
the patch.

It makes the history for the DT only tree generated from git 
filter-branch cleaner. Any other maintainers tell you to combine them, 
they need to talk to me.

Rob

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Rob Herring]

On Tue, Mar 07, 2017 at 09:44:55AM +0100, Enric Balletbo i Serra wrote:
> Hi Jarkko,
> 
> On 06/03/17 21:59, Jarkko Sakkinen wrote:
> > On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> >> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >>
> >> The suspend/resume behavior of the TPM can be controlled by setting
> >> "powered-while-suspended" in the DTS. This is useful for the cases
> >> when hardware does not power-off the TPM.
> >>
> >> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> >> ---
> >> Changes since v2:
> >>  Jarkko Sakkinen
> >>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
> >>   - Remove a trailing newline.
> >> Changes since v1:
> >>  Jason Gunthorpe :
> >>   - Move the code to handle suspend/resume in the common chip code.
> >>
> >>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++
> >>  drivers/char/tpm/tpm-interface.c                           | 3 +++
> >>  drivers/char/tpm/tpm.h                                     | 1 +
> >>  drivers/char/tpm/tpm_of.c                                  | 3 +++
> >>  4 files changed, 13 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> index 8cb638b..85c8216 100644
> >> --- a/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> +++ b/Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt
> >> @@ -8,6 +8,12 @@ Required properties:
> >>                     the firmware event log
> >>  - linux,sml-size : size of the memory allocated for the firmware event log
> >>  
> >> +Optional properties:
> >> +
> >> +- powered-while-suspended: present when the TPM is left powered on between
> >> +                           suspend and resume (makes the suspend/resume
> >> +                           callbacks do nothing).
> >> +
> >>  Example (for OpenPower Systems with Nuvoton TPM 2.0 on I2C)
> >>  ----------------------------------------------------------
> > 
> > Hey, just noticed something. Shouldn't this be a separate commit?
> 
> During my life submitting patches I saw the both options, sometimes the maintainer asked me to join the DT patch and the driver and sometimes he asked me to do in different patches, so I think this is more a maintainer option. Maybe Rob Herring or Mark Rutland can share their preferences?
> 

Need to fix your mailer to wrap lines...

We prefer DT bindings to be separate commits, but don't enforce it. I 
don't ask for on small changes or if there's no other reason to respin 
the patch.

It makes the history for the DT only tree generated from git 
filter-branch cleaner. Any other maintainers tell you to combine them, 
they need to talk to me.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Rob Herring]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao@chromium.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++

I would say this shouldn't be TPM specific, but looks like we already 
have a dts with this, so:

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)

Re: [PATCH v3] tpm: do not suspend/resume if power stays on

[Rob Herring]

On Fri, Mar 03, 2017 at 05:09:59PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
> Changes since v2:
>  Jarkko Sakkinen
>   - Add a new TPM_CHIP_FLAG_ALWAYS_POWERED flag instead of using a boolean variable.
>   - Remove a trailing newline.
> Changes since v1:
>  Jason Gunthorpe :
>   - Move the code to handle suspend/resume in the common chip code.
> 
>  Documentation/devicetree/bindings/security/tpm/tpm-i2c.txt | 6 ++++++

I would say this shouldn't be TPM specific, but looks like we already 
have a dts with this, so:

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/char/tpm/tpm-interface.c                           | 3 +++
>  drivers/char/tpm/tpm.h                                     | 1 +
>  drivers/char/tpm/tpm_of.c                                  | 3 +++
>  4 files changed, 13 insertions(+)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html