Messages in this thread | | | From | Alexey Dobriyan <> | Subject | Re: [PATCH] tpm: Support for new chip type | Date | Fri, 8 Jul 2005 11:16:31 +0400 |
| |
On Friday 08 July 2005 02:42, Marcel Selhorst wrote: > after some corrections here is a newer patch supporting the Infineon Trusted > Platform Module SLD 9630 (TPM 1.1b), which is embedded on Intel-mainboards or > in HP/Fujitsu-Siemens/Toshiba-Notebooks.
> --- linux-2.6.12.2/drivers/char/tpm/tpm_infineon.c > +++ linux/drivers/char/tpm/tpm_infineon.c
> +static int empty_fifo(struct tpm_chip *chip, int clear_wrfifo) > +{ > + int status; > + int check = 0; > + int i; > + int j = 0; > + > + if (clear_wrfifo) { > + for (i = 0; i < 4096; i++) { > + status = inb(chip->vendor->base + WRFIFO); > + if (status == 0xff) { > + if (check == 5) > + break; > + else > + check++; > + } > + } > + } > + /* Note: The values which are currently in the FIFO of the TPM > + are thrown away since there is no usage for them. Usually, > + this has nothing to say, since the TPM will give its answer immediately > + or will be aborted anyway, so the data here is usually garbage and useless. > + We have to clean this, because the next communication with the TPM would > + be rubbish, if there is still some old data in the Read FIFO. > + */
Could you reformat it a little to fit in 80 columns?
> + i = 0;
i is used only in clear_wrfifo loop. Or sacrifice j instead.
> + do { > + status = inb(chip->vendor->base + RDFIFO); > + status = inb(chip->vendor->base + STAT); > + j++; > + if (j == TPM_MAX_TRIES) > + return -EIO; > + } while ((status & (1 << STAT_RDA)) != 0); > + return 0; > +}
> +static int wait(struct tpm_chip *chip, int wait_for_bit) > +{
> + if (i == TPM_MAX_TRIES) { /* when timeout occurs, print information and return -EIO */
We see dev_err() calls, we see -EIO. So only "timeout occurs" should be left.
> + dev_err(&chip->pci_dev->dev, "Timeout in wait("); > + if (wait_for_bit == STAT_XFE) > + dev_err(&chip->pci_dev->dev, "STAT_XFE)\n"); > + if (wait_for_bit == STAT_RDA) > + dev_err(&chip->pci_dev->dev, "STAT_RDA)\n"); > + return -EIO; > + }
> +/* Note: WTX means Waiting-Time-Extension. Whenever the TPM > +needs more calculation time, it sends a WTX-package, which has to > +be acknowledged or aborted. This usually occurs if you are hammering > +the TPM with key creation. Set the maximum number of WTX-packages in > +the definitions above, if the number is reached, the waiting-time will be denied > +and the TPM command has to be resend. > +*/
Be consistent with the rest of the driver:
/* This style is OK. */
> +static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count) > +{
> + recv_begin:
Labels are placed at column zero usually.
> + dev_dbg(&chip->pci_dev->dev, "Receiving header: "); > + > + for (i = 0; i < 4; i++) { > + ret = wait(chip, STAT_RDA); > + if (ret) > + return -EIO; > + > + buf[i] = inb(chip->vendor->base + RDFIFO); > + dev_dbg(&chip->pci_dev->dev, "%02x ", buf[i]); > + } > + dev_dbg(&chip->pci_dev->dev, "\n");
One of dev_dbg jobs is to print KERN_DEBUG, but only at the very beginning of the line.
> + dev_dbg(&chip->pci_dev->dev, "Receiving data: "); > + > + for (i = 0; i < size; i++) { > + wait(chip, STAT_RDA); > + buf[i] = inb(chip->vendor->base + RDFIFO); > + dev_dbg(&chip->pci_dev->dev, "%02x ", buf[i]); > + } > + dev_dbg(&chip->pci_dev->dev, "\n");
dev_dbg() in the middle.
> + "-> Negative acknowledgement - retransmit commando!\n");
command?
> +static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count) > +{
> + count_high = 0; > + count_low = 0;
What's the point?
> + > + count_high = (count & 0xffff0000); > + count_low = (count & 0x0000ffff);
> + dev_dbg(&chip->pci_dev->dev, > + "Sending data %02x %02x %02x %02x %02x %02x ", TPM_VL_VER, > + TPM_VL_CHANNEL_TPM, count_4, count_3, count_2, count_1); > + > + wait_and_send(chip, TPM_VL_VER); > + wait_and_send(chip, TPM_VL_CHANNEL_TPM); > + wait_and_send(chip, count_4); > + wait_and_send(chip, count_3); > + wait_and_send(chip, count_2); > + wait_and_send(chip, count_1); > + > + for (i = 0; i < count; i++) { > + wait_and_send(chip, buf[i]); > + dev_dbg(&chip->pci_dev->dev, "%02x ", buf[i]); > + }
dev_dbg in the middle.
> +static int __init tpm_inf_probe(struct pci_dev *pci_dev, > + const struct pci_device_id *pci_id) > +{
> + dev_dbg(&pci_dev->dev, > + "Device activate register status : %x \n", status);
%x\n - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |