All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Tomas Winkler <tomas.winkler@intel.com>
Cc: tpmdd-devel@lists.sourceforge.net,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state
Date: Thu, 15 Sep 2016 09:22:04 +0300	[thread overview]
Message-ID: <20160915062204.GA12289@intel.com> (raw)
In-Reply-To: <1473685461-6540-2-git-send-email-tomas.winkler@intel.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
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
Date: Thu, 15 Sep 2016 09:22:04 +0300	[thread overview]
Message-ID: <20160915062204.GA12289@intel.com> (raw)
In-Reply-To: <1473685461-6540-2-git-send-email-tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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

------------------------------------------------------------------------------

  reply	other threads:[~2016-09-15  6:22 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-12 13:04 [PATCH v3 0/4] tpm/tpm_crb: implement power management Tomas Winkler
2016-09-12 13:04 ` Tomas Winkler
2016-09-12 13:04 ` [PATCH v3 1/4] tpm/tpm_crb: implement tpm crb idle state Tomas Winkler
2016-09-12 13:04   ` Tomas Winkler
2016-09-15  6:22   ` Jarkko Sakkinen [this message]
2016-09-15  6:22     ` Jarkko Sakkinen
2016-09-15  6:28     ` Winkler, Tomas
2016-09-15  6:28       ` Winkler, Tomas
2016-09-15  8:20       ` Jarkko Sakkinen
2016-09-15  8:20         ` Jarkko Sakkinen
2016-09-15  8:23         ` Winkler, Tomas
2016-09-15  8:23           ` Winkler, Tomas
2016-09-15 10:53           ` Jarkko Sakkinen
2016-09-15 10:53             ` Jarkko Sakkinen
2016-09-15 13:09             ` Winkler, Tomas
2016-09-15 13:09               ` Winkler, Tomas
2016-09-12 13:04 ` [PATCH v3 2/4] tmp/tpm_crb: fix Intel PTT hw bug during " Tomas Winkler
2016-09-12 13:04   ` Tomas Winkler
2016-09-15  6:23   ` Jarkko Sakkinen
2016-09-27  9:31     ` Jarkko Sakkinen
2016-09-27  9:31       ` Jarkko Sakkinen
2016-09-27  9:54       ` Winkler, Tomas
2016-09-27  9:54         ` Winkler, Tomas
2016-09-12 13:04 ` [PATCH v3 3/4] tpm/tpm_crb: open code the crb_init into acpi_add Tomas Winkler
2016-09-12 13:04   ` Tomas Winkler
2016-09-15  6:24   ` Jarkko Sakkinen
2016-09-12 13:04 ` [PATCH v4 4/4] tmp/tpm_crb: implement runtime pm for tpm_crb Tomas Winkler
2016-09-12 13:04   ` Tomas Winkler
2016-09-14  6:28 ` [PATCH v3 0/4] tpm/tpm_crb: implement power management Winkler, Tomas
2016-09-14  6:28   ` Winkler, Tomas
2016-09-14 16:06   ` Jarkko Sakkinen
2016-09-14 16:06     ` Jarkko Sakkinen
2016-09-14 16:06     ` Jarkko Sakkinen
2016-09-14 16:06       ` Jarkko Sakkinen
2016-09-14 18:25       ` Jarkko Sakkinen
2016-09-14 18:25         ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160915062204.GA12289@intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tomas.winkler@intel.com \
    --cc=tpmdd-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.