Re: [PATCH 2/2] drivers/char/tpm/tpm_stm_st19_i2c.c: Add missing break and neatening

[Rajiv Andrade <srajiv@linux.vnet.ibm.com>]

Chris, did you have the chance to test it against this specific TPM?
Despite this, the patch looks pretty good, thanks Joe.

Acked-by: Rajiv Andrade <srajiv@linux.vnet.ibm.com>

On Fri, 2010-09-03 at 16:29 -0700, Joe Perches wrote:
> Add missing break in wait_until_good_shape
> Use init; while (test) loops not for(init;test;)
> Calculate bytes to transfer using if and min_t not multiple ?:
> Miscellaneous function whitespace aliging
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/char/tpm/tpm_stm_st19_i2c.c |  158 +++++++++++++++++++----------------
>  1 files changed, 85 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_stm_st19_i2c.c b/drivers/char/tpm/tpm_stm_st19_i2c.c
> index cdf2eb3..4b1ed05 100644
> --- a/drivers/char/tpm/tpm_stm_st19_i2c.c
> +++ b/drivers/char/tpm/tpm_stm_st19_i2c.c
> @@ -158,8 +158,10 @@ static int wait_until_good_shape(void)
>  		else if (!signal_pending(current)) {
>  			ret = schedule_timeout(time);
>  			wait_time += time;
> -		} else
> +		} else {
>  			ret = -ERESTARTSYS;
> +			break;
> +		}
>  	} while (1);
>  	finish_wait(&queue, &__wait);
> 
> @@ -204,10 +206,10 @@ static int wait_event_interruptible_on_gpio(wait_queue_head_t queue,
>  		else if (wait_time >= timeout)
>  			break;
>  		else if (!signal_pending(current)) {
> -			ret =
> -			    schedule_timeout(msecs_to_jiffies
> -					     (TICK_GPIO_SPOOLING));
> -			wait_time += msecs_to_jiffies(TICK_GPIO_SPOOLING);
> +			int time = msecs_to_jiffies(TICK_GPIO_SPOOLING);
> +
> +			ret = schedule_timeout(time);
> +			wait_time += time;
>  		} else {
>  			ret = -ERESTARTSYS;
>  			break;
> @@ -286,9 +288,9 @@ static int tpm_stm_i2c_send(struct tpm_chip *chip, unsigned char *buf,
>  	/* sending data (data_avail_pin hight) */
>  	/* If data are available, we read the data */
>  	init_waitqueue_head(&pin_infos->write_queue);
> -	pin = wait_event_interruptible_on_gpio(pin_infos->write_queue,
> -					       tpm_calc_ordinal_duration
> -					       (chip, ordinal));
> +	pin = wait_event_interruptible_on_gpio(
> +			pin_infos->write_queue,
> +			tpm_calc_ordinal_duration(chip, ordinal));
>  	if (pin < 0) {
>  		ret = pin;
>  		goto end;
> @@ -297,25 +299,30 @@ static int tpm_stm_i2c_send(struct tpm_chip *chip, unsigned char *buf,
>  	client->flags = I2C_M_RD;
> 
>  	size = TPM_HEADER_SIZE;
> -	for (i = 0; pin == DATA_ON && i < size;) {
> -		ret = i2c_master_recv(client,
> -				      pin_infos->tpm_i2c_buffer[1],
> -				      (i == 0) ? TPM_HEADER_SIZE :
> -				      count - i > TPM_I2C_BLOCK_SIZE ?
> -				      TPM_I2C_BLOCK_SIZE : count - i);
> +	i = 0;
> +	while (i < size && pin == DATA_ON) {
> +		int bytes;
> +
> +		if (i == 0)
> +			bytes = TPM_HEADER_SIZE;
> +		else
> +			bytes = min_t(int, count - i, TPM_I2C_BLOCK_SIZE);
> +
> +		ret = i2c_master_recv(client, pin_infos->tpm_i2c_buffer[1],
> +				      bytes);
>  		if (ret < 0)
>  			goto end;
> -		if (i == 0)
> -			size =
> -			    responseSize(pin_infos->tpm_i2c_buffer[1], count);
> -		(i == 0) ? (i += TPM_HEADER_SIZE) : (i += TPM_I2C_BLOCK_SIZE);
> +		if (i == 0) {
> +			size = responseSize(pin_infos->tpm_i2c_buffer[1],
> +					    count);
> +			i += TPM_HEADER_SIZE;
> +		} else
> +			i += TPM_I2C_BLOCK_SIZE;
> 
>  		if (i < size)
> -			pin =
> -			    wait_event_interruptible_on_gpio(pin_infos->
> -							     write_queue,
> -							     msecs_to_jiffies
> -							     (TPM_I2C_SHORT));
> +			pin = wait_event_interruptible_on_gpio(
> +					pin_infos->write_queue,
> +					msecs_to_jiffies(TPM_I2C_SHORT));
>  	}
> 
>  	pin = wait_event_interruptible_on_gpio(pin_infos->write_queue,
> @@ -325,36 +332,41 @@ static int tpm_stm_i2c_send(struct tpm_chip *chip, unsigned char *buf,
>  	client->flags = 0;
> 
>  	size = TPM_HEADER_SIZE;
> -	for (i = 0; i < size && pin == COMMAND_ON;) {
> -		memcpy(pin_infos->tpm_i2c_buffer[0], buf + i,
> -		       (i == 0) ? TPM_HEADER_SIZE : count - i >
> -		       TPM_I2C_BLOCK_SIZE ? TPM_I2C_BLOCK_SIZE : count - i);
> +	i = 0;
> +	while (i < size && pin == COMMAND_ON) {
> +		int bytes;
> +
> +		if (i == 0)
> +			bytes = TPM_HEADER_SIZE;
> +		else
> +			bytes = min_t(int, count - i, TPM_I2C_BLOCK_SIZE);
> +
> +		memcpy(pin_infos->tpm_i2c_buffer[0], buf + i, bytes);
> 
>  		if (i == 0) {
>  			size = responseSize(buf, count);
> -			size = (size < count ? size : count);
> +			size = size < count ? size : count;
>  		}
> -		ret =
> -		    i2c_master_send(client,
> -				    pin_infos->tpm_i2c_buffer[0],
> -				    count >= TPM_HEADER_SIZE ? (i ==
> -								0) ?
> -				    TPM_HEADER_SIZE : count - i >
> -				    TPM_I2C_BLOCK_SIZE ? TPM_I2C_BLOCK_SIZE :
> -				    count - i : count);
> +
> +		if (count < TPM_HEADER_SIZE)
> +			bytes = count;
> +
> +		ret = i2c_master_send(client, pin_infos->tpm_i2c_buffer[0],
> +				      bytes);
>  		if (ret < 0) {
>  			pr_info("Failed to send data\n");
>  			goto end;
>  		}
> 
> -		(i == 0) ? (i += TPM_HEADER_SIZE) : (i += TPM_I2C_BLOCK_SIZE);
> +		if (i == 0)
> +			i += TPM_HEADER_SIZE;
> +		else
> +			i += TPM_I2C_BLOCK_SIZE;
>  		/* Wait for AcceptCmd signal hight */
>  		if (i < size)
> -			pin =
> -			    wait_event_interruptible_on_gpio(pin_infos->
> -							     write_queue,
> -							     msecs_to_jiffies
> -							     (TPM_I2C_SHORT));
> +			pin = wait_event_interruptible_on_gpio(
> +					pin_infos->write_queue,
> +					msecs_to_jiffies(TPM_I2C_SHORT));
> 
>  		if (pin != COMMAND_ON) {
>  			pr_info("Failed to read gpio pin (AcceptCmd)\n");
> @@ -362,6 +374,7 @@ static int tpm_stm_i2c_send(struct tpm_chip *chip, unsigned char *buf,
>  			goto end;
>  		}
>  	}
> +
>  	if (i == 0) {
>  		pr_info("Failed to read gpio pin (AcceptCmd)\n");
>  		ret = -EIO;
> @@ -412,49 +425,48 @@ static int tpm_stm_i2c_recv(struct tpm_chip *chip, unsigned char *buf,
> 
>  	/* Spool on the good gpio as long as pin GPIO 3 not HIGHT */
>  	init_waitqueue_head(&chip->vendor.read_queue);
> -	pin = wait_event_interruptible_on_gpio(chip->vendor.read_queue,
> -					       tpm_calc_ordinal_duration
> -					       (chip, TPM_I2C_ORDINAL_LONG));
> +	pin = wait_event_interruptible_on_gpio(
> +			chip->vendor.read_queue,
> +			tpm_calc_ordinal_duration(chip, TPM_I2C_ORDINAL_LONG));
> 
>  	size = TPM_HEADER_SIZE;
> -	for (i = 0; i < size && pin == DATA_ON;) {
> -		ret =
> -		    i2c_master_recv(client,
> -				    pin_infos->tpm_i2c_buffer[1],
> -				    (count >= TPM_HEADER_SIZE ? i ==
> -				     0 ? TPM_HEADER_SIZE : (size - i) >
> -				     TPM_I2C_BLOCK_SIZE ? TPM_I2C_BLOCK_SIZE :
> -				     size - i : count));
> +	i = 0;
> +	while (i < size && pin == DATA_ON) {
> +		int bytes;
> +
> +		if (count < TPM_HEADER_SIZE)
> +			bytes = count;
> +		else if (i == 0)
> +			bytes = TPM_HEADER_SIZE;
> +		else
> +			bytes = min_t(int, size - i, TPM_I2C_BLOCK_SIZE);
> +
> +		ret = i2c_master_recv(client, pin_infos->tpm_i2c_buffer[1],
> +				      bytes);
>  		if (ret < 0) {
>  			pr_info("Failed to read gpio pin (DataAvailable)\n");
>  			goto end;
>  		}
> 
> -		if (buf != NULL) {
> -			memcpy(buf + i, pin_infos->tpm_i2c_buffer[1],
> -			       (count >= TPM_HEADER_SIZE ? i == 0 ?
> -				TPM_HEADER_SIZE : (size - i) >
> -				TPM_I2C_BLOCK_SIZE ? TPM_I2C_BLOCK_SIZE : size -
> -				i : count));
> -
> -			if (i == 0) {
> -				size = responseSize(buf, size);
> -				if (size > count)
> -					size = count;
> -			}
> -		} else {
> +		if (!buf) {
>  			pr_info("read buffer is NULL\n");
>  			goto end;
>  		}
> 
> -		(i == 0) ? (i += TPM_HEADER_SIZE) : (i += TPM_I2C_BLOCK_SIZE);
> +		memcpy(buf + i, pin_infos->tpm_i2c_buffer[1], size);
> +
> +		if (i == 0) {
> +			size = responseSize(buf, size);
> +			if (size > count)
> +				size = count;
> +			i += TPM_HEADER_SIZE;
> +		} else
> +			i += TPM_I2C_BLOCK_SIZE;
> 
>  		if (i < size)
> -			pin =
> -			    wait_event_interruptible_on_gpio(chip->vendor.
> -							     read_queue,
> -							     msecs_to_jiffies
> -							     (TPM_I2C_SHORT));
> +			pin = wait_event_interruptible_on_gpio(
> +					chip->vendor.read_queue,
> +					msecs_to_jiffies(TPM_I2C_SHORT));
>  	}
> 
>  	if (i == 0) {
> @@ -532,7 +544,7 @@ static ssize_t tpm_st19_i2c_ioctl(struct inode *inode, struct file *file,
>  		return -ENOSYS;
>  	case TPMIOC_TRANSMIT:
>  		if (copy_from_user(pin_infos->tpm_i2c_buffer[0],
> -		    (const char *)arg, TPM_HEADER_SIZE))
> +				   (const char *)arg, TPM_HEADER_SIZE))
>  			return -EFAULT;
>  		in_size = responseSize(pin_infos->tpm_i2c_buffer[0],
>  				      TPM_HEADER_SIZE);