All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 11:51 ` Enric Balletbo i Serra
  0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2017-03-01 11:51 UTC (permalink / raw)
  To: Rob Herring, Peter Huewe
  Cc: linux-kernel, tpmdd-devel, Mark Rutland, Jarkko Sakkinen, Sonny Rao

From: Sonny Rao <sonnyrao@chromium.org>

The suspend/resume behavior of the TPM can be controlled
by setting "powered-while-suspended" in the DTS.

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 Documentation/devicetree/bindings/tpm/tpm.txt | 25 +++++++++++++++++++++++++
 drivers/char/tpm/tpm_i2c_infineon.c           | 25 ++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt

diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt b/Documentation/devicetree/bindings/tpm/tpm.txt
new file mode 100644
index 0000000..af4de0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/tpm/tpm.txt
@@ -0,0 +1,25 @@
+TPM (Trusted Platform Module)
+
+A TPM on the I2C bus is a child of the node for the bus.
+
+Required properties:
+- compatible: should be "infineon,<chip>"
+- reg: the I2C address
+
+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:
+	i2c@12C90000 {
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <66000>;
+		gpios = <&gpa1 2 3 3 0>,
+			<&gpa1 3 3 3 0>;
+
+		tpm {
+			compatible = "infineon,slb9635tt";
+			reg = <0x20>;
+			powered-while-suspended;
+		};
+	};
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 62ee44e..19d9522 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -70,6 +70,7 @@ struct tpm_inf_dev {
 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
 	struct tpm_chip *chip;
 	enum i2c_chip_type chip_type;
+	bool powered_while_suspended;
 };
 
 static struct tpm_inf_dev tpm_dev;
@@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
 		goto out_err;
 	}
 
+	if (dev->of_node &&
+	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
+		tpm_dev.powered_while_suspended = true;
+	}
+
 	/* read four bytes from DID_VID register */
 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
 		dev_err(dev, "could not read vendor id\n");
@@ -662,7 +668,24 @@ static const struct of_device_id tpm_tis_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
 #endif
 
-static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend, tpm_pm_resume);
+static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
+{
+	if (tpm_dev.powered_while_suspended)
+		return 0;
+
+	return tpm_pm_suspend(dev);
+}
+
+static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
+{
+	if (tpm_dev.powered_while_suspended)
+		return 0;
+
+	return tpm_pm_resume(dev);
+}
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
+			 tpm_tis_i2c_resume);
 
 static int tpm_tis_i2c_probe(struct i2c_client *client,
 			     const struct i2c_device_id *id)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 11:51 ` Enric Balletbo i Serra
  0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2017-03-01 11:51 UTC (permalink / raw)
  To: Rob Herring, Peter Huewe
  Cc: Mark Rutland, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Sonny Rao, linux-kernel-u79uwXL29TY76Z2rM5mHXA

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.

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>
---
 Documentation/devicetree/bindings/tpm/tpm.txt | 25 +++++++++++++++++++++++++
 drivers/char/tpm/tpm_i2c_infineon.c           | 25 ++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt

diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt b/Documentation/devicetree/bindings/tpm/tpm.txt
new file mode 100644
index 0000000..af4de0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/tpm/tpm.txt
@@ -0,0 +1,25 @@
+TPM (Trusted Platform Module)
+
+A TPM on the I2C bus is a child of the node for the bus.
+
+Required properties:
+- compatible: should be "infineon,<chip>"
+- reg: the I2C address
+
+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:
+	i2c@12C90000 {
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <66000>;
+		gpios = <&gpa1 2 3 3 0>,
+			<&gpa1 3 3 3 0>;
+
+		tpm {
+			compatible = "infineon,slb9635tt";
+			reg = <0x20>;
+			powered-while-suspended;
+		};
+	};
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 62ee44e..19d9522 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -70,6 +70,7 @@ struct tpm_inf_dev {
 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
 	struct tpm_chip *chip;
 	enum i2c_chip_type chip_type;
+	bool powered_while_suspended;
 };
 
 static struct tpm_inf_dev tpm_dev;
@@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
 		goto out_err;
 	}
 
+	if (dev->of_node &&
+	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
+		tpm_dev.powered_while_suspended = true;
+	}
+
 	/* read four bytes from DID_VID register */
 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
 		dev_err(dev, "could not read vendor id\n");
@@ -662,7 +668,24 @@ static const struct of_device_id tpm_tis_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
 #endif
 
-static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend, tpm_pm_resume);
+static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
+{
+	if (tpm_dev.powered_while_suspended)
+		return 0;
+
+	return tpm_pm_suspend(dev);
+}
+
+static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
+{
+	if (tpm_dev.powered_while_suspended)
+		return 0;
+
+	return tpm_pm_resume(dev);
+}
+
+static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
+			 tpm_tis_i2c_resume);
 
 static int tpm_tis_i2c_probe(struct i2c_client *client,
 			     const struct i2c_device_id *id)
-- 
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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 12:00   ` Peter Huewe
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Huewe @ 2017-03-01 12:00 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Rob Herring
  Cc: linux-kernel, tpmdd-devel, Mark Rutland, Jarkko Sakkinen, Sonny Rao



Am 1. März 2017 12:51:16 MEZ schrieb Enric Balletbo i Serra <enric.balletbo@collabora.com>:
>From: Sonny Rao <sonnyrao@chromium.org>
>
>The suspend/resume behavior of the TPM can be controlled
>by setting "powered-while-suspended" in the DTS.
>
>Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>---
>Documentation/devicetree/bindings/tpm/tpm.txt | 25
>+++++++++++++++++++++++++
>drivers/char/tpm/tpm_i2c_infineon.c           | 25
>++++++++++++++++++++++++-
> 2 files changed, 49 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt
>
>diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt
>b/Documentation/devicetree/bindings/tpm/tpm.txt
>new file mode 100644
>index 0000000..af4de0d
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/tpm/tpm.txt

Hi, for this device there exists a binding in the i2c subfolder

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/i2c/trivial-devices.txt?id=refs/tags/v4.10

Peter
>@@ -0,0 +1,25 @@
>+TPM (Trusted Platform Module)
>+
>+A TPM on the I2C bus is a child of the node for the bus.
>+
>+Required properties:
>+- compatible: should be "infineon,<chip>"
>+- reg: the I2C address
>+
>+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:
>+	i2c@12C90000 {
>+		samsung,i2c-sda-delay = <100>;
>+		samsung,i2c-max-bus-freq = <66000>;
>+		gpios = <&gpa1 2 3 3 0>,
>+			<&gpa1 3 3 3 0>;
>+
>+		tpm {
>+			compatible = "infineon,slb9635tt";
>+			reg = <0x20>;
>+			powered-while-suspended;
>+		};
>+	};
>diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
>b/drivers/char/tpm/tpm_i2c_infineon.c
>index 62ee44e..19d9522 100644
>--- a/drivers/char/tpm/tpm_i2c_infineon.c
>+++ b/drivers/char/tpm/tpm_i2c_infineon.c
>@@ -70,6 +70,7 @@ struct tpm_inf_dev {
> 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
> 	struct tpm_chip *chip;
> 	enum i2c_chip_type chip_type;
>+	bool powered_while_suspended;
> };
> 
> static struct tpm_inf_dev tpm_dev;
>@@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
> 		goto out_err;
> 	}
> 
>+	if (dev->of_node &&
>+	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
>+		tpm_dev.powered_while_suspended = true;
>+	}
>+
> 	/* read four bytes from DID_VID register */
> 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
> 		dev_err(dev, "could not read vendor id\n");
>@@ -662,7 +668,24 @@ static const struct of_device_id
>tpm_tis_i2c_of_match[] = {
> MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
> #endif
> 
>-static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend,
>tpm_pm_resume);
>+static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
>+{
>+	if (tpm_dev.powered_while_suspended)
>+		return 0;
>+
>+	return tpm_pm_suspend(dev);
>+}
>+
>+static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
>+{
>+	if (tpm_dev.powered_while_suspended)
>+		return 0;
>+
>+	return tpm_pm_resume(dev);
>+}
>+
>+static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
>+			 tpm_tis_i2c_resume);
> 
> static int tpm_tis_i2c_probe(struct i2c_client *client,
> 			     const struct i2c_device_id *id)

-- 
Sent from my mobile

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 12:00   ` Peter Huewe
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Huewe @ 2017-03-01 12:00 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Rob Herring
  Cc: Mark Rutland, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Sonny Rao, linux-kernel-u79uwXL29TY76Z2rM5mHXA



Am 1. März 2017 12:51:16 MEZ schrieb Enric Balletbo i Serra <enric.balletbo@collabora.com>:
>From: Sonny Rao <sonnyrao@chromium.org>
>
>The suspend/resume behavior of the TPM can be controlled
>by setting "powered-while-suspended" in the DTS.
>
>Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>---
>Documentation/devicetree/bindings/tpm/tpm.txt | 25
>+++++++++++++++++++++++++
>drivers/char/tpm/tpm_i2c_infineon.c           | 25
>++++++++++++++++++++++++-
> 2 files changed, 49 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt
>
>diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt
>b/Documentation/devicetree/bindings/tpm/tpm.txt
>new file mode 100644
>index 0000000..af4de0d
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/tpm/tpm.txt

Hi, for this device there exists a binding in the i2c subfolder

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/i2c/trivial-devices.txt?id=refs/tags/v4.10

Peter
>@@ -0,0 +1,25 @@
>+TPM (Trusted Platform Module)
>+
>+A TPM on the I2C bus is a child of the node for the bus.
>+
>+Required properties:
>+- compatible: should be "infineon,<chip>"
>+- reg: the I2C address
>+
>+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:
>+	i2c@12C90000 {
>+		samsung,i2c-sda-delay = <100>;
>+		samsung,i2c-max-bus-freq = <66000>;
>+		gpios = <&gpa1 2 3 3 0>,
>+			<&gpa1 3 3 3 0>;
>+
>+		tpm {
>+			compatible = "infineon,slb9635tt";
>+			reg = <0x20>;
>+			powered-while-suspended;
>+		};
>+	};
>diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
>b/drivers/char/tpm/tpm_i2c_infineon.c
>index 62ee44e..19d9522 100644
>--- a/drivers/char/tpm/tpm_i2c_infineon.c
>+++ b/drivers/char/tpm/tpm_i2c_infineon.c
>@@ -70,6 +70,7 @@ struct tpm_inf_dev {
> 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
> 	struct tpm_chip *chip;
> 	enum i2c_chip_type chip_type;
>+	bool powered_while_suspended;
> };
> 
> static struct tpm_inf_dev tpm_dev;
>@@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
> 		goto out_err;
> 	}
> 
>+	if (dev->of_node &&
>+	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
>+		tpm_dev.powered_while_suspended = true;
>+	}
>+
> 	/* read four bytes from DID_VID register */
> 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
> 		dev_err(dev, "could not read vendor id\n");
>@@ -662,7 +668,24 @@ static const struct of_device_id
>tpm_tis_i2c_of_match[] = {
> MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
> #endif
> 
>-static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend,
>tpm_pm_resume);
>+static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
>+{
>+	if (tpm_dev.powered_while_suspended)
>+		return 0;
>+
>+	return tpm_pm_suspend(dev);
>+}
>+
>+static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
>+{
>+	if (tpm_dev.powered_while_suspended)
>+		return 0;
>+
>+	return tpm_pm_resume(dev);
>+}
>+
>+static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
>+			 tpm_tis_i2c_resume);
> 
> static int tpm_tis_i2c_probe(struct i2c_client *client,
> 			     const struct i2c_device_id *id)

-- 
Sent from my mobile

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 12:08     ` Enric Balletbo i Serra
  0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2017-03-01 12:08 UTC (permalink / raw)
  To: Peter Huewe, Rob Herring
  Cc: linux-kernel, tpmdd-devel, Mark Rutland, Jarkko Sakkinen, Sonny Rao

Hi Peter,

On 01/03/17 13:00, Peter Huewe wrote:
> 
> 
> Am 1. März 2017 12:51:16 MEZ schrieb Enric Balletbo i Serra <enric.balletbo@collabora.com>:
>> From: Sonny Rao <sonnyrao@chromium.org>
>>
>> The suspend/resume behavior of the TPM can be controlled
>> by setting "powered-while-suspended" in the DTS.
>>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>> Documentation/devicetree/bindings/tpm/tpm.txt | 25
>> +++++++++++++++++++++++++
>> drivers/char/tpm/tpm_i2c_infineon.c           | 25
>> ++++++++++++++++++++++++-
>> 2 files changed, 49 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt
>> b/Documentation/devicetree/bindings/tpm/tpm.txt
>> new file mode 100644
>> index 0000000..af4de0d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tpm/tpm.txt
> 
> Hi, for this device there exists a binding in the i2c subfolder
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/i2c/trivial-devices.txt?id=refs/tags/v4.10
> 

Thanks to catch this, I propose remove from trivial-devices.txt and create a new binding called Documentation/devicetree/bindings/i2c/i2c-tpm-infineon.txt for tpm-infineon devices? What do you think?

Thanks,
 Enric

> Peter
>> @@ -0,0 +1,25 @@
>> +TPM (Trusted Platform Module)
>> +
>> +A TPM on the I2C bus is a child of the node for the bus.
>> +
>> +Required properties:
>> +- compatible: should be "infineon,<chip>"
>> +- reg: the I2C address
>> +
>> +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:
>> +	i2c@12C90000 {
>> +		samsung,i2c-sda-delay = <100>;
>> +		samsung,i2c-max-bus-freq = <66000>;
>> +		gpios = <&gpa1 2 3 3 0>,
>> +			<&gpa1 3 3 3 0>;
>> +
>> +		tpm {
>> +			compatible = "infineon,slb9635tt";
>> +			reg = <0x20>;
>> +			powered-while-suspended;
>> +		};
>> +	};
>> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
>> b/drivers/char/tpm/tpm_i2c_infineon.c
>> index 62ee44e..19d9522 100644
>> --- a/drivers/char/tpm/tpm_i2c_infineon.c
>> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
>> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
>> 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
>> 	struct tpm_chip *chip;
>> 	enum i2c_chip_type chip_type;
>> +	bool powered_while_suspended;
>> };
>>
>> static struct tpm_inf_dev tpm_dev;
>> @@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
>> 		goto out_err;
>> 	}
>>
>> +	if (dev->of_node &&
>> +	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
>> +		tpm_dev.powered_while_suspended = true;
>> +	}
>> +
>> 	/* read four bytes from DID_VID register */
>> 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
>> 		dev_err(dev, "could not read vendor id\n");
>> @@ -662,7 +668,24 @@ static const struct of_device_id
>> tpm_tis_i2c_of_match[] = {
>> MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
>> #endif
>>
>> -static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend,
>> tpm_pm_resume);
>> +static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
>> +{
>> +	if (tpm_dev.powered_while_suspended)
>> +		return 0;
>> +
>> +	return tpm_pm_suspend(dev);
>> +}
>> +
>> +static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
>> +{
>> +	if (tpm_dev.powered_while_suspended)
>> +		return 0;
>> +
>> +	return tpm_pm_resume(dev);
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
>> +			 tpm_tis_i2c_resume);
>>
>> static int tpm_tis_i2c_probe(struct i2c_client *client,
>> 			     const struct i2c_device_id *id)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 12:08     ` Enric Balletbo i Serra
  0 siblings, 0 replies; 13+ messages in thread
From: Enric Balletbo i Serra @ 2017-03-01 12:08 UTC (permalink / raw)
  To: Peter Huewe, Rob Herring
  Cc: Mark Rutland, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Sonny Rao, linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi Peter,

On 01/03/17 13:00, Peter Huewe wrote:
> 
> 
> Am 1. März 2017 12:51:16 MEZ schrieb Enric Balletbo i Serra <enric.balletbo@collabora.com>:
>> From: Sonny Rao <sonnyrao@chromium.org>
>>
>> The suspend/resume behavior of the TPM can be controlled
>> by setting "powered-while-suspended" in the DTS.
>>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>> Documentation/devicetree/bindings/tpm/tpm.txt | 25
>> +++++++++++++++++++++++++
>> drivers/char/tpm/tpm_i2c_infineon.c           | 25
>> ++++++++++++++++++++++++-
>> 2 files changed, 49 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt
>> b/Documentation/devicetree/bindings/tpm/tpm.txt
>> new file mode 100644
>> index 0000000..af4de0d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/tpm/tpm.txt
> 
> Hi, for this device there exists a binding in the i2c subfolder
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/i2c/trivial-devices.txt?id=refs/tags/v4.10
> 

Thanks to catch this, I propose remove from trivial-devices.txt and create a new binding called Documentation/devicetree/bindings/i2c/i2c-tpm-infineon.txt for tpm-infineon devices? What do you think?

Thanks,
 Enric

> Peter
>> @@ -0,0 +1,25 @@
>> +TPM (Trusted Platform Module)
>> +
>> +A TPM on the I2C bus is a child of the node for the bus.
>> +
>> +Required properties:
>> +- compatible: should be "infineon,<chip>"
>> +- reg: the I2C address
>> +
>> +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:
>> +	i2c@12C90000 {
>> +		samsung,i2c-sda-delay = <100>;
>> +		samsung,i2c-max-bus-freq = <66000>;
>> +		gpios = <&gpa1 2 3 3 0>,
>> +			<&gpa1 3 3 3 0>;
>> +
>> +		tpm {
>> +			compatible = "infineon,slb9635tt";
>> +			reg = <0x20>;
>> +			powered-while-suspended;
>> +		};
>> +	};
>> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
>> b/drivers/char/tpm/tpm_i2c_infineon.c
>> index 62ee44e..19d9522 100644
>> --- a/drivers/char/tpm/tpm_i2c_infineon.c
>> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
>> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
>> 	u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
>> 	struct tpm_chip *chip;
>> 	enum i2c_chip_type chip_type;
>> +	bool powered_while_suspended;
>> };
>>
>> static struct tpm_inf_dev tpm_dev;
>> @@ -599,6 +600,11 @@ static int tpm_tis_i2c_init(struct device *dev)
>> 		goto out_err;
>> 	}
>>
>> +	if (dev->of_node &&
>> +	    of_get_property(dev->of_node, "powered-while-suspended", NULL)) {
>> +		tpm_dev.powered_while_suspended = true;
>> +	}
>> +
>> 	/* read four bytes from DID_VID register */
>> 	if (iic_tpm_read(TPM_DID_VID(0), (u8 *)&vendor, 4) < 0) {
>> 		dev_err(dev, "could not read vendor id\n");
>> @@ -662,7 +668,24 @@ static const struct of_device_id
>> tpm_tis_i2c_of_match[] = {
>> MODULE_DEVICE_TABLE(of, tpm_tis_i2c_of_match);
>> #endif
>>
>> -static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_pm_suspend,
>> tpm_pm_resume);
>> +static int __maybe_unused tpm_tis_i2c_suspend(struct device *dev)
>> +{
>> +	if (tpm_dev.powered_while_suspended)
>> +		return 0;
>> +
>> +	return tpm_pm_suspend(dev);
>> +}
>> +
>> +static int __maybe_unused tpm_tis_i2c_resume(struct device *dev)
>> +{
>> +	if (tpm_dev.powered_while_suspended)
>> +		return 0;
>> +
>> +	return tpm_pm_resume(dev);
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(tpm_tis_i2c_ops, tpm_tis_i2c_suspend,
>> +			 tpm_tis_i2c_resume);
>>
>> static int tpm_tis_i2c_probe(struct i2c_client *client,
>> 			     const struct i2c_device_id *id)
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
  2017-03-01 11:51 ` Enric Balletbo i Serra
  (?)
  (?)
@ 2017-03-01 13:54 ` Mark Rutland
  2017-03-01 18:43     ` Jason Gunthorpe
  -1 siblings, 1 reply; 13+ messages in thread
From: Mark Rutland @ 2017-03-01 13:54 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Rob Herring, Peter Huewe, linux-kernel, tpmdd-devel,
	Jarkko Sakkinen, Sonny Rao

On Wed, Mar 01, 2017 at 12:51:16PM +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.
> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  Documentation/devicetree/bindings/tpm/tpm.txt | 25 +++++++++++++++++++++++++
>  drivers/char/tpm/tpm_i2c_infineon.c           | 25 ++++++++++++++++++++++++-
>  2 files changed, 49 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/tpm/tpm.txt
> 
> diff --git a/Documentation/devicetree/bindings/tpm/tpm.txt b/Documentation/devicetree/bindings/tpm/tpm.txt
> new file mode 100644
> index 0000000..af4de0d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/tpm/tpm.txt
> @@ -0,0 +1,25 @@
> +TPM (Trusted Platform Module)
> +
> +A TPM on the I2C bus is a child of the node for the bus.
> +
> +Required properties:
> +- compatible: should be "infineon,<chip>"
> +- reg: the I2C address
> +
> +Optional properties:
> +- powered-while-suspended: present when the TPM is left powered on between
> +  suspend and resume (makes the suspend/resume callbacks do nothing).

This reads like configuration rather than a HW property.

Why do you want this property?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [tpmdd-devel] [PATCH] tpm: do not suspend/resume if power stays on
  2017-03-01 13:54 ` Mark Rutland
@ 2017-03-01 18:43     ` Jason Gunthorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2017-03-01 18:43 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Enric Balletbo i Serra, linux-kernel, Rob Herring, tpmdd-devel,
	Sonny Rao

> > +Optional properties:
> > +- powered-while-suspended: present when the TPM is left powered on between
> > +  suspend and resume (makes the suspend/resume callbacks do nothing).
> 
> This reads like configuration rather than a HW property.

I read this to mean the HW does not cut power to the TPM when Linux
does 'suspend'.

We recently added global suspend/resume callbacks to the TPM
core. Those call backs do not power off the TPM, they just prepare its
internal state to loose power to the chip. Skipping that process on
hardware that does not power-off the TPM makes sense to me.

But, Sonny, perhaps this should be a global flag in tpm_chip, not a
per-interface-driver override?

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 18:43     ` Jason Gunthorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2017-03-01 18:43 UTC (permalink / raw)
  To: Mark Rutland
  Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Rob Herring,
	Sonny Rao, linux-kernel-u79uwXL29TY76Z2rM5mHXA

> > +Optional properties:
> > +- powered-while-suspended: present when the TPM is left powered on between
> > +  suspend and resume (makes the suspend/resume callbacks do nothing).
> 
> This reads like configuration rather than a HW property.

I read this to mean the HW does not cut power to the TPM when Linux
does 'suspend'.

We recently added global suspend/resume callbacks to the TPM
core. Those call backs do not power off the TPM, they just prepare its
internal state to loose power to the chip. Skipping that process on
hardware that does not power-off the TPM makes sense to me.

But, Sonny, perhaps this should be a global flag in tpm_chip, not a
per-interface-driver override?

Jason

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [tpmdd-devel] [PATCH] tpm: do not suspend/resume if power stays on
  2017-03-01 18:43     ` Jason Gunthorpe
  (?)
@ 2017-03-01 22:39     ` Sonny Rao
  2017-03-01 23:18         ` Jason Gunthorpe
  -1 siblings, 1 reply; 13+ messages in thread
From: Sonny Rao @ 2017-03-01 22:39 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Mark Rutland, Enric Balletbo i Serra, linux-kernel, Rob Herring,
	tpmdd-devel

On Wed, Mar 1, 2017 at 10:43 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
>> > +Optional properties:
>> > +- powered-while-suspended: present when the TPM is left powered on between
>> > +  suspend and resume (makes the suspend/resume callbacks do nothing).
>>
>> This reads like configuration rather than a HW property.
>
> I read this to mean the HW does not cut power to the TPM when Linux
> does 'suspend'.

That's correct, it is a hardware property describing whether power is
removed during suspend.

>
> We recently added global suspend/resume callbacks to the TPM
> core. Those call backs do not power off the TPM, they just prepare its
> internal state to loose power to the chip. Skipping that process on
> hardware that does not power-off the TPM makes sense to me.
>
> But, Sonny, perhaps this should be a global flag in tpm_chip, not a
> per-interface-driver override?

It's a property of the board design not the chip -- maybe I'm misunderstanding?

>
> Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [tpmdd-devel] [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 23:18         ` Jason Gunthorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2017-03-01 23:18 UTC (permalink / raw)
  To: Sonny Rao
  Cc: Mark Rutland, Enric Balletbo i Serra, linux-kernel, Rob Herring,
	tpmdd-devel

On Wed, Mar 01, 2017 at 02:39:09PM -0800, Sonny Rao wrote:

> > We recently added global suspend/resume callbacks to the TPM
> > core. Those call backs do not power off the TPM, they just prepare its
> > internal state to loose power to the chip. Skipping that process on
> > hardware that does not power-off the TPM makes sense to me.
> >
> > But, Sonny, perhaps this should be a global flag in tpm_chip, not a
> > per-interface-driver override?
> 
> It's a property of the board design not the chip -- maybe I'm
> misunderstanding?

I mean do not add the code to handle this to tpm_i2c_infineon.c but in
the common chip code instead.

tpm_i2c_infineon.c should only parse DT properties that are relavent
to the bus that delivers commands to the TPM, things that apply to how
a TPM chip operates should be handled in the core code because they
apply to any command transport bus.

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] tpm: do not suspend/resume if power stays on
@ 2017-03-01 23:18         ` Jason Gunthorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2017-03-01 23:18 UTC (permalink / raw)
  To: Sonny Rao
  Cc: Mark Rutland, Rob Herring, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Wed, Mar 01, 2017 at 02:39:09PM -0800, Sonny Rao wrote:

> > We recently added global suspend/resume callbacks to the TPM
> > core. Those call backs do not power off the TPM, they just prepare its
> > internal state to loose power to the chip. Skipping that process on
> > hardware that does not power-off the TPM makes sense to me.
> >
> > But, Sonny, perhaps this should be a global flag in tpm_chip, not a
> > per-interface-driver override?
> 
> It's a property of the board design not the chip -- maybe I'm
> misunderstanding?

I mean do not add the code to handle this to tpm_i2c_infineon.c but in
the common chip code instead.

tpm_i2c_infineon.c should only parse DT properties that are relavent
to the bus that delivers commands to the TPM, things that apply to how
a TPM chip operates should be handled in the core code because they
apply to any command transport bus.

Jason

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [tpmdd-devel] [PATCH] tpm: do not suspend/resume if power stays on
  2017-03-01 23:18         ` Jason Gunthorpe
  (?)
@ 2017-03-02  0:02         ` Sonny Rao
  -1 siblings, 0 replies; 13+ messages in thread
From: Sonny Rao @ 2017-03-02  0:02 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Mark Rutland, Enric Balletbo i Serra, linux-kernel, Rob Herring,
	tpmdd-devel

On Wed, Mar 1, 2017 at 3:18 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Wed, Mar 01, 2017 at 02:39:09PM -0800, Sonny Rao wrote:
>
>> > We recently added global suspend/resume callbacks to the TPM
>> > core. Those call backs do not power off the TPM, they just prepare its
>> > internal state to loose power to the chip. Skipping that process on
>> > hardware that does not power-off the TPM makes sense to me.
>> >
>> > But, Sonny, perhaps this should be a global flag in tpm_chip, not a
>> > per-interface-driver override?
>>
>> It's a property of the board design not the chip -- maybe I'm
>> misunderstanding?
>
> I mean do not add the code to handle this to tpm_i2c_infineon.c but in
> the common chip code instead.
>
> tpm_i2c_infineon.c should only parse DT properties that are relavent
> to the bus that delivers commands to the TPM, things that apply to how
> a TPM chip operates should be handled in the core code because they
> apply to any command transport bus.

Oh right, sorry -- yes this makes perfect sense.

>
> Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-03-02  0:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-01 11:51 [PATCH] tpm: do not suspend/resume if power stays on Enric Balletbo i Serra
2017-03-01 11:51 ` Enric Balletbo i Serra
2017-03-01 12:00 ` Peter Huewe
2017-03-01 12:00   ` Peter Huewe
2017-03-01 12:08   ` Enric Balletbo i Serra
2017-03-01 12:08     ` Enric Balletbo i Serra
2017-03-01 13:54 ` Mark Rutland
2017-03-01 18:43   ` [tpmdd-devel] " Jason Gunthorpe
2017-03-01 18:43     ` Jason Gunthorpe
2017-03-01 22:39     ` [tpmdd-devel] " Sonny Rao
2017-03-01 23:18       ` Jason Gunthorpe
2017-03-01 23:18         ` Jason Gunthorpe
2017-03-02  0:02         ` [tpmdd-devel] " Sonny Rao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.