* [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-12 13:04 ` Tomas Winkler
@ 2016-09-12 13:04 ` Tomas Winkler
-1 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jason Gunthorpe,
Jarkko Sakkinen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
SW to indicate that the device can enter or should exit the idle state.
The legacy ACPI-start (SMI + DMA) based devices do not support these
bits and the idle state management is not exposed to the host SW.
Thus, this functionality only is enabled only for a CRB start (MMIO)
based devices.
Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
oringal patch:
'tpm_crb: implement power tpm crb power management'
Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
V2: do not export the functions via tpm ops
V3: fix lower case corruption; adjust function documentation
drivers/char/tpm/tpm_crb.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 6e9d1bca712f..b6923a8b3ff7 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -83,6 +83,75 @@ struct crb_priv {
u32 cmd_size;
};
+/**
+ * crb_go_idle - request tpm crb device to go the idle state
+ *
+ * @dev: crb device
+ * @priv: crb private data
+ *
+ * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
+ * The device should respond within TIMEOUT_C by clearing the bit.
+ * Anyhow, we do not wait here as a consequent CMD_READY request
+ * will be handled correctly even if idle was not completed.
+ *
+ * The function does nothing for devices with ACPI-start method.
+ *
+ * Return: 0 always
+ */
+static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
+{
+ if (priv->flags & CRB_FL_ACPI_START)
+ return 0;
+
+ iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
+ /* we don't really care when this settles */
+
+ return 0;
+}
+
+/**
+ * crb_cmd_ready - request tpm crb device to enter ready state
+ *
+ * @dev: crb device
+ * @priv: crb private data
+ *
+ * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
+ * and poll till the device acknowledge it by clearing the bit.
+ * The device should respond within TIMEOUT_C.
+ *
+ * The function does nothing for devices with ACPI-start method
+ *
+ * Return: 0 on success -ETIME on timeout;
+ */
+static int __maybe_unused crb_cmd_ready(struct device *dev,
+ struct crb_priv *priv)
+{
+ ktime_t stop, start;
+
+ if (priv->flags & CRB_FL_ACPI_START)
+ return 0;
+
+ iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
+
+ start = ktime_get();
+ stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
+ do {
+ if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
+ dev_dbg(dev, "cmdReady in %lld usecs\n",
+ ktime_to_us(ktime_sub(ktime_get(), start)));
+ return 0;
+ }
+ usleep_range(50, 100);
+ } while (ktime_before(ktime_get(), stop));
+
+ if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
+ dev_warn(dev, "cmdReady timed out\n");
+ return -ETIME;
+ }
+
+ return 0;
+}
+
static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
static u8 crb_status(struct tpm_chip *chip)
--
2.7.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-12 13:04 ` Tomas Winkler
0 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel, Jason Gunthorpe, Jarkko Sakkinen; +Cc: linux-kernel, Tomas Winkler
The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
SW to indicate that the device can enter or should exit the idle state.
The legacy ACPI-start (SMI + DMA) based devices do not support these
bits and the idle state management is not exposed to the host SW.
Thus, this functionality only is enabled only for a CRB start (MMIO)
based devices.
Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
oringal patch:
'tpm_crb: implement power tpm crb power management'
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: do not export the functions via tpm ops
V3: fix lower case corruption; adjust function documentation
drivers/char/tpm/tpm_crb.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 6e9d1bca712f..b6923a8b3ff7 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -83,6 +83,75 @@ struct crb_priv {
u32 cmd_size;
};
+/**
+ * crb_go_idle - request tpm crb device to go the idle state
+ *
+ * @dev: crb device
+ * @priv: crb private data
+ *
+ * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
+ * The device should respond within TIMEOUT_C by clearing the bit.
+ * Anyhow, we do not wait here as a consequent CMD_READY request
+ * will be handled correctly even if idle was not completed.
+ *
+ * The function does nothing for devices with ACPI-start method.
+ *
+ * Return: 0 always
+ */
+static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
+{
+ if (priv->flags & CRB_FL_ACPI_START)
+ return 0;
+
+ iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
+ /* we don't really care when this settles */
+
+ return 0;
+}
+
+/**
+ * crb_cmd_ready - request tpm crb device to enter ready state
+ *
+ * @dev: crb device
+ * @priv: crb private data
+ *
+ * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
+ * and poll till the device acknowledge it by clearing the bit.
+ * The device should respond within TIMEOUT_C.
+ *
+ * The function does nothing for devices with ACPI-start method
+ *
+ * Return: 0 on success -ETIME on timeout;
+ */
+static int __maybe_unused crb_cmd_ready(struct device *dev,
+ struct crb_priv *priv)
+{
+ ktime_t stop, start;
+
+ if (priv->flags & CRB_FL_ACPI_START)
+ return 0;
+
+ iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
+
+ start = ktime_get();
+ stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
+ do {
+ if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
+ dev_dbg(dev, "cmdReady in %lld usecs\n",
+ ktime_to_us(ktime_sub(ktime_get(), start)));
+ return 0;
+ }
+ usleep_range(50, 100);
+ } while (ktime_before(ktime_get(), stop));
+
+ if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
+ dev_warn(dev, "cmdReady timed out\n");
+ return -ETIME;
+ }
+
+ return 0;
+}
+
static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
static u8 crb_status(struct tpm_chip *chip)
--
2.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread[parent not found: <1473685461-6540-2-git-send-email-tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-12 13:04 ` Tomas Winkler
@ 2016-09-15 6:22 ` Jarkko Sakkinen
-1 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 6:22 UTC (permalink / raw)
To: Tomas Winkler
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> SW to indicate that the device can enter or should exit the idle state.
>
> The legacy ACPI-start (SMI + DMA) based devices do not support these
> bits and the idle state management is not exposed to the host SW.
> Thus, this functionality only is enabled only for a CRB start (MMIO)
> based devices.
>
> Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> oringal patch:
> 'tpm_crb: implement power tpm crb power management'
>
>
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> V2: do not export the functions via tpm ops
> V3: fix lower case corruption; adjust function documentation
>
> drivers/char/tpm/tpm_crb.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 6e9d1bca712f..b6923a8b3ff7 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -83,6 +83,75 @@ struct crb_priv {
> u32 cmd_size;
> };
>
> +/**
> + * crb_go_idle - request tpm crb device to go the idle state
> + *
> + * @dev: crb device
> + * @priv: crb private data
> + *
> + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> + * The device should respond within TIMEOUT_C by clearing the bit.
> + * Anyhow, we do not wait here as a consequent CMD_READY request
> + * will be handled correctly even if idle was not completed.
> + *
> + * The function does nothing for devices with ACPI-start method.
> + *
> + * Return: 0 always
> + */
> +static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
> +{
> + if (priv->flags & CRB_FL_ACPI_START)
> + return 0;
> +
> + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> + /* we don't really care when this settles */
> +
> + return 0;
> +}
> +
> +/**
> + * crb_cmd_ready - request tpm crb device to enter ready state
> + *
> + * @dev: crb device
> + * @priv: crb private data
> + *
> + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> + * and poll till the device acknowledge it by clearing the bit.
> + * The device should respond within TIMEOUT_C.
> + *
> + * The function does nothing for devices with ACPI-start method
> + *
> + * Return: 0 on success -ETIME on timeout;
> + */
> +static int __maybe_unused crb_cmd_ready(struct device *dev,
> + struct crb_priv *priv)
> +{
> + ktime_t stop, start;
> +
> + if (priv->flags & CRB_FL_ACPI_START)
> + return 0;
> +
> + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> +
> + start = ktime_get();
> + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> + do {
> + if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
> + dev_dbg(dev, "cmdReady in %lld usecs\n",
> + ktime_to_us(ktime_sub(ktime_get(), start)));
> + return 0;
> + }
> + usleep_range(50, 100);
> + } while (ktime_before(ktime_get(), stop));
Since this is HW specific this is right thing to do and not abuse
wait_for_tpm_stat. However, this should be documented to the commit
message.
> +
> + if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
> + dev_warn(dev, "cmdReady timed out\n");
> + return -ETIME;
> + }
> +
> + return 0;
> +}
> +
> static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
>
> static u8 crb_status(struct tpm_chip *chip)
> --
> 2.7.4
>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
/Jarkko
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 6:22 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 6:22 UTC (permalink / raw)
To: Tomas Winkler; +Cc: tpmdd-devel, Jason Gunthorpe, linux-kernel
On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> SW to indicate that the device can enter or should exit the idle state.
>
> The legacy ACPI-start (SMI + DMA) based devices do not support these
> bits and the idle state management is not exposed to the host SW.
> Thus, this functionality only is enabled only for a CRB start (MMIO)
> based devices.
>
> Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> oringal patch:
> 'tpm_crb: implement power tpm crb power management'
>
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> V2: do not export the functions via tpm ops
> V3: fix lower case corruption; adjust function documentation
>
> drivers/char/tpm/tpm_crb.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 6e9d1bca712f..b6923a8b3ff7 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -83,6 +83,75 @@ struct crb_priv {
> u32 cmd_size;
> };
>
> +/**
> + * crb_go_idle - request tpm crb device to go the idle state
> + *
> + * @dev: crb device
> + * @priv: crb private data
> + *
> + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> + * The device should respond within TIMEOUT_C by clearing the bit.
> + * Anyhow, we do not wait here as a consequent CMD_READY request
> + * will be handled correctly even if idle was not completed.
> + *
> + * The function does nothing for devices with ACPI-start method.
> + *
> + * Return: 0 always
> + */
> +static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
> +{
> + if (priv->flags & CRB_FL_ACPI_START)
> + return 0;
> +
> + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> + /* we don't really care when this settles */
> +
> + return 0;
> +}
> +
> +/**
> + * crb_cmd_ready - request tpm crb device to enter ready state
> + *
> + * @dev: crb device
> + * @priv: crb private data
> + *
> + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> + * and poll till the device acknowledge it by clearing the bit.
> + * The device should respond within TIMEOUT_C.
> + *
> + * The function does nothing for devices with ACPI-start method
> + *
> + * Return: 0 on success -ETIME on timeout;
> + */
> +static int __maybe_unused crb_cmd_ready(struct device *dev,
> + struct crb_priv *priv)
> +{
> + ktime_t stop, start;
> +
> + if (priv->flags & CRB_FL_ACPI_START)
> + return 0;
> +
> + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> +
> + start = ktime_get();
> + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> + do {
> + if (!(ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY)) {
> + dev_dbg(dev, "cmdReady in %lld usecs\n",
> + ktime_to_us(ktime_sub(ktime_get(), start)));
> + return 0;
> + }
> + usleep_range(50, 100);
> + } while (ktime_before(ktime_get(), stop));
Since this is HW specific this is right thing to do and not abuse
wait_for_tpm_stat. However, this should be documented to the commit
message.
> +
> + if (ioread32(&priv->cca->req) & CRB_CTRL_REQ_CMD_READY) {
> + dev_warn(dev, "cmdReady timed out\n");
> + return -ETIME;
> + }
> +
> + return 0;
> +}
> +
> static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
>
> static u8 crb_status(struct tpm_chip *chip)
> --
> 2.7.4
>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread[parent not found: <20160915062204.GA12289-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-15 6:22 ` Jarkko Sakkinen
@ 2016-09-15 6:28 ` Winkler, Tomas
-1 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 6:28 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
>
> On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> > SW to indicate that the device can enter or should exit the idle state.
> >
> > The legacy ACPI-start (SMI + DMA) based devices do not support these
> > bits and the idle state management is not exposed to the host SW.
> > Thus, this functionality only is enabled only for a CRB start (MMIO)
> > based devices.
> >
> > Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> oringal
> > patch:
> > 'tpm_crb: implement power tpm crb power management'
> >
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > V2: do not export the functions via tpm ops
> > V3: fix lower case corruption; adjust function documentation
> >
> > drivers/char/tpm/tpm_crb.c | 69
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 6e9d1bca712f..b6923a8b3ff7 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -83,6 +83,75 @@ struct crb_priv {
> > u32 cmd_size;
> > };
> >
> > +/**
> > + * crb_go_idle - request tpm crb device to go the idle state
> > + *
> > + * @dev: crb device
> > + * @priv: crb private data
> > + *
> > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > + * The device should respond within TIMEOUT_C by clearing the bit.
> > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > + * will be handled correctly even if idle was not completed.
> > + *
> > + * The function does nothing for devices with ACPI-start method.
> > + *
> > + * Return: 0 always
> > + */
> > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > +crb_priv *priv) {
> > + if (priv->flags & CRB_FL_ACPI_START)
> > + return 0;
> > +
> > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > + /* we don't really care when this settles */
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * crb_cmd_ready - request tpm crb device to enter ready state
> > + *
> > + * @dev: crb device
> > + * @priv: crb private data
> > + *
> > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > + * and poll till the device acknowledge it by clearing the bit.
> > + * The device should respond within TIMEOUT_C.
> > + *
> > + * The function does nothing for devices with ACPI-start method
> > + *
> > + * Return: 0 on success -ETIME on timeout; */ static int
> > +__maybe_unused crb_cmd_ready(struct device *dev,
> > + struct crb_priv *priv)
> > +{
> > + ktime_t stop, start;
> > +
> > + if (priv->flags & CRB_FL_ACPI_START)
> > + return 0;
> > +
> > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > +
> > + start = ktime_get();
> > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > + do {
> > + if (!(ioread32(&priv->cca->req) &
> CRB_CTRL_REQ_CMD_READY)) {
> > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > + return 0;
> > + }
> > + usleep_range(50, 100);
> > + } while (ktime_before(ktime_get(), stop));
>
> Since this is HW specific this is right thing to do and not abuse
> wait_for_tpm_stat. However, this should be documented to the commit
> message.
I will respin just this patch and not the whole series, as the fix is only in the commit message.
Tomas
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> /Jarkko
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 6:28 ` Winkler, Tomas
0 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 6:28 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe,
linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
>
> On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> > SW to indicate that the device can enter or should exit the idle state.
> >
> > The legacy ACPI-start (SMI + DMA) based devices do not support these
> > bits and the idle state management is not exposed to the host SW.
> > Thus, this functionality only is enabled only for a CRB start (MMIO)
> > based devices.
> >
> > Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> oringal
> > patch:
> > 'tpm_crb: implement power tpm crb power management'
> >
> >
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> > V2: do not export the functions via tpm ops
> > V3: fix lower case corruption; adjust function documentation
> >
> > drivers/char/tpm/tpm_crb.c | 69
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > index 6e9d1bca712f..b6923a8b3ff7 100644
> > --- a/drivers/char/tpm/tpm_crb.c
> > +++ b/drivers/char/tpm/tpm_crb.c
> > @@ -83,6 +83,75 @@ struct crb_priv {
> > u32 cmd_size;
> > };
> >
> > +/**
> > + * crb_go_idle - request tpm crb device to go the idle state
> > + *
> > + * @dev: crb device
> > + * @priv: crb private data
> > + *
> > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > + * The device should respond within TIMEOUT_C by clearing the bit.
> > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > + * will be handled correctly even if idle was not completed.
> > + *
> > + * The function does nothing for devices with ACPI-start method.
> > + *
> > + * Return: 0 always
> > + */
> > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > +crb_priv *priv) {
> > + if (priv->flags & CRB_FL_ACPI_START)
> > + return 0;
> > +
> > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > + /* we don't really care when this settles */
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * crb_cmd_ready - request tpm crb device to enter ready state
> > + *
> > + * @dev: crb device
> > + * @priv: crb private data
> > + *
> > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > + * and poll till the device acknowledge it by clearing the bit.
> > + * The device should respond within TIMEOUT_C.
> > + *
> > + * The function does nothing for devices with ACPI-start method
> > + *
> > + * Return: 0 on success -ETIME on timeout; */ static int
> > +__maybe_unused crb_cmd_ready(struct device *dev,
> > + struct crb_priv *priv)
> > +{
> > + ktime_t stop, start;
> > +
> > + if (priv->flags & CRB_FL_ACPI_START)
> > + return 0;
> > +
> > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > +
> > + start = ktime_get();
> > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > + do {
> > + if (!(ioread32(&priv->cca->req) &
> CRB_CTRL_REQ_CMD_READY)) {
> > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > + return 0;
> > + }
> > + usleep_range(50, 100);
> > + } while (ktime_before(ktime_get(), stop));
>
> Since this is HW specific this is right thing to do and not abuse
> wait_for_tpm_stat. However, this should be documented to the commit
> message.
I will respin just this patch and not the whole series, as the fix is only in the commit message.
Tomas
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> /Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread[parent not found: <5B8DA87D05A7694D9FA63FD143655C1B542DBC0D-Jy8z56yoSI8MvF1YICWikbfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-15 6:28 ` Winkler, Tomas
@ 2016-09-15 8:20 ` Jarkko Sakkinen
-1 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 8:20 UTC (permalink / raw)
To: Winkler, Tomas
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Thu, Sep 15, 2016 at 06:28:27AM +0000, Winkler, Tomas wrote:
> > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
> >
> > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> > > SW to indicate that the device can enter or should exit the idle state.
> > >
> > > The legacy ACPI-start (SMI + DMA) based devices do not support these
> > > bits and the idle state management is not exposed to the host SW.
> > > Thus, this functionality only is enabled only for a CRB start (MMIO)
> > > based devices.
> > >
> > > Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> oringal
> > > patch:
> > > 'tpm_crb: implement power tpm crb power management'
> > >
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > ---
> > > V2: do not export the functions via tpm ops
> > > V3: fix lower case corruption; adjust function documentation
> > >
> > > drivers/char/tpm/tpm_crb.c | 69
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > index 6e9d1bca712f..b6923a8b3ff7 100644
> > > --- a/drivers/char/tpm/tpm_crb.c
> > > +++ b/drivers/char/tpm/tpm_crb.c
> > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > u32 cmd_size;
> > > };
> > >
> > > +/**
> > > + * crb_go_idle - request tpm crb device to go the idle state
> > > + *
> > > + * @dev: crb device
> > > + * @priv: crb private data
> > > + *
> > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > + * will be handled correctly even if idle was not completed.
> > > + *
> > > + * The function does nothing for devices with ACPI-start method.
> > > + *
> > > + * Return: 0 always
> > > + */
> > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > +crb_priv *priv) {
> > > + if (priv->flags & CRB_FL_ACPI_START)
> > > + return 0;
> > > +
> > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > + /* we don't really care when this settles */
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > + *
> > > + * @dev: crb device
> > > + * @priv: crb private data
> > > + *
> > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > + * and poll till the device acknowledge it by clearing the bit.
> > > + * The device should respond within TIMEOUT_C.
> > > + *
> > > + * The function does nothing for devices with ACPI-start method
> > > + *
> > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > + struct crb_priv *priv)
> > > +{
> > > + ktime_t stop, start;
> > > +
> > > + if (priv->flags & CRB_FL_ACPI_START)
> > > + return 0;
> > > +
> > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > +
> > > + start = ktime_get();
> > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > + do {
> > > + if (!(ioread32(&priv->cca->req) &
> > CRB_CTRL_REQ_CMD_READY)) {
> > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > + return 0;
> > > + }
> > > + usleep_range(50, 100);
> > > + } while (ktime_before(ktime_get(), stop));
> >
> > Since this is HW specific this is right thing to do and not abuse
> > wait_for_tpm_stat. However, this should be documented to the commit
> > message.
> I will respin just this patch and not the whole series, as the fix is only in the commit message.
> Tomas
Works for me. I can update pm_runtime_sync(). Then I'm ready
to apply these.
/Jarkko
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 8:20 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 8:20 UTC (permalink / raw)
To: Winkler, Tomas
Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe,
linux-kernel@vger.kernel.org
On Thu, Sep 15, 2016 at 06:28:27AM +0000, Winkler, Tomas wrote:
> > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
> >
> > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady for
> > > SW to indicate that the device can enter or should exit the idle state.
> > >
> > > The legacy ACPI-start (SMI + DMA) based devices do not support these
> > > bits and the idle state management is not exposed to the host SW.
> > > Thus, this functionality only is enabled only for a CRB start (MMIO)
> > > based devices.
> > >
> > > Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> oringal
> > > patch:
> > > 'tpm_crb: implement power tpm crb power management'
> > >
> > >
> > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > ---
> > > V2: do not export the functions via tpm ops
> > > V3: fix lower case corruption; adjust function documentation
> > >
> > > drivers/char/tpm/tpm_crb.c | 69
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> > > index 6e9d1bca712f..b6923a8b3ff7 100644
> > > --- a/drivers/char/tpm/tpm_crb.c
> > > +++ b/drivers/char/tpm/tpm_crb.c
> > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > u32 cmd_size;
> > > };
> > >
> > > +/**
> > > + * crb_go_idle - request tpm crb device to go the idle state
> > > + *
> > > + * @dev: crb device
> > > + * @priv: crb private data
> > > + *
> > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > + * will be handled correctly even if idle was not completed.
> > > + *
> > > + * The function does nothing for devices with ACPI-start method.
> > > + *
> > > + * Return: 0 always
> > > + */
> > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > +crb_priv *priv) {
> > > + if (priv->flags & CRB_FL_ACPI_START)
> > > + return 0;
> > > +
> > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > + /* we don't really care when this settles */
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > + *
> > > + * @dev: crb device
> > > + * @priv: crb private data
> > > + *
> > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > + * and poll till the device acknowledge it by clearing the bit.
> > > + * The device should respond within TIMEOUT_C.
> > > + *
> > > + * The function does nothing for devices with ACPI-start method
> > > + *
> > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > + struct crb_priv *priv)
> > > +{
> > > + ktime_t stop, start;
> > > +
> > > + if (priv->flags & CRB_FL_ACPI_START)
> > > + return 0;
> > > +
> > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > +
> > > + start = ktime_get();
> > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > + do {
> > > + if (!(ioread32(&priv->cca->req) &
> > CRB_CTRL_REQ_CMD_READY)) {
> > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > + return 0;
> > > + }
> > > + usleep_range(50, 100);
> > > + } while (ktime_before(ktime_get(), stop));
> >
> > Since this is HW specific this is right thing to do and not abuse
> > wait_for_tpm_stat. However, this should be documented to the commit
> > message.
> I will respin just this patch and not the whole series, as the fix is only in the commit message.
> Tomas
Works for me. I can update pm_runtime_sync(). Then I'm ready
to apply these.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread[parent not found: <20160915082037.GA16233-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-15 8:20 ` Jarkko Sakkinen
@ 2016-09-15 8:23 ` Winkler, Tomas
-1 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 8:23 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > state
> > >
> > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady
> > > > for SW to indicate that the device can enter or should exit the idle state.
> > > >
> > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > these bits and the idle state management is not exposed to the host
> SW.
> > > > Thus, this functionality only is enabled only for a CRB start
> > > > (MMIO) based devices.
> > > >
> > > > Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> oringal
> > > > patch:
> > > > 'tpm_crb: implement power tpm crb power management'
> > > >
> > > >
> > > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > > V2: do not export the functions via tpm ops
> > > > V3: fix lower case corruption; adjust function documentation
> > > >
> > > > drivers/char/tpm/tpm_crb.c | 69
> > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 69 insertions(+)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > 100644
> > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > u32 cmd_size;
> > > > };
> > > >
> > > > +/**
> > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > + *
> > > > + * @dev: crb device
> > > > + * @priv: crb private data
> > > > + *
> > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > > + * will be handled correctly even if idle was not completed.
> > > > + *
> > > > + * The function does nothing for devices with ACPI-start method.
> > > > + *
> > > > + * Return: 0 always
> > > > + */
> > > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > > +crb_priv *priv) {
> > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > + return 0;
> > > > +
> > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > + /* we don't really care when this settles */
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/**
> > > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > > + *
> > > > + * @dev: crb device
> > > > + * @priv: crb private data
> > > > + *
> > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > + * The device should respond within TIMEOUT_C.
> > > > + *
> > > > + * The function does nothing for devices with ACPI-start method
> > > > + *
> > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > + struct crb_priv *priv)
> > > > +{
> > > > + ktime_t stop, start;
> > > > +
> > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > + return 0;
> > > > +
> > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > +
> > > > + start = ktime_get();
> > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > + do {
> > > > + if (!(ioread32(&priv->cca->req) &
> > > CRB_CTRL_REQ_CMD_READY)) {
> > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > > + return 0;
> > > > + }
> > > > + usleep_range(50, 100);
> > > > + } while (ktime_before(ktime_get(), stop));
> > >
> > > Since this is HW specific this is right thing to do and not abuse
> > > wait_for_tpm_stat. However, this should be documented to the commit
> > > message.
> > I will respin just this patch and not the whole series, as the fix is only in the
> commit message.
> > Tomas
>
> Works for me. I can update pm_runtime_sync(). Then I'm ready to apply
> these.
What do you mean by pm_runtime_sync()?
Tomas
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 8:23 ` Winkler, Tomas
0 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 8:23 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe,
linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > state
> > >
> > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady
> > > > for SW to indicate that the device can enter or should exit the idle state.
> > > >
> > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > these bits and the idle state management is not exposed to the host
> SW.
> > > > Thus, this functionality only is enabled only for a CRB start
> > > > (MMIO) based devices.
> > > >
> > > > Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> oringal
> > > > patch:
> > > > 'tpm_crb: implement power tpm crb power management'
> > > >
> > > >
> > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > ---
> > > > V2: do not export the functions via tpm ops
> > > > V3: fix lower case corruption; adjust function documentation
> > > >
> > > > drivers/char/tpm/tpm_crb.c | 69
> > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 69 insertions(+)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > 100644
> > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > u32 cmd_size;
> > > > };
> > > >
> > > > +/**
> > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > + *
> > > > + * @dev: crb device
> > > > + * @priv: crb private data
> > > > + *
> > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > > + * will be handled correctly even if idle was not completed.
> > > > + *
> > > > + * The function does nothing for devices with ACPI-start method.
> > > > + *
> > > > + * Return: 0 always
> > > > + */
> > > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > > +crb_priv *priv) {
> > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > + return 0;
> > > > +
> > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > + /* we don't really care when this settles */
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/**
> > > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > > + *
> > > > + * @dev: crb device
> > > > + * @priv: crb private data
> > > > + *
> > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > + * The device should respond within TIMEOUT_C.
> > > > + *
> > > > + * The function does nothing for devices with ACPI-start method
> > > > + *
> > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > + struct crb_priv *priv)
> > > > +{
> > > > + ktime_t stop, start;
> > > > +
> > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > + return 0;
> > > > +
> > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > +
> > > > + start = ktime_get();
> > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > + do {
> > > > + if (!(ioread32(&priv->cca->req) &
> > > CRB_CTRL_REQ_CMD_READY)) {
> > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > > + return 0;
> > > > + }
> > > > + usleep_range(50, 100);
> > > > + } while (ktime_before(ktime_get(), stop));
> > >
> > > Since this is HW specific this is right thing to do and not abuse
> > > wait_for_tpm_stat. However, this should be documented to the commit
> > > message.
> > I will respin just this patch and not the whole series, as the fix is only in the
> commit message.
> > Tomas
>
> Works for me. I can update pm_runtime_sync(). Then I'm ready to apply
> these.
What do you mean by pm_runtime_sync()?
Tomas
^ permalink raw reply [flat|nested] 33+ messages in thread[parent not found: <5B8DA87D05A7694D9FA63FD143655C1B542DBD21-Jy8z56yoSI8MvF1YICWikbfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-15 8:23 ` Winkler, Tomas
@ 2016-09-15 10:53 ` Jarkko Sakkinen
-1 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 10:53 UTC (permalink / raw)
To: Winkler, Tomas
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Thu, Sep 15, 2016 at 08:23:03AM +0000, Winkler, Tomas wrote:
> > > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > > state
> > > >
> > > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady
> > > > > for SW to indicate that the device can enter or should exit the idle state.
> > > > >
> > > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > > these bits and the idle state management is not exposed to the host
> > SW.
> > > > > Thus, this functionality only is enabled only for a CRB start
> > > > > (MMIO) based devices.
> > > > >
> > > > > Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> oringal
> > > > > patch:
> > > > > 'tpm_crb: implement power tpm crb power management'
> > > > >
> > > > >
> > > > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > ---
> > > > > V2: do not export the functions via tpm ops
> > > > > V3: fix lower case corruption; adjust function documentation
> > > > >
> > > > > drivers/char/tpm/tpm_crb.c | 69
> > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 69 insertions(+)
> > > > >
> > > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > > 100644
> > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > > u32 cmd_size;
> > > > > };
> > > > >
> > > > > +/**
> > > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > > + *
> > > > > + * @dev: crb device
> > > > > + * @priv: crb private data
> > > > > + *
> > > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > > > + * will be handled correctly even if idle was not completed.
> > > > > + *
> > > > > + * The function does nothing for devices with ACPI-start method.
> > > > > + *
> > > > > + * Return: 0 always
> > > > > + */
> > > > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > > > +crb_priv *priv) {
> > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > + return 0;
> > > > > +
> > > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > > + /* we don't really care when this settles */
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > > > + *
> > > > > + * @dev: crb device
> > > > > + * @priv: crb private data
> > > > > + *
> > > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > > + * The device should respond within TIMEOUT_C.
> > > > > + *
> > > > > + * The function does nothing for devices with ACPI-start method
> > > > > + *
> > > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > > + struct crb_priv *priv)
> > > > > +{
> > > > > + ktime_t stop, start;
> > > > > +
> > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > + return 0;
> > > > > +
> > > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > > +
> > > > > + start = ktime_get();
> > > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > > + do {
> > > > > + if (!(ioread32(&priv->cca->req) &
> > > > CRB_CTRL_REQ_CMD_READY)) {
> > > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > > > + return 0;
> > > > > + }
> > > > > + usleep_range(50, 100);
> > > > > + } while (ktime_before(ktime_get(), stop));
> > > >
> > > > Since this is HW specific this is right thing to do and not abuse
> > > > wait_for_tpm_stat. However, this should be documented to the commit
> > > > message.
> > > I will respin just this patch and not the whole series, as the fix is only in the
> > commit message.
> > > Tomas
> >
> > Works for me. I can update pm_runtime_sync(). Then I'm ready to apply
> > these.
>
> What do you mean by pm_runtime_sync()?
Typo. I already commente v2 of the series that pm_runtime_put should be
used instead of pm_runtime_put_sync.
/Jarkko
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 10:53 ` Jarkko Sakkinen
0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 10:53 UTC (permalink / raw)
To: Winkler, Tomas
Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe,
linux-kernel@vger.kernel.org
On Thu, Sep 15, 2016 at 08:23:03AM +0000, Winkler, Tomas wrote:
> > > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > > state
> > > >
> > > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and cmdReady
> > > > > for SW to indicate that the device can enter or should exit the idle state.
> > > > >
> > > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > > these bits and the idle state management is not exposed to the host
> > SW.
> > > > > Thus, this functionality only is enabled only for a CRB start
> > > > > (MMIO) based devices.
> > > > >
> > > > > Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> oringal
> > > > > patch:
> > > > > 'tpm_crb: implement power tpm crb power management'
> > > > >
> > > > >
> > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > ---
> > > > > V2: do not export the functions via tpm ops
> > > > > V3: fix lower case corruption; adjust function documentation
> > > > >
> > > > > drivers/char/tpm/tpm_crb.c | 69
> > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 69 insertions(+)
> > > > >
> > > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > > 100644
> > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > > u32 cmd_size;
> > > > > };
> > > > >
> > > > > +/**
> > > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > > + *
> > > > > + * @dev: crb device
> > > > > + * @priv: crb private data
> > > > > + *
> > > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > > + * The device should respond within TIMEOUT_C by clearing the bit.
> > > > > + * Anyhow, we do not wait here as a consequent CMD_READY request
> > > > > + * will be handled correctly even if idle was not completed.
> > > > > + *
> > > > > + * The function does nothing for devices with ACPI-start method.
> > > > > + *
> > > > > + * Return: 0 always
> > > > > + */
> > > > > +static int __maybe_unused crb_go_idle(struct device *dev, struct
> > > > > +crb_priv *priv) {
> > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > + return 0;
> > > > > +
> > > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > > + /* we don't really care when this settles */
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * crb_cmd_ready - request tpm crb device to enter ready state
> > > > > + *
> > > > > + * @dev: crb device
> > > > > + * @priv: crb private data
> > > > > + *
> > > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > > + * The device should respond within TIMEOUT_C.
> > > > > + *
> > > > > + * The function does nothing for devices with ACPI-start method
> > > > > + *
> > > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > > + struct crb_priv *priv)
> > > > > +{
> > > > > + ktime_t stop, start;
> > > > > +
> > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > + return 0;
> > > > > +
> > > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > > +
> > > > > + start = ktime_get();
> > > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > > + do {
> > > > > + if (!(ioread32(&priv->cca->req) &
> > > > CRB_CTRL_REQ_CMD_READY)) {
> > > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > > + ktime_to_us(ktime_sub(ktime_get(), start)));
> > > > > + return 0;
> > > > > + }
> > > > > + usleep_range(50, 100);
> > > > > + } while (ktime_before(ktime_get(), stop));
> > > >
> > > > Since this is HW specific this is right thing to do and not abuse
> > > > wait_for_tpm_stat. However, this should be documented to the commit
> > > > message.
> > > I will respin just this patch and not the whole series, as the fix is only in the
> > commit message.
> > > Tomas
> >
> > Works for me. I can update pm_runtime_sync(). Then I'm ready to apply
> > these.
>
> What do you mean by pm_runtime_sync()?
Typo. I already commente v2 of the series that pm_runtime_put should be
used instead of pm_runtime_put_sync.
/Jarkko
^ permalink raw reply [flat|nested] 33+ messages in thread[parent not found: <20160915105347.GB22431-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
2016-09-15 10:53 ` Jarkko Sakkinen
@ 2016-09-15 13:09 ` Winkler, Tomas
-1 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 13:09 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> On Thu, Sep 15, 2016 at 08:23:03AM +0000, Winkler, Tomas wrote:
> > > > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > > > state
> > > > >
> > > > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and
> > > > > > cmdReady for SW to indicate that the device can enter or should exit
> the idle state.
> > > > > >
> > > > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > > > these bits and the idle state management is not exposed to the
> > > > > > host
> > > SW.
> > > > > > Thus, this functionality only is enabled only for a CRB start
> > > > > > (MMIO) based devices.
> > > > > >
> > > > > > Based on Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > > > > > oringal
> > > > > > patch:
> > > > > > 'tpm_crb: implement power tpm crb power management'
> > > > > >
> > > > > >
> > > > > > Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > > > > ---
> > > > > > V2: do not export the functions via tpm ops
> > > > > > V3: fix lower case corruption; adjust function documentation
> > > > > >
> > > > > > drivers/char/tpm/tpm_crb.c | 69
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 69 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > > > 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > > > u32 cmd_size;
> > > > > > };
> > > > > >
> > > > > > +/**
> > > > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > > > + *
> > > > > > + * @dev: crb device
> > > > > > + * @priv: crb private data
> > > > > > + *
> > > > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > > > + * The device should respond within TIMEOUT_C by clearing the
> bit.
> > > > > > + * Anyhow, we do not wait here as a consequent CMD_READY
> > > > > > +request
> > > > > > + * will be handled correctly even if idle was not completed.
> > > > > > + *
> > > > > > + * The function does nothing for devices with ACPI-start method.
> > > > > > + *
> > > > > > + * Return: 0 always
> > > > > > + */
> > > > > > +static int __maybe_unused crb_go_idle(struct device *dev,
> > > > > > +struct crb_priv *priv) {
> > > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > > + return 0;
> > > > > > +
> > > > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > > > + /* we don't really care when this settles */
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * crb_cmd_ready - request tpm crb device to enter ready
> > > > > > +state
> > > > > > + *
> > > > > > + * @dev: crb device
> > > > > > + * @priv: crb private data
> > > > > > + *
> > > > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > > > + * The device should respond within TIMEOUT_C.
> > > > > > + *
> > > > > > + * The function does nothing for devices with ACPI-start
> > > > > > +method
> > > > > > + *
> > > > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > > > + struct crb_priv *priv)
> > > > > > +{
> > > > > > + ktime_t stop, start;
> > > > > > +
> > > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > > + return 0;
> > > > > > +
> > > > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > > > +
> > > > > > + start = ktime_get();
> > > > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > > > + do {
> > > > > > + if (!(ioread32(&priv->cca->req) &
> > > > > CRB_CTRL_REQ_CMD_READY)) {
> > > > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > > > + ktime_to_us(ktime_sub(ktime_get(),
> start)));
> > > > > > + return 0;
> > > > > > + }
> > > > > > + usleep_range(50, 100);
> > > > > > + } while (ktime_before(ktime_get(), stop));
> > > > >
> > > > > Since this is HW specific this is right thing to do and not
> > > > > abuse wait_for_tpm_stat. However, this should be documented to
> > > > > the commit message.
> > > > I will respin just this patch and not the whole series, as the fix
> > > > is only in the
> > > commit message.
> > > > Tomas
> > >
> > > Works for me. I can update pm_runtime_sync(). Then I'm ready to
> > > apply these.
> >
> > What do you mean by pm_runtime_sync()?
>
> Typo. I already commente v2 of the series that pm_runtime_put should be
> used instead of pm_runtime_put_sync.
This is not necessary, it's okay to suspend in asynchronous mode.
Tomas
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 33+ messages in thread* RE: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
@ 2016-09-15 13:09 ` Winkler, Tomas
0 siblings, 0 replies; 33+ messages in thread
From: Winkler, Tomas @ 2016-09-15 13:09 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: tpmdd-devel@lists.sourceforge.net, Jason Gunthorpe,
linux-kernel@vger.kernel.org
> On Thu, Sep 15, 2016 at 08:23:03AM +0000, Winkler, Tomas wrote:
> > > > > Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle
> > > > > state
> > > > >
> > > > > On Mon, Sep 12, 2016 at 04:04:18PM +0300, Tomas Winkler wrote:
> > > > > > The register TPM_CRB_CTRL_REQ_x contains bits goIdle and
> > > > > > cmdReady for SW to indicate that the device can enter or should exit
> the idle state.
> > > > > >
> > > > > > The legacy ACPI-start (SMI + DMA) based devices do not support
> > > > > > these bits and the idle state management is not exposed to the
> > > > > > host
> > > SW.
> > > > > > Thus, this functionality only is enabled only for a CRB start
> > > > > > (MMIO) based devices.
> > > > > >
> > > > > > Based on Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > > > oringal
> > > > > > patch:
> > > > > > 'tpm_crb: implement power tpm crb power management'
> > > > > >
> > > > > >
> > > > > > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > > > > > ---
> > > > > > V2: do not export the functions via tpm ops
> > > > > > V3: fix lower case corruption; adjust function documentation
> > > > > >
> > > > > > drivers/char/tpm/tpm_crb.c | 69
> > > > > > ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 69 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > > > b/drivers/char/tpm/tpm_crb.c index 6e9d1bca712f..b6923a8b3ff7
> > > > > > 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > > @@ -83,6 +83,75 @@ struct crb_priv {
> > > > > > u32 cmd_size;
> > > > > > };
> > > > > >
> > > > > > +/**
> > > > > > + * crb_go_idle - request tpm crb device to go the idle state
> > > > > > + *
> > > > > > + * @dev: crb device
> > > > > > + * @priv: crb private data
> > > > > > + *
> > > > > > + * Write CRB_CTRL_REQ_GO_IDLE to TPM_CRB_CTRL_REQ
> > > > > > + * The device should respond within TIMEOUT_C by clearing the
> bit.
> > > > > > + * Anyhow, we do not wait here as a consequent CMD_READY
> > > > > > +request
> > > > > > + * will be handled correctly even if idle was not completed.
> > > > > > + *
> > > > > > + * The function does nothing for devices with ACPI-start method.
> > > > > > + *
> > > > > > + * Return: 0 always
> > > > > > + */
> > > > > > +static int __maybe_unused crb_go_idle(struct device *dev,
> > > > > > +struct crb_priv *priv) {
> > > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > > + return 0;
> > > > > > +
> > > > > > + iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> > > > > > + /* we don't really care when this settles */
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * crb_cmd_ready - request tpm crb device to enter ready
> > > > > > +state
> > > > > > + *
> > > > > > + * @dev: crb device
> > > > > > + * @priv: crb private data
> > > > > > + *
> > > > > > + * Write CRB_CTRL_REQ_CMD_READY to TPM_CRB_CTRL_REQ
> > > > > > + * and poll till the device acknowledge it by clearing the bit.
> > > > > > + * The device should respond within TIMEOUT_C.
> > > > > > + *
> > > > > > + * The function does nothing for devices with ACPI-start
> > > > > > +method
> > > > > > + *
> > > > > > + * Return: 0 on success -ETIME on timeout; */ static int
> > > > > > +__maybe_unused crb_cmd_ready(struct device *dev,
> > > > > > + struct crb_priv *priv)
> > > > > > +{
> > > > > > + ktime_t stop, start;
> > > > > > +
> > > > > > + if (priv->flags & CRB_FL_ACPI_START)
> > > > > > + return 0;
> > > > > > +
> > > > > > + iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> > > > > > +
> > > > > > + start = ktime_get();
> > > > > > + stop = ktime_add(start, ms_to_ktime(TPM2_TIMEOUT_C));
> > > > > > + do {
> > > > > > + if (!(ioread32(&priv->cca->req) &
> > > > > CRB_CTRL_REQ_CMD_READY)) {
> > > > > > + dev_dbg(dev, "cmdReady in %lld usecs\n",
> > > > > > + ktime_to_us(ktime_sub(ktime_get(),
> start)));
> > > > > > + return 0;
> > > > > > + }
> > > > > > + usleep_range(50, 100);
> > > > > > + } while (ktime_before(ktime_get(), stop));
> > > > >
> > > > > Since this is HW specific this is right thing to do and not
> > > > > abuse wait_for_tpm_stat. However, this should be documented to
> > > > > the commit message.
> > > > I will respin just this patch and not the whole series, as the fix
> > > > is only in the
> > > commit message.
> > > > Tomas
> > >
> > > Works for me. I can update pm_runtime_sync(). Then I'm ready to
> > > apply these.
> >
> > What do you mean by pm_runtime_sync()?
>
> Typo. I already commente v2 of the series that pm_runtime_put should be
> used instead of pm_runtime_put_sync.
This is not necessary, it's okay to suspend in asynchronous mode.
Tomas
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 2/4] tmp/tpm_crb: fix Intel PTT hw bug during idle state
2016-09-12 13:04 ` Tomas Winkler
@ 2016-09-12 13:04 ` Tomas Winkler
-1 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jason Gunthorpe,
Jarkko Sakkinen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
There is a HW bug in Skylake, and Broxton PCH Intel PTT device, where
most of the registers in the control area except START, REQUEST, CANCEL,
and LOC_CTRL lost retention when the device is in the idle state. Hence
we need to bring the device to ready state before accessing the other
registers. The fix brings device to ready state before trying to read
command and response buffer addresses in order to remap the for access.
Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
V2: cmd read need to be called also before crb_init as this will run
self test.
V3: resend.
drivers/char/tpm/tpm_crb.c | 47 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index b6923a8b3ff7..e945177cf2c8 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -318,6 +318,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
struct list_head resources;
struct resource io_res;
struct device *dev = &device->dev;
+ u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
u64 rsp_pa;
@@ -345,12 +346,27 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (IS_ERR(priv->cca))
return PTR_ERR(priv->cca);
- cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
- (u64) ioread32(&priv->cca->cmd_pa_low);
+ /*
+ * PTT HW bug w/a: wake up the device to access
+ * possibly not retained registers.
+ */
+ ret = crb_cmd_ready(dev, priv);
+ if (ret)
+ return ret;
+
+ pa_high = ioread32(&priv->cca->cmd_pa_high);
+ pa_low = ioread32(&priv->cca->cmd_pa_low);
+ cmd_pa = ((u64)pa_high << 32) | pa_low;
cmd_size = ioread32(&priv->cca->cmd_size);
+
+ dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
+ pa_high, pa_low, cmd_size);
+
priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
- if (IS_ERR(priv->cmd))
- return PTR_ERR(priv->cmd);
+ if (IS_ERR(priv->cmd)) {
+ ret = PTR_ERR(priv->cmd);
+ goto out;
+ }
memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
rsp_pa = le64_to_cpu(rsp_pa);
@@ -358,7 +374,8 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (cmd_pa != rsp_pa) {
priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
- return PTR_ERR_OR_ZERO(priv->rsp);
+ ret = PTR_ERR_OR_ZERO(priv->rsp);
+ goto out;
}
/* According to the PTP specification, overlapping command and response
@@ -366,12 +383,18 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
*/
if (cmd_size != rsp_size) {
dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
+
priv->cmd_size = cmd_size;
priv->rsp = priv->cmd;
- return 0;
+
+out:
+ crb_go_idle(dev, priv);
+
+ return ret;
}
static int crb_acpi_add(struct acpi_device *device)
@@ -415,7 +438,15 @@ static int crb_acpi_add(struct acpi_device *device)
if (rc)
return rc;
- return crb_init(device, priv);
+ rc = crb_cmd_ready(dev, priv);
+ if (rc)
+ return rc;
+
+ rc = crb_init(device, priv);
+ if (rc)
+ crb_go_idle(dev, priv);
+
+ return rc;
}
static int crb_acpi_remove(struct acpi_device *device)
--
2.7.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 2/4] tmp/tpm_crb: fix Intel PTT hw bug during idle state
@ 2016-09-12 13:04 ` Tomas Winkler
0 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel, Jason Gunthorpe, Jarkko Sakkinen; +Cc: linux-kernel, Tomas Winkler
There is a HW bug in Skylake, and Broxton PCH Intel PTT device, where
most of the registers in the control area except START, REQUEST, CANCEL,
and LOC_CTRL lost retention when the device is in the idle state. Hence
we need to bring the device to ready state before accessing the other
registers. The fix brings device to ready state before trying to read
command and response buffer addresses in order to remap the for access.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: cmd read need to be called also before crb_init as this will run
self test.
V3: resend.
drivers/char/tpm/tpm_crb.c | 47 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index b6923a8b3ff7..e945177cf2c8 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -318,6 +318,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
struct list_head resources;
struct resource io_res;
struct device *dev = &device->dev;
+ u32 pa_high, pa_low;
u64 cmd_pa;
u32 cmd_size;
u64 rsp_pa;
@@ -345,12 +346,27 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (IS_ERR(priv->cca))
return PTR_ERR(priv->cca);
- cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
- (u64) ioread32(&priv->cca->cmd_pa_low);
+ /*
+ * PTT HW bug w/a: wake up the device to access
+ * possibly not retained registers.
+ */
+ ret = crb_cmd_ready(dev, priv);
+ if (ret)
+ return ret;
+
+ pa_high = ioread32(&priv->cca->cmd_pa_high);
+ pa_low = ioread32(&priv->cca->cmd_pa_low);
+ cmd_pa = ((u64)pa_high << 32) | pa_low;
cmd_size = ioread32(&priv->cca->cmd_size);
+
+ dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
+ pa_high, pa_low, cmd_size);
+
priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
- if (IS_ERR(priv->cmd))
- return PTR_ERR(priv->cmd);
+ if (IS_ERR(priv->cmd)) {
+ ret = PTR_ERR(priv->cmd);
+ goto out;
+ }
memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
rsp_pa = le64_to_cpu(rsp_pa);
@@ -358,7 +374,8 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
if (cmd_pa != rsp_pa) {
priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
- return PTR_ERR_OR_ZERO(priv->rsp);
+ ret = PTR_ERR_OR_ZERO(priv->rsp);
+ goto out;
}
/* According to the PTP specification, overlapping command and response
@@ -366,12 +383,18 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
*/
if (cmd_size != rsp_size) {
dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out;
}
+
priv->cmd_size = cmd_size;
priv->rsp = priv->cmd;
- return 0;
+
+out:
+ crb_go_idle(dev, priv);
+
+ return ret;
}
static int crb_acpi_add(struct acpi_device *device)
@@ -415,7 +438,15 @@ static int crb_acpi_add(struct acpi_device *device)
if (rc)
return rc;
- return crb_init(device, priv);
+ rc = crb_cmd_ready(dev, priv);
+ if (rc)
+ return rc;
+
+ rc = crb_init(device, priv);
+ if (rc)
+ crb_go_idle(dev, priv);
+
+ return rc;
}
static int crb_acpi_remove(struct acpi_device *device)
--
2.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 2/4] tmp/tpm_crb: fix Intel PTT hw bug during idle state
2016-09-12 13:04 ` Tomas Winkler
(?)
@ 2016-09-15 6:23 ` Jarkko Sakkinen
[not found] ` <20160915062329.GB12289-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
-1 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 6:23 UTC (permalink / raw)
To: Tomas Winkler; +Cc: tpmdd-devel, Jason Gunthorpe, linux-kernel
On Mon, Sep 12, 2016 at 04:04:19PM +0300, Tomas Winkler wrote:
> There is a HW bug in Skylake, and Broxton PCH Intel PTT device, where
> most of the registers in the control area except START, REQUEST, CANCEL,
> and LOC_CTRL lost retention when the device is in the idle state. Hence
> we need to bring the device to ready state before accessing the other
> registers. The fix brings device to ready state before trying to read
> command and response buffer addresses in order to remap the for access.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinn@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinn@linux.intel.com>
/Jarkko
> ---
> V2: cmd read need to be called also before crb_init as this will run
> self test.
> V3: resend.
>
> drivers/char/tpm/tpm_crb.c | 47 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index b6923a8b3ff7..e945177cf2c8 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -318,6 +318,7 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
> struct list_head resources;
> struct resource io_res;
> struct device *dev = &device->dev;
> + u32 pa_high, pa_low;
> u64 cmd_pa;
> u32 cmd_size;
> u64 rsp_pa;
> @@ -345,12 +346,27 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
> if (IS_ERR(priv->cca))
> return PTR_ERR(priv->cca);
>
> - cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) |
> - (u64) ioread32(&priv->cca->cmd_pa_low);
> + /*
> + * PTT HW bug w/a: wake up the device to access
> + * possibly not retained registers.
> + */
> + ret = crb_cmd_ready(dev, priv);
> + if (ret)
> + return ret;
> +
> + pa_high = ioread32(&priv->cca->cmd_pa_high);
> + pa_low = ioread32(&priv->cca->cmd_pa_low);
> + cmd_pa = ((u64)pa_high << 32) | pa_low;
> cmd_size = ioread32(&priv->cca->cmd_size);
> +
> + dev_dbg(dev, "cmd_hi = %X cmd_low = %X cmd_size %X\n",
> + pa_high, pa_low, cmd_size);
> +
> priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size);
> - if (IS_ERR(priv->cmd))
> - return PTR_ERR(priv->cmd);
> + if (IS_ERR(priv->cmd)) {
> + ret = PTR_ERR(priv->cmd);
> + goto out;
> + }
>
> memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8);
> rsp_pa = le64_to_cpu(rsp_pa);
> @@ -358,7 +374,8 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
>
> if (cmd_pa != rsp_pa) {
> priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size);
> - return PTR_ERR_OR_ZERO(priv->rsp);
> + ret = PTR_ERR_OR_ZERO(priv->rsp);
> + goto out;
> }
>
> /* According to the PTP specification, overlapping command and response
> @@ -366,12 +383,18 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv,
> */
> if (cmd_size != rsp_size) {
> dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
> +
> priv->cmd_size = cmd_size;
>
> priv->rsp = priv->cmd;
> - return 0;
> +
> +out:
> + crb_go_idle(dev, priv);
> +
> + return ret;
> }
>
> static int crb_acpi_add(struct acpi_device *device)
> @@ -415,7 +438,15 @@ static int crb_acpi_add(struct acpi_device *device)
> if (rc)
> return rc;
>
> - return crb_init(device, priv);
> + rc = crb_cmd_ready(dev, priv);
> + if (rc)
> + return rc;
> +
> + rc = crb_init(device, priv);
> + if (rc)
> + crb_go_idle(dev, priv);
> +
> + return rc;
> }
>
> static int crb_acpi_remove(struct acpi_device *device)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add
2016-09-12 13:04 ` Tomas Winkler
@ 2016-09-12 13:04 ` Tomas Winkler
-1 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jason Gunthorpe,
Jarkko Sakkinen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
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
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add
@ 2016-09-12 13:04 ` Tomas Winkler
0 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel, Jason Gunthorpe, Jarkko Sakkinen; +Cc: linux-kernel, Tomas Winkler
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add
2016-09-12 13:04 ` Tomas Winkler
(?)
@ 2016-09-15 6:24 ` Jarkko Sakkinen
-1 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2016-09-15 6:24 UTC (permalink / raw)
To: Tomas Winkler; +Cc: tpmdd-devel, Jason Gunthorpe, linux-kernel
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4 4/4] tmp/tpm_crb: implement runtime pm for tpm_crb
2016-09-12 13:04 ` Tomas Winkler
@ 2016-09-12 13:04 ` Tomas Winkler
-1 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jason Gunthorpe,
Jarkko Sakkinen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Utilize runtime_pm for driving tpm crb idle states.
The framework calls cmd_ready from the pm_runtime_resume handler
and go idle from the pm_runtime_suspend handler.
The TPM framework should wake the device before transmit and receive.
In case the runtime_pm framework is not enabled, the device will be in
ready state.
Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
V2: new in the series
V3: resend
drivers/char/tpm/tpm-interface.c | 5 +++++
drivers/char/tpm/tpm_crb.c | 37 ++++++++++++++++++++++++++++++++++---
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fd863ff30f79..9ccad3a9875f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -29,6 +29,7 @@
#include <linux/mutex.h>
#include <linux/spinlock.h>
#include <linux/freezer.h>
+#include <linux/pm_runtime.h>
#include "tpm.h"
#include "tpm_eventlog.h"
@@ -353,6 +354,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
if (!(flags & TPM_TRANSMIT_UNLOCKED))
mutex_lock(&chip->tpm_mutex);
+ pm_runtime_get_sync(chip->dev.parent);
+
rc = chip->ops->send(chip, (u8 *) buf, count);
if (rc < 0) {
dev_err(&chip->dev,
@@ -394,6 +397,8 @@ out_recv:
dev_err(&chip->dev,
"tpm_transmit: tpm_recv: error %zd\n", rc);
out:
+ pm_runtime_put_sync(chip->dev.parent);
+
if (!(flags & TPM_TRANSMIT_UNLOCKED))
mutex_unlock(&chip->tpm_mutex);
return rc;
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index b0c0e2c9022b..94700f71d65e 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -19,6 +19,7 @@
#include <linux/highmem.h>
#include <linux/rculist.h>
#include <linux/module.h>
+#include <linux/pm_runtime.h>
#include "tpm.h"
#define ACPI_SIG_TPM2 "TPM2"
@@ -152,8 +153,6 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
return 0;
}
-static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
-
static u8 crb_status(struct tpm_chip *chip)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
@@ -436,9 +435,16 @@ static int crb_acpi_add(struct acpi_device *device)
if (rc)
return rc;
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
rc = tpm_chip_register(chip);
- if (rc)
+ if (rc) {
crb_go_idle(dev, priv);
+ pm_runtime_disable(dev);
+ }
+
+ pm_runtime_put(dev);
return rc;
}
@@ -450,9 +456,34 @@ static int crb_acpi_remove(struct acpi_device *device)
tpm_chip_unregister(chip);
+ pm_runtime_disable(dev);
+
return 0;
}
+#ifdef CONFIG_PM
+static int crb_pm_runtime_suspend(struct device *dev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+
+ return crb_go_idle(dev, priv);
+}
+
+static int crb_pm_runtime_resume(struct device *dev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+
+ return crb_cmd_ready(dev, priv);
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops crb_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
+ SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
+};
+
static struct acpi_device_id crb_device_ids[] = {
{"MSFT0101", 0},
{"", 0},
--
2.7.4
------------------------------------------------------------------------------
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v4 4/4] tmp/tpm_crb: implement runtime pm for tpm_crb
@ 2016-09-12 13:04 ` Tomas Winkler
0 siblings, 0 replies; 33+ messages in thread
From: Tomas Winkler @ 2016-09-12 13:04 UTC (permalink / raw)
To: tpmdd-devel, Jason Gunthorpe, Jarkko Sakkinen; +Cc: linux-kernel, Tomas Winkler
Utilize runtime_pm for driving tpm crb idle states.
The framework calls cmd_ready from the pm_runtime_resume handler
and go idle from the pm_runtime_suspend handler.
The TPM framework should wake the device before transmit and receive.
In case the runtime_pm framework is not enabled, the device will be in
ready state.
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: new in the series
V3: resend
drivers/char/tpm/tpm-interface.c | 5 +++++
drivers/char/tpm/tpm_crb.c | 37 ++++++++++++++++++++++++++++++++++---
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fd863ff30f79..9ccad3a9875f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -29,6 +29,7 @@
#include <linux/mutex.h>
#include <linux/spinlock.h>
#include <linux/freezer.h>
+#include <linux/pm_runtime.h>
#include "tpm.h"
#include "tpm_eventlog.h"
@@ -353,6 +354,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
if (!(flags & TPM_TRANSMIT_UNLOCKED))
mutex_lock(&chip->tpm_mutex);
+ pm_runtime_get_sync(chip->dev.parent);
+
rc = chip->ops->send(chip, (u8 *) buf, count);
if (rc < 0) {
dev_err(&chip->dev,
@@ -394,6 +397,8 @@ out_recv:
dev_err(&chip->dev,
"tpm_transmit: tpm_recv: error %zd\n", rc);
out:
+ pm_runtime_put_sync(chip->dev.parent);
+
if (!(flags & TPM_TRANSMIT_UNLOCKED))
mutex_unlock(&chip->tpm_mutex);
return rc;
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index b0c0e2c9022b..94700f71d65e 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -19,6 +19,7 @@
#include <linux/highmem.h>
#include <linux/rculist.h>
#include <linux/module.h>
+#include <linux/pm_runtime.h>
#include "tpm.h"
#define ACPI_SIG_TPM2 "TPM2"
@@ -152,8 +153,6 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
return 0;
}
-static SIMPLE_DEV_PM_OPS(crb_pm, tpm_pm_suspend, tpm_pm_resume);
-
static u8 crb_status(struct tpm_chip *chip)
{
struct crb_priv *priv = dev_get_drvdata(&chip->dev);
@@ -436,9 +435,16 @@ static int crb_acpi_add(struct acpi_device *device)
if (rc)
return rc;
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
rc = tpm_chip_register(chip);
- if (rc)
+ if (rc) {
crb_go_idle(dev, priv);
+ pm_runtime_disable(dev);
+ }
+
+ pm_runtime_put(dev);
return rc;
}
@@ -450,9 +456,34 @@ static int crb_acpi_remove(struct acpi_device *device)
tpm_chip_unregister(chip);
+ pm_runtime_disable(dev);
+
return 0;
}
+#ifdef CONFIG_PM
+static int crb_pm_runtime_suspend(struct device *dev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+
+ return crb_go_idle(dev, priv);
+}
+
+static int crb_pm_runtime_resume(struct device *dev)
+{
+ struct tpm_chip *chip = dev_get_drvdata(dev);
+ struct crb_priv *priv = dev_get_drvdata(&chip->dev);
+
+ return crb_cmd_ready(dev, priv);
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops crb_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(tpm_pm_suspend, tpm_pm_resume)
+ SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
+};
+
static struct acpi_device_id crb_device_ids[] = {
{"MSFT0101", 0},
{"", 0},
--
2.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread