Re: [tpmdd-devel] [PATCH v2] tpm: Apply a sane minimum adapterlimit value for retransmission.

From: Jarkko Sakkinen
Date: Thu Mar 02 2017 - 07:57:15 EST


On Wed, Mar 01, 2017 at 04:36:17PM +0100, Enric Balletbo i Serra wrote:
> From: Bryan Freed <bfreed@xxxxxxxxxxxx>
>
> When the I2C Infineon part is attached to an I2C adapter that imposes
> a size limitation, large requests will fail with -EOPNOTSUPP. Retry
> them with a sane minimum size without re-issuing the 0x05 command
> as this appears to occasionally put the TPM in a bad state.

What is 0x05 command?

/Jarkko

> Signed-off-by: Bryan Freed <bfreed@xxxxxxxxxxxx>
> [rework the patch to adapt to the feedback received]
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
> ---
> This is a reworked version of the original patch based on the
> suggestion made by Wolfram Sang to simply fall back to a sane minimum
> when the maximum fails.
>
> Changes since v1:
> - Check the correct return value (-EOPNOTSUPP instead of -EINVAL)
> - Fall back len to I2C_SMBUS_BLOCK_MAX if fails.
>
> drivers/char/tpm/tpm_i2c_infineon.c | 45 +++++++++++++++++++++----------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 62ee44e..88bf947 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -107,39 +107,27 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> .len = len,
> .buf = buffer
> };
> - struct i2c_msg msgs[] = {msg1, msg2};
>
> int rc = 0;
> int count;
> + unsigned int adapterlimit = len;
>
> /* Lock the adapter for the duration of the whole sequence. */
> if (!tpm_dev.client->adapter->algo->master_xfer)
> return -EOPNOTSUPP;
> i2c_lock_adapter(tpm_dev.client->adapter);
>
> - if (tpm_dev.chip_type == SLB9645) {
> - /* use a combined read for newer chips
> - * unfortunately the smbus functions are not suitable due to
> - * the 32 byte limit of the smbus.
> - * retries should usually not be needed, but are kept just to
> - * be on the safe side.
> - */
> - for (count = 0; count < MAX_COUNT; count++) {
> - rc = __i2c_transfer(tpm_dev.client->adapter, msgs, 2);
> - if (rc > 0)
> - break; /* break here to skip sleep */
> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> - }
> - } else {
> + /* Expect to send one command message and one data message, but
> + * support looping over each or both if necessary.
> + */
> + while (len > 0) {
> /* slb9635 protocol should work in all cases */
> for (count = 0; count < MAX_COUNT; count++) {
> rc = __i2c_transfer(tpm_dev.client->adapter, &msg1, 1);
> if (rc > 0)
> - break; /* break here to skip sleep */
> -
> + break;
> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> }
> -
> if (rc <= 0)
> goto out;
>
> @@ -148,11 +136,30 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> * retrieving the data
> */
> for (count = 0; count < MAX_COUNT; count++) {
> + unsigned int msglen = msg2.len =
> + min_t(unsigned int, adapterlimit, len);
> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
> - if (rc > 0)
> + if (rc > 0) {
> + /* Since len is unsigned, make doubly sure we
> + * do not underflow it.
> + */
> + if (msglen > len)
> + len = 0;
> + else
> + len -= msglen;
> + msg2.buf += msglen;
> break;
> + }
> + /* If the I2C adapter rejected the request (e.g when
> + * the quirk read_max_len < len) fall back to a sane
> + * minimum value and try again.
> + */
> + if (rc == -EOPNOTSUPP)
> + adapterlimit = I2C_SMBUS_BLOCK_MAX;
> }
> + if (rc <= 0)
> + goto out;
> }
>
> out:
> --
> 2.9.3
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel