RE: [PATCH 2/2] drivers/char/tpm/tpm_stm_st19_i2c.c: Add missingbreak and neatening
From: James Morris
Date: Thu Sep 23 2010 - 19:40:42 EST
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/