diff mbox

[tpmdd-devel,v3,12/15] tpm/tpm_i2c_stm_st33: Remove useless i2c read on interrupt registers

Message ID 1413231817-5174-13-git-send-email-christophe-h.ricard@st.com
State Superseded, archived
Headers show

Commit Message

Christophe Ricard Oct. 13, 2014, 8:23 p.m. UTC
Remove useless i2c read on TPM_INT_ENABLE and TPM_INT_STATUS

Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
---
 drivers/char/tpm/tpm_i2c_stm_st33.c | 9 ---------
 1 file changed, 9 deletions(-)

Comments

Marcin Obara Oct. 14, 2014, 8:11 a.m. UTC | #1
Wasn't last read required for flush write?

Remove useless i2c read on TPM_INT_ENABLE and TPM_INT_STATUS

>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>  drivers/char/tpm/tpm_i2c_stm_st33.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c
> b/drivers/char/tpm/tpm_i2c_stm_st33.c
> index 388bb64..ed4176e 100644
> --- a/drivers/char/tpm/tpm_i2c_stm_st33.c
> +++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
> @@ -184,7 +184,6 @@ static void clear_interruption(struct tpm_stm_dev
> *tpm_dev)
>
>         I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>         I2C_WRITE_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
> -       I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>  } /* clear_interruption() */
>
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
Marcin Obara Oct. 14, 2014, 8:17 a.m. UTC | #2
Wasn't last read required for flush write?

2014-10-13 22:23 GMT+02:00 Christophe Ricard <christophe.ricard@gmail.com>:
> Remove useless i2c read on TPM_INT_ENABLE and TPM_INT_STATUS
>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>  drivers/char/tpm/tpm_i2c_stm_st33.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
> index 388bb64..ed4176e 100644
> --- a/drivers/char/tpm/tpm_i2c_stm_st33.c
> +++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
> @@ -184,7 +184,6 @@ static void clear_interruption(struct tpm_stm_dev *tpm_dev)
>
>         I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>         I2C_WRITE_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
> -       I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>  } /* clear_interruption() */

------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
Christophe Henri RICARD Oct. 14, 2014, 8:34 a.m. UTC | #3
Hi Marcin,

In the current clear_interruption function, reading the TPM_INT_STATUS after a write has no effect (aka: flushing the current request).
I am removing it because the interrupt value retrieved after this read is never used.

The only thing I can think of would to verify that the TPM_INT_STATUS got the expected state and return a status code in case of error.
However, I believe in case of such situation, the TPM will generate other error such as transmission error that will be detected on the other layers.

Best Regards
Christophe

-----Original Message-----
From: marcin.obara@gmail.com [mailto:marcin.obara@gmail.com] On Behalf Of Marcin Obara
Sent: mardi 14 octobre 2014 10:18
To: Christophe Ricard
Cc: peterhuewe@gmx.de; ashley@ashleylai.com; tpmdd@selhorst.net; devicetree@vger.kernel.org; Jean-Luc BLANC; tpmdd-devel; Christophe Henri RICARD
Subject: Re: [tpmdd-devel] [PATCH v3 12/15] tpm/tpm_i2c_stm_st33: Remove useless i2c read on interrupt registers

Wasn't last read required for flush write?

2014-10-13 22:23 GMT+02:00 Christophe Ricard <christophe.ricard@gmail.com>:
> Remove useless i2c read on TPM_INT_ENABLE and TPM_INT_STATUS
>
> Signed-off-by: Christophe Ricard <christophe-h.ricard@st.com>
> ---
>  drivers/char/tpm/tpm_i2c_stm_st33.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
> index 388bb64..ed4176e 100644
> --- a/drivers/char/tpm/tpm_i2c_stm_st33.c
> +++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
> @@ -184,7 +184,6 @@ static void clear_interruption(struct tpm_stm_dev *tpm_dev)
>
>         I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>         I2C_WRITE_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
> -       I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
>  } /* clear_interruption() */
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index 388bb64..ed4176e 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -184,7 +184,6 @@  static void clear_interruption(struct tpm_stm_dev *tpm_dev)
 
 	I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
 	I2C_WRITE_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
-	I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &interrupt, 1);
 } /* clear_interruption() */
 
 /*
@@ -725,10 +724,6 @@  tpm_stm_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 			goto _tpm_clean_answer;
 		}
 
-		r = I2C_READ_DATA(tpm_dev, TPM_INT_ENABLE, &intmask, 1);
-		if (r < 0)
-			goto _tpm_clean_answer;
-
 		intmask |= TPM_INTF_CMD_READY_INT
 			|  TPM_INTF_STS_VALID_INT
 			|  TPM_INTF_DATA_AVAIL_INT;
@@ -742,10 +737,6 @@  tpm_stm_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (r < 0)
 			goto _tpm_clean_answer;
 
-		r = I2C_READ_DATA(tpm_dev, TPM_INT_STATUS, &intmask, 1);
-		if (r < 0)
-			goto _tpm_clean_answer;
-
 		chip->vendor.irq = client->irq;
 
 		disable_irq_nosync(chip->vendor.irq);