diff mbox

[v2,1/2] tpm/st33zp24: Add proper wait for ordinal duration in case of irq mode

Message ID 1427047988-18132-2-git-send-email-christophe-h.ricard@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Christophe Ricard March 22, 2015, 6:13 p.m. UTC
In case the driver is configured to use irq, we are not waiting the answer
for a duration period to see the DATA_AVAIL status bit to raise but at
maximum timeout_c. This may result in critical failure as we will
not wait long enough for the command completion.

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---
 drivers/char/tpm/st33zp24/st33zp24.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Jason Gunthorpe March 23, 2015, 7:09 p.m. UTC | #1
On Sun, Mar 22, 2015 at 07:13:07PM +0100, Christophe Ricard wrote:
> In case the driver is configured to use irq, we are not waiting the answer
> for a duration period to see the DATA_AVAIL status bit to raise but at
> maximum timeout_c. This may result in critical failure as we will
> not wait long enough for the command completion.
> 
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
>  drivers/char/tpm/st33zp24/st33zp24.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
> index 03f2543..976ccb5 100644
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -393,7 +393,7 @@ static irqreturn_t tpm_ioserirq_handler(int irq, void *dev_id)
>  static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
>  			 size_t len)
>  {
> -	u32 status, i, size;
> +	u32 status, i, size, ordinal;
>  	int burstcnt = 0;
>  	int ret;
>  	u8 data;
> @@ -456,6 +456,17 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
>  	if (ret < 0)
>  		goto out_err;
>  
> +	if (chip->vendor.irq) {
> +		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +
> +		if (wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
> +				  tpm_calc_ordinal_duration(chip, ordinal),
> +				  &chip->vendor.read_queue, true) < 0) {
> +			ret = -ETIME;

When check_cancel=true wait_for_stat can return -ECANCELED, which is
ignored here.

This should always return rc from wait_for_stat, and I notice other
drives have this same cut and paste error, it would be great if you
could send a patch for those too.

The bigger question is if check_cancel should be false or true, I
notice it is false in tpm_tis in a similar context??

Jason

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
diff mbox

Patch

diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 03f2543..976ccb5 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -393,7 +393,7 @@  static irqreturn_t tpm_ioserirq_handler(int irq, void *dev_id)
 static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
 			 size_t len)
 {
-	u32 status, i, size;
+	u32 status, i, size, ordinal;
 	int burstcnt = 0;
 	int ret;
 	u8 data;
@@ -456,6 +456,17 @@  static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
 	if (ret < 0)
 		goto out_err;
 
+	if (chip->vendor.irq) {
+		ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
+
+		if (wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+				  tpm_calc_ordinal_duration(chip, ordinal),
+				  &chip->vendor.read_queue, true) < 0) {
+			ret = -ETIME;
+			goto out_err;
+		}
+	}
+
 	return len;
 out_err:
 	st33zp24_cancel(chip);