From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755131AbdDEKGg convert rfc822-to-8bit (ORCPT ); Wed, 5 Apr 2017 06:06:36 -0400 Received: from mout.gmx.net ([212.227.17.21]:58467 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753606AbdDEKGH (ORCPT ); Wed, 5 Apr 2017 06:06:07 -0400 Date: Wed, 05 Apr 2017 12:05:32 +0200 User-Agent: K-9 Mail for Android In-Reply-To: <20170405090327.pssnm3mjcpajpoy6@intel.com> References: <20170328152938.14539-1-enric.balletbo@collabora.com> <20170405090327.pssnm3mjcpajpoy6@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission. To: Jarkko Sakkinen , Enric Balletbo i Serra CC: Andrew Lunn , tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, wsa@the-dreams.de, Bryan Freed From: Peter Huewe Message-ID: <384D02FC-BCCF-45F0-896D-058326EC6783@gmx.de> X-Provags-ID: V03:K0:A0BKprXzplFc2sglhaL/NLFxfkq4wrRxKCxpovN+BtW3QqSTtiw 03e4WtAS/6iHgNXIwNokPr+SjNz8gBCyVeVEY1TC4D0qOCfSvnIZzSStOsLpvYXhHlP1Gs5 iXT6QYXoV5gYW/cjUsDs6ZCDkYHGxXPdI+FABsawXYsiIrBFrVRSf0YPQgiKuk8O2QGTGCE MN0CWuST5lfHilDLfseYg== X-UI-Out-Filterresults: notjunk:1;V01:K0:zZmVwkCg8iU=:E1ZhQ8xz3hehKlM9p+lMlT bxURAhBLpI64EeorfqWpwwdXPSVVLJubBDXnmIjSZsvIrUJSlrvKMulkvjCcMG4RCz6o4xK8x yjFOUzMCPO/sHvsdpM8mReWfR/N2TFjr7dAUl4WfpW/nqLKx3PqFWHH/+8C49unHy5jxjLms+ SB4ge+oJUggK7ThUzMNvQ7GTa8Nxw4HJlOo0Qy/JEAtfTyWtg2g+2QJa6Ok6krb+JEViH03B5 ZTx1thVLg2VGSqQrIs2VuYDQ/lHhrxm2Ou8wORtTB3MFF4Sr6UdjorPj+SZvDXAYnlvYq7kdp UxzEDFthwd1R93/bD7K81hCQOkykhz02bzmBvJmtRlBGceTfQywLAmnFMZJf5jME2folxqx1v 6E/PN4C67QxW+hfCMdu/TddB10dhsz2GgB8dWTDpZLM6uL5fUcikOQa14oFyjIR4iXW35LlFL UOqVPiI0OhE/9QlCp753hswbX8ETh8+5fRoqmokcq8rg/4J7QuJUxuxEiwj8utT2OHnRKgsez 0BVMMn48kNcs1L/uyLk0Cg60o8OUDARTngGKwUreH0NdkxxQlP9cxTkaFuTlSqvWClwgTXD+z tNK2hG1bBacV9TiQimZlkIyPW7ossnE2EMcV17uvwlEZxsZ1F7IHwwOOxiGD2zjjKgsdOVUXw AhlSV/0H3u1xAs3iBAeOkiMhlQhDCE/2MRpylmQzRS07hM//cJQ1udioSLNty/DP6eQduvJLl gn4LWLoja4CFB+cpVhuPH/KCfYoKuSuE4C9IC94vjA47UwFBuShBsBTgIpxbJVls1deacUU1s /fRucJ3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 5. April 2017 11:03:27 MESZ schrieb Jarkko Sakkinen : >On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote: >> From: Bryan Freed >> >> 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 >> [rework the patch to adapt to the feedback received] >> Signed-off-by: Enric Balletbo i Serra >> --- >> 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 > >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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Huewe Subject: Re: [PATCH v3] tpm: Apply a sane minimum adapterlimit value for retransmission. Date: Wed, 05 Apr 2017 12:05:32 +0200 Message-ID: <384D02FC-BCCF-45F0-896D-058326EC6783@gmx.de> References: <20170328152938.14539-1-enric.balletbo@collabora.com> <20170405090327.pssnm3mjcpajpoy6@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170405090327.pssnm3mjcpajpoy6-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Jarkko Sakkinen , Enric Balletbo i Serra Cc: Bryan Freed , tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org List-Id: tpmdd-devel@lists.sourceforge.net Am 5. April 2017 11:03:27 MESZ schrieb Jarkko Sakkinen : >On Tue, Mar 28, 2017 at 05:29:38PM +0200, Enric Balletbo i Serra wrote: >> From: Bryan Freed >> >> 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 >> [rework the patch to adapt to the feedback received] >> Signed-off-by: Enric Balletbo i Serra >> --- >> 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 > >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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot