2017-03-28 15:30:15

by Enric Balletbo i Serra

[permalink] [raw]
Subject: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

From: Bryan Freed <[email protected]>

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.

Signed-off-by: Bryan Freed <[email protected]>
[rework the patch to adapt to the feedback received]
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---
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 v2:
- Do not remove faster transfers when chip is SLB9645 (Peter Huewe)
- Remember the adapterlimit length once it fails to not generate extra
i2c core messages (suggested by Andrew Lunn)
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 | 76 +++++++++++++++++++++++++++----------
1 file changed, 56 insertions(+), 20 deletions(-)

diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 62ee44e..fdefcdb 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -70,6 +70,7 @@ struct tpm_inf_dev {
u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
struct tpm_chip *chip;
enum i2c_chip_type chip_type;
+ unsigned int adapterlimit;
};

static struct tpm_inf_dev tpm_dev;
@@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)

int rc = 0;
int count;
+ unsigned int msglen = len;

/* Lock the adapter for the duration of the whole sequence. */
if (!tpm_dev.client->adapter->algo->master_xfer)
@@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
}
} else {
- /* 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 */
-
- usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
- }
-
- if (rc <= 0)
- goto out;
-
- /* After the TPM has successfully received the register address
- * it needs some time, thus we're sleeping here again, before
- * retrieving the data
+ /* Expect to send one command message and one data message, but
+ * support looping over each or both if necessary.
*/
- for (count = 0; count < MAX_COUNT; count++) {
- usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
- rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
- if (rc > 0)
- break;
+ 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 */
+
+ usleep_range(SLEEP_DURATION_LOW,
+ SLEEP_DURATION_HI);
+ }
+
+ if (rc <= 0)
+ goto out;
+
+ /* After the TPM has successfully received the register
+ * address it needs some time, thus we're sleeping here
+ * again, before retrieving the data
+ */
+ for (count = 0; count < MAX_COUNT; count++) {
+ if (tpm_dev.adapterlimit) {
+ msglen = min_t(unsigned int,
+ tpm_dev.adapterlimit,
+ len);
+ msg2.len = msglen;
+ }
+ usleep_range(SLEEP_DURATION_LOW,
+ SLEEP_DURATION_HI);
+ rc = __i2c_transfer(tpm_dev.client->adapter,
+ &msg2, 1);
+ 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)
+ tpm_dev.adapterlimit =
+ I2C_SMBUS_BLOCK_MAX;
+ }
+
+ if (rc <= 0)
+ goto out;
}
}

--
2.9.3


2017-03-28 16:20:51

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote:
> From: Bryan Freed <[email protected]>
>
> 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.
>
> Signed-off-by: Bryan Freed <[email protected]>
> [rework the patch to adapt to the feedback received]
> Signed-off-by: Enric Balletbo i Serra <[email protected]>

This looks O.K. now. Thanks for remembering the adaptor limit.

Acked-by: Andrew Lunn <[email protected]>

Andrew

2017-03-31 08:13:30

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote:
> From: Bryan Freed <[email protected]>
>
> 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.
>
> Signed-off-by: Bryan Freed <[email protected]>
> [rework the patch to adapt to the feedback received]
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
> 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 v2:
> - Do not remove faster transfers when chip is SLB9645 (Peter Huewe)
> - Remember the adapterlimit length once it fails to not generate extra
> i2c core messages (suggested by Andrew Lunn)
> 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 | 76 +++++++++++++++++++++++++++----------
> 1 file changed, 56 insertions(+), 20 deletions(-)

Looking into this after taking of the request for 4.12.

/Jarkko

>
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 62ee44e..fdefcdb 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
> u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
> struct tpm_chip *chip;
> enum i2c_chip_type chip_type;
> + unsigned int adapterlimit;
> };
>
> static struct tpm_inf_dev tpm_dev;
> @@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
>
> int rc = 0;
> int count;
> + unsigned int msglen = len;
>
> /* Lock the adapter for the duration of the whole sequence. */
> if (!tpm_dev.client->adapter->algo->master_xfer)
> @@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> }
> } else {
> - /* 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 */
> -
> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> - }
> -
> - if (rc <= 0)
> - goto out;
> -
> - /* After the TPM has successfully received the register address
> - * it needs some time, thus we're sleeping here again, before
> - * retrieving the data
> + /* Expect to send one command message and one data message, but
> + * support looping over each or both if necessary.
> */
> - for (count = 0; count < MAX_COUNT; count++) {
> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
> - if (rc > 0)
> - break;
> + 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 */
> +
> + usleep_range(SLEEP_DURATION_LOW,
> + SLEEP_DURATION_HI);
> + }
> +
> + if (rc <= 0)
> + goto out;
> +
> + /* After the TPM has successfully received the register
> + * address it needs some time, thus we're sleeping here
> + * again, before retrieving the data
> + */
> + for (count = 0; count < MAX_COUNT; count++) {
> + if (tpm_dev.adapterlimit) {
> + msglen = min_t(unsigned int,
> + tpm_dev.adapterlimit,
> + len);
> + msg2.len = msglen;
> + }
> + usleep_range(SLEEP_DURATION_LOW,
> + SLEEP_DURATION_HI);
> + rc = __i2c_transfer(tpm_dev.client->adapter,
> + &msg2, 1);
> + 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)
> + tpm_dev.adapterlimit =
> + I2C_SMBUS_BLOCK_MAX;
> + }
> +
> + if (rc <= 0)
> + goto out;
> }
> }
>
> --
> 2.9.3
>

2017-04-05 09:03:33

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote:
> From: Bryan Freed <[email protected]>
>
> 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.
>
> Signed-off-by: Bryan Freed <[email protected]>
> [rework the patch to adapt to the feedback received]
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
> 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 v2:
> - Do not remove faster transfers when chip is SLB9645 (Peter Huewe)
> - Remember the adapterlimit length once it fails to not generate extra
> i2c core messages (suggested by Andrew Lunn)
> 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 | 76 +++++++++++++++++++++++++++----------
> 1 file changed, 56 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 62ee44e..fdefcdb 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
> u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
> struct tpm_chip *chip;
> enum i2c_chip_type chip_type;
> + unsigned int adapterlimit;
> };
>
> static struct tpm_inf_dev tpm_dev;
> @@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
>
> int rc = 0;
> int count;
> + unsigned int msglen = len;
>
> /* Lock the adapter for the duration of the whole sequence. */
> if (!tpm_dev.client->adapter->algo->master_xfer)
> @@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> }
> } else {
> - /* 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 */
> -
> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> - }
> -
> - if (rc <= 0)
> - goto out;
> -
> - /* After the TPM has successfully received the register address
> - * it needs some time, thus we're sleeping here again, before
> - * retrieving the data
> + /* Expect to send one command message and one data message, but
> + * support looping over each or both if necessary.
> */
> - for (count = 0; count < MAX_COUNT; count++) {
> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
> - if (rc > 0)
> - break;
> + 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 */
> +
> + usleep_range(SLEEP_DURATION_LOW,
> + SLEEP_DURATION_HI);
> + }
> +
> + if (rc <= 0)
> + goto out;
> +
> + /* After the TPM has successfully received the register
> + * address it needs some time, thus we're sleeping here
> + * again, before retrieving the data
> + */
> + for (count = 0; count < MAX_COUNT; count++) {
> + if (tpm_dev.adapterlimit) {
> + msglen = min_t(unsigned int,
> + tpm_dev.adapterlimit,
> + len);
> + msg2.len = msglen;
> + }
> + usleep_range(SLEEP_DURATION_LOW,
> + SLEEP_DURATION_HI);
> + rc = __i2c_transfer(tpm_dev.client->adapter,
> + &msg2, 1);
> + 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)
> + tpm_dev.adapterlimit =
> + I2C_SMBUS_BLOCK_MAX;
> + }
> +
> + if (rc <= 0)
> + goto out;
> }
> }
>
> --
> 2.9.3
>

Reviewed-by: Jarkko Sakkinen <[email protected]>

Peter, Andrew, anyone: Tested-by?

/Jarkko

2017-04-05 10:06:36

by Peter Huewe

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.



Am 5. April 2017 11:03:27 MESZ schrieb Jarkko Sakkinen <[email protected]>:
>On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote:
>> From: Bryan Freed <[email protected]>
>>
>> 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.
>>
>> Signed-off-by: Bryan Freed <[email protected]>
>> [rework the patch to adapt to the feedback received]
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>> ---
>> 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 v2:
>> - Do not remove faster transfers when chip is SLB9645 (Peter Huewe)
>> - Remember the adapterlimit length once it fails to not generate
>extra
>> i2c core messages (suggested by Andrew Lunn)
>> 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 | 76
>+++++++++++++++++++++++++++----------
>> 1 file changed, 56 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
>b/drivers/char/tpm/tpm_i2c_infineon.c
>> index 62ee44e..fdefcdb 100644
>> --- a/drivers/char/tpm/tpm_i2c_infineon.c
>> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
>> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
>> u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
>> struct tpm_chip *chip;
>> enum i2c_chip_type chip_type;
>> + unsigned int adapterlimit;
>> };
>>
>> static struct tpm_inf_dev tpm_dev;
>> @@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer,
>size_t len)
>>
>> int rc = 0;
>> int count;
>> + unsigned int msglen = len;
>>
>> /* Lock the adapter for the duration of the whole sequence. */
>> if (!tpm_dev.client->adapter->algo->master_xfer)
>> @@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer,
>size_t len)
>> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
>> }
>> } else {
>> - /* 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 */
>> -
>> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
>> - }
>> -
>> - if (rc <= 0)
>> - goto out;
>> -
>> - /* After the TPM has successfully received the register address
>> - * it needs some time, thus we're sleeping here again, before
>> - * retrieving the data
>> + /* Expect to send one command message and one data message, but
>> + * support looping over each or both if necessary.
>> */
>> - for (count = 0; count < MAX_COUNT; count++) {
>> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
>> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
>> - if (rc > 0)
>> - break;
>> + 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 */
>> +
>> + usleep_range(SLEEP_DURATION_LOW,
>> + SLEEP_DURATION_HI);
>> + }
>> +
>> + if (rc <= 0)
>> + goto out;
>> +
>> + /* After the TPM has successfully received the register
>> + * address it needs some time, thus we're sleeping here
>> + * again, before retrieving the data
>> + */
>> + for (count = 0; count < MAX_COUNT; count++) {
>> + if (tpm_dev.adapterlimit) {
>> + msglen = min_t(unsigned int,
>> + tpm_dev.adapterlimit,
>> + len);
>> + msg2.len = msglen;
>> + }
>> + usleep_range(SLEEP_DURATION_LOW,
>> + SLEEP_DURATION_HI);
>> + rc = __i2c_transfer(tpm_dev.client->adapter,
>> + &msg2, 1);
>> + 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)
>> + tpm_dev.adapterlimit =
>> + I2C_SMBUS_BLOCK_MAX;
>> + }
>> +
>> + if (rc <= 0)
>> + goto out;
>> }
>> }
>>
>> --
>> 2.9.3
>>
>
>Reviewed-by: Jarkko Sakkinen <[email protected]>
>
>Peter, Andrew, anyone: Tested-by?
>

Not yet, I'll put it on my list to test.
Hopefully by next tuesday.
Peter

>/Jarkko

--
Sent from my mobile

2017-04-05 13:24:43

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> Peter, Andrew, anyone: Tested-by?

Sorry, i don't have the hardware. All i can give you is:

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2017-04-05 13:37:03

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

On Wed, Apr 05, 2017 at 03:24:31PM +0200, Andrew Lunn wrote:
> > Reviewed-by: Jarkko Sakkinen <[email protected]>
> >
> > Peter, Andrew, anyone: Tested-by?
>
> Sorry, i don't have the hardware. All i can give you is:
>
> Reviewed-by: Andrew Lunn <[email protected]>
>
> Andrew

Thank you.

/Jarkko

2017-04-05 13:41:09

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission.

On Wed, Apr 05, 2017 at 12:05:32PM +0200, Peter Huewe wrote:
>
>
> Am 5. April 2017 11:03:27 MESZ schrieb Jarkko Sakkinen <[email protected]>:
> >On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote:
> >> From: Bryan Freed <[email protected]>
> >>
> >> 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.
> >>
> >> Signed-off-by: Bryan Freed <[email protected]>
> >> [rework the patch to adapt to the feedback received]
> >> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> >> ---
> >> 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 v2:
> >> - Do not remove faster transfers when chip is SLB9645 (Peter Huewe)
> >> - Remember the adapterlimit length once it fails to not generate
> >extra
> >> i2c core messages (suggested by Andrew Lunn)
> >> 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 | 76
> >+++++++++++++++++++++++++++----------
> >> 1 file changed, 56 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c
> >b/drivers/char/tpm/tpm_i2c_infineon.c
> >> index 62ee44e..fdefcdb 100644
> >> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> >> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> >> @@ -70,6 +70,7 @@ struct tpm_inf_dev {
> >> u8 buf[TPM_BUFSIZE + sizeof(u8)]; /* max. buffer size + addr */
> >> struct tpm_chip *chip;
> >> enum i2c_chip_type chip_type;
> >> + unsigned int adapterlimit;
> >> };
> >>
> >> static struct tpm_inf_dev tpm_dev;
> >> @@ -111,6 +112,7 @@ static int iic_tpm_read(u8 addr, u8 *buffer,
> >size_t len)
> >>
> >> int rc = 0;
> >> int count;
> >> + unsigned int msglen = len;
> >>
> >> /* Lock the adapter for the duration of the whole sequence. */
> >> if (!tpm_dev.client->adapter->algo->master_xfer)
> >> @@ -131,27 +133,61 @@ static int iic_tpm_read(u8 addr, u8 *buffer,
> >size_t len)
> >> usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> >> }
> >> } else {
> >> - /* 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 */
> >> -
> >> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> >> - }
> >> -
> >> - if (rc <= 0)
> >> - goto out;
> >> -
> >> - /* After the TPM has successfully received the register address
> >> - * it needs some time, thus we're sleeping here again, before
> >> - * retrieving the data
> >> + /* Expect to send one command message and one data message, but
> >> + * support looping over each or both if necessary.
> >> */
> >> - for (count = 0; count < MAX_COUNT; count++) {
> >> - usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> >> - rc = __i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
> >> - if (rc > 0)
> >> - break;
> >> + 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 */
> >> +
> >> + usleep_range(SLEEP_DURATION_LOW,
> >> + SLEEP_DURATION_HI);
> >> + }
> >> +
> >> + if (rc <= 0)
> >> + goto out;
> >> +
> >> + /* After the TPM has successfully received the register
> >> + * address it needs some time, thus we're sleeping here
> >> + * again, before retrieving the data
> >> + */
> >> + for (count = 0; count < MAX_COUNT; count++) {
> >> + if (tpm_dev.adapterlimit) {
> >> + msglen = min_t(unsigned int,
> >> + tpm_dev.adapterlimit,
> >> + len);
> >> + msg2.len = msglen;
> >> + }
> >> + usleep_range(SLEEP_DURATION_LOW,
> >> + SLEEP_DURATION_HI);
> >> + rc = __i2c_transfer(tpm_dev.client->adapter,
> >> + &msg2, 1);
> >> + 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)
> >> + tpm_dev.adapterlimit =
> >> + I2C_SMBUS_BLOCK_MAX;
> >> + }
> >> +
> >> + if (rc <= 0)
> >> + goto out;
> >> }
> >> }
> >>
> >> --
> >> 2.9.3
> >>
> >
> >Reviewed-by: Jarkko Sakkinen <[email protected]>
> >
> >Peter, Andrew, anyone: Tested-by?
> >
>
> Not yet, I'll put it on my list to test.
> Hopefully by next tuesday.
> Peter

Ok, thanks. I can push this to my master branch if that would help
to ease the testig?

/Jarkko