Please see my comments below. Did you get a chance to run the trousers
testsuite?
git://trousers.git.sourceforge.net/gitroot/trousers/testsuite
+static u8 spi_read8_reg(struct tpm_chip *tpm, u8 tpm_register,
+ u8 *tpm_data, u16 tpm_size)
+{
+ ...
+
+ /* header + status byte + size of the data + status byte */
+ value = spi_sync_transfer(dev, &xfer, 1);
+
+ if (tpm_size > 0 && value == 0) {
+ nbr_dummy_bytes = 2;
Why use 2 for the dummy bytes? Some comments here would be
helpful. Should we use a #define for 2?
...
+static unsigned long wait_for_serirq_timeout(struct tpm_chip *chip,
+ bool condition, unsigned long timeout)
+{
+ long status = 0;
+ struct spi_device *client;
+ struct st33zp24_platform_data *pin_infos;
+
+ client = (struct spi_device __force *)chip->vendor.iobase;
+ pin_infos = client->dev.platform_data;
+
+ status = wait_for_completion_interruptible_timeout(
+ &pin_infos->irq_detection, timeout);
+ if (status > 0)
+ enable_irq(gpio_to_irq(pin_infos->io_serirq));
+ gpio_direction_input(pin_infos->io_serirq);
+
+ if (!status)
+ return -EBUSY;
This function returns -EBUSY but the return function type is unsigned.
...
+static int recv_data(struct tpm_chip *chip, u8 *buf, size_t count)
+{
+ u32 size = 0, burstcnt, len;
+ long status = 0;
+
+ while (size < count &&
+ wait_for_stat(chip,
+ TPM_STS_DATA_AVAIL | TPM_STS_VALID,
+ chip->vendor.timeout_c,
+ &chip->vendor.read_queue)
+ == 0) {
+ burstcnt = get_burstcount(chip);
+ len = min_t(int, burstcnt, count - size);
What if get_burstcount returns -EBUSY? Should burstcnt be checked
and handled here in case of error?
+ status = spi_read8_reg(chip, TPM_DATA_FIFO, buf + size,
len);
+ if (status < 0)
+ return status;
+
+
Extra empty line here.
+ size += len;
See above comment. If len is -EBUSY, there might be an issue here.
+ }
+return size;
...
+static int tpm_stm_spi_send(struct tpm_chip *chip, unsigned char *buf,
+ size_t len)
+{
...
+ for (i = 0; i < len - 1;) {
+ burstcnt = get_burstcount(chip);
Again burstcnt needs to be checked to make sure it is > 0.
+ size = min_t(int, len - i - 1, burstcnt);
+ ret = spi_write8_reg(chip, TPM_DATA_FIFO, buf, size);
+ if (ret < 0)
+ goto out_err;
+ i += size;
+ }
...
+ /* write last byte */
+ spi_write8_reg(chip, TPM_DATA_FIFO, buf + len - 1, 1);
Should we check for the return code here? A few lines below it checks
for the return value.
...
+
+
+ /* go and do it */
+ data = TPM_STS_GO;
+ ret = spi_write8_reg(chip, TPM_STS, &data, 1);
Regards,
--Ashley Lai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
