Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 1/3] tpm: move TPM_POLL_SLEEP from tpm_tis_core.c to tpm.h
To:     Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc:     linux-integrity@vger.kernel.org, zohar@linux.vnet.ibm.com,
        linux-security-module@vger.kernel.org,
        linux-kernel@vger.kernel.org, peterhuewe@gmx.de,
        tpmdd@selhorst.net, jgunthorpe@obsidianresearch.com,
        patrickc@us.ibm.com
References: <20180228191828.20056-1-nayna@linux.vnet.ibm.com>
 <20180301083740.GB29420@linux.intel.com>
From:   Nayna Jain <nayna@linux.vnet.ibm.com>
Date:   Fri, 2 Mar 2018 00:14:57 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <20180301083740.GB29420@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Message-Id: <409652f8-74db-2af5-3faa-137c71a27a59@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 03/01/2018 02:07 PM, Jarkko Sakkinen wrote:
> Hi
>
> On Wed, Feb 28, 2018 at 02:18:26PM -0500, Nayna Jain wrote:
>> This patch moves TPM_POLL_SLEEP from tpm_tis_core.c to tpm.h, renaming
>> it to TPM_TIMEOUT_POLL, to follow the existing enum naming
>> conventions.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> The cover letter is missing. Are this meant to be a patch set or
> individual patches? I'll check these anyway.

These patches continue to improve TPM performance.
The first patch exports and renames the timeout enum for polling, that 
is subsequently used in the second patch.
The third patch is posted as an RFC  for further performance improvement 
by improving granularity.
>
>> ---
>>   drivers/char/tpm/tpm.h          |  3 ++-
>>   drivers/char/tpm/tpm_tis_core.c | 10 ++--------
>>   2 files changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>> index f895fba4e20d..7e797377e1eb 100644
>> --- a/drivers/char/tpm/tpm.h
>> +++ b/drivers/char/tpm/tpm.h
>> @@ -53,7 +53,8 @@ enum tpm_const {
>>   enum tpm_timeout {
>>   	TPM_TIMEOUT = 5,	/* msecs */
>>   	TPM_TIMEOUT_RETRY = 100, /* msecs */
>> -	TPM_TIMEOUT_RANGE_US = 300	/* usecs */
>> +	TPM_TIMEOUT_RANGE_US = 300,	/* usecs */
> What is happening here?
Addition of comma to add next enum value.

Thanks & Regards,
     - Nayna

>
>> +	TPM_TIMEOUT_POLL = 1	/* msecs */
>>   };
>>   
>>   /* TPM addresses */
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 183a5f54d875..dc474e7244a6 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -31,12 +31,6 @@
>>   #include "tpm.h"
>>   #include "tpm_tis_core.h"
>>   
>> -/* This is a polling delay to check for status and burstcount.
>> - * As per ddwg input, expectation is that status check and burstcount
>> - * check should return within few usecs.
>> - */
>> -#define TPM_POLL_SLEEP	1  /* msec */
>> -
>>   static void tpm_tis_clkrun_enable(struct tpm_chip *chip, bool value);
>>   
>>   static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
>> @@ -90,7 +84,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
>>   		}
>>   	} else {
>>   		do {
>> -			tpm_msleep(TPM_POLL_SLEEP);
>> +			tpm_msleep(TPM_TIMEOUT_POLL);
>>   			status = chip->ops->status(chip);
>>   			if ((status & mask) == mask)
>>   				return 0;
>> @@ -232,7 +226,7 @@ static int get_burstcount(struct tpm_chip *chip)
>>   		burstcnt = (value >> 8) & 0xFFFF;
>>   		if (burstcnt)
>>   			return burstcnt;
>> -		tpm_msleep(TPM_POLL_SLEEP);
>> +		tpm_msleep(TPM_TIMEOUT_POLL);
>>   	} while (time_before(jiffies, stop));
>>   	return -EBUSY;
>>   }
>> -- 
>> 2.13.3
>>
> Otherwise, looks fine.
> /Jarkko
>