Re: [PATCH 2/2] drivers/char/tpm/tpm_stm_st19_i2c.c: Add missing break and neatening
From: Rajiv Andrade
Date: Wed Sep 29 2010 - 13:38:10 EST
Hi James,
I'll send you the complete patchset against your -next tree today.
Thanks,
Rajiv
On 29/09/2010, at 14:08, Christophe Henri RICARD wrote:
> Hi James, Rajiv,
>
> The last one includes all the patches and some comments cleaning.
> Let me know if you need some information's or have some improvements I have to do.
>
> Thanks
> Christophe
>
> -----Original Message-----
> From: James Morris [mailto:jmorris@xxxxxxxxx]
> Sent: Thursday, September 23, 2010 6:39 PM
> To: Christophe Henri RICARD
> Cc: Rajiv Andrade; Joe Perches; Marcel Selhorst; Debora Velarde; tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx; LKML
> Subject: RE: [PATCH 2/2] drivers/char/tpm/tpm_stm_st19_i2c.c: Add missing break and neatening
>
> Which patches need to be applied to the tree?
>
> Rajiv: can you please send them to me as plain text inline, against my
> -next tree, with your signoffs?
>
>
> On Fri, 17 Sep 2010, Christophe Henri RICARD wrote:
>
>> Rajiv,
>> Please find in attached file a full patch (driver + Joe Perches observations/patches + comments update).
>> This patch has been tested.
>>
>> Thanks
>> Christophe
>>
>> -----Original Message-----
>> From: Rajiv Andrade [mailto:srajiv@xxxxxxxxxxxxxxxxxx]
>> Sent: Tuesday, September 14, 2010 12:11 PM
>> To: Joe Perches
>> Cc: Christophe Henri RICARD; Marcel Selhorst; Debora Velarde; James Morris; tpmdd-devel@xxxxxxxxxxxxxxxxxxxxx; LKML
>> Subject: Re: [PATCH 2/2] drivers/char/tpm/tpm_stm_st19_i2c.c: Add missing break and neatening
>>
>> 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@xxxxxxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>>> ---
>>> 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);
>>
>>
>>
>
> --
> James Morris
> <jmorris@xxxxxxxxx>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/