Hi Jason, Hi Kent,
>
>Discussion on this got a bit sidetracked talking about
>suspend/resume.. To be clear, this fixes a real, serious, problem on
>normal embedded cases where the kernel refuses to attach the TPM driver
>at all.
>
>Key, are you happy with this as-is for the next merge window?
>
>This version is rebased and retested to 3.7-rc6. Tested on Atmel
>and Nuvoton LPC TPMs on PPC32.
Sorry I was tied up at work with other stuff, so I couldn't try out your
initial patch.
Thanks for resubmitting!
I just gave the new version a run on my beagleboard with our Infineon SLB9635
TT 1.2 Soft I2C TPM
and it seems to work as expected. (Tested with and without previous startup).
Tested-by: Peter Huewe <peter.hu...@infineon.com>
I just have some minor comments - please see below.
>+++ b/drivers/char/tpm/tpm.c
>+#define TPM_ORD_STARTUP cpu_to_be32(153)
>+#define TPM_ST_CLEAR cpu_to_be16(1)
>+#define TPM_ST_STATE cpu_to_be16(2)
>+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
>+static const struct tpm_input_header tpm_startup_header = {
>+ .tag = TPM_TAG_RQU_COMMAND,
>+ .length = cpu_to_be32(12),
>+ .ordinal = TPM_ORD_STARTUP
>+};
>+
> ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> const char *desc)
> {
Purely cosmetic question, but why did you define this before the tpm_getcap and
not tpm_startup?
All the other definitions are made before they are used - so this should
perhaps better be moved directly before tpm_startup.
(Maybe we should move out these definitions to a common location? Header?)
>@@ -541,11 +560,27 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
> tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
>+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
Please use NULL instead of 0, otherwise sparse complains - so
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
>+ if (rc == TPM_ERR_INVALID_POSTINIT) {
> ...
>+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
Same here
please use NULL instead of 0, otherwise sparse complains - so
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
NULL);
>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
>@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
> __be32 num_bytes;
> }__attribute__((packed));
>
>+struct tpm_startup_in {
>+ __be16 startup_type;
>+} __packed;
All the other user
__attribute__((packed));
Care to change to be consistent?
Apart from these three minor topics also a
Reviewed-by: Peter Huewe <peter.hu...@infineon.com>
Thanks,
Peter
--
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/
