Re: [PATCH 7/7] tpm: Driver for next generation TPM chips

[Nishanth Aravamudan <nacc@us.ibm.com>]

On 10.04.2006 [09:37:28 -0500], Kylene Jo Hall wrote:
> This patch contains the driver for the next generation of TPM chips
> version 1.2 including support for interrupts.  The Trusted Computing
> Group has written the TPM Interface Specification (TIS) which defines a
> common interface for all manufacturer's 1.2 TPM's thus the name
> tpm_tis.
> 
> This updated version of the patch uses the new sysfs files that came
> about from the comments and changes in patch 6/7.  It replaces the 7/7
> patch from the original set.

<snip>

> +static int request_locality(struct tpm_chip *chip, int l)
> +{
> +	unsigned long stop;
> +
> +	if (check_locality(chip, l) >= 0)
> +		return l;
> +
> +	iowrite8(TPM_ACCESS_REQUEST_USE,
> +		 chip->vendor.iobase + TPM_ACCESS(l));
> +
> +	if (chip->vendor.irq) {
> +		interruptible_sleep_on_timeout(&chip->vendor.int_queue,
> +					       HZ *
> +					       chip->vendor.timeout_a /
> +					       1000);
> +		if (check_locality(chip, l) >= 0)
> +			return l;
> +
> +	} else {
> +		/* wait for burstcount */
> +		stop = jiffies + (HZ * chip->vendor.timeout_a / 1000);
> +		do {
> +			if (check_locality(chip, l) >= 0)
> +				return l;
> +			msleep(TPM_TIMEOUT);
> +		}
> +		while (time_before(jiffies, stop));
> +	}

This looks like it could take the msecs_to_jiffies() conversion as well.
Might as well cache it before the if/else, as both clauses use it?
Really, it is just wait_event*() without the wait-queue. Well, this is
at least one more consumer potentially of the poll_event*() API I had
written a while back, I'll dust it off again if I have the time.

<snip>

> +static int get_burstcount(struct tpm_chip *chip)
> +{
> +	unsigned long stop;
> +	int burstcnt;
> +
> +	/* wait for burstcount */
> +	/* which timeout value, spec has 2 answers (c & d) */
> +	stop = jiffies + (HZ * chip->vendor.timeout_d / 1000);

msecs_to_jiffies().

<snip>

With the changes you've already made, that should clean up the sleeping
code a bit, at least.

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center