From: Sonny Rao <[email protected]>
The suspend/resume behavior of the TPM can be controlled
by setting "powered-while-suspended" in the DTS.
Signed-off-by: Sonny Rao <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---
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
Am 1. März 2017 12:51:16 MEZ schrieb Enric Balletbo i Serra <[email protected]>:
>From: Sonny Rao <[email protected]>
>
>The suspend/resume behavior of the TPM can be controlled
>by setting "powered-while-suspended" in the DTS.
>
>Signed-off-by: Sonny Rao <[email protected]>
>Signed-off-by: Enric Balletbo i Serra <[email protected]>
>---
>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
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 <[email protected]>:
>> From: Sonny Rao <[email protected]>
>>
>> The suspend/resume behavior of the TPM can be controlled
>> by setting "powered-while-suspended" in the DTS.
>>
>> Signed-off-by: Sonny Rao <[email protected]>
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>> ---
>> 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)
>
On Wed, Mar 01, 2017 at 12:51:16PM +0100, Enric Balletbo i Serra wrote:
> From: Sonny Rao <[email protected]>
>
> The suspend/resume behavior of the TPM can be controlled
> by setting "powered-while-suspended" in the DTS.
>
> Signed-off-by: Sonny Rao <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
> 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.
> > +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
On Wed, Mar 1, 2017 at 10:43 AM, Jason Gunthorpe
<[email protected]> 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
On Wed, Mar 1, 2017 at 3:18 PM, Jason Gunthorpe
<[email protected]> 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
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