| Message ID | 20191110162137.230913-1-amirmizi6@gmail.com |
|---|---|
| Headers | show |
| Series | add tpm i2c ptp driver | expand |
On Sun Nov 10 19, amirmizi6@gmail.com wrote: >From: Amir Mizinski <amirmizi6@gmail.com> > >using this function while read/write data resulted in aborted operation. >after investigating according to TCG TPM Profile (PTP) Specifications, >i found cancel should happen only if TPM_STS.commandReady bit is lit >and couldn't find a case when the current condition is valid. >also only cmdReady bit need to be compared instead of the full lower status register byte. > >Signed-off-by: Amir Mizinski <amirmizi6@gmail.com> >--- > drivers/char/tpm/tpm_tis_core.c | 12 +----------- > 1 file changed, 1 insertion(+), 11 deletions(-) > >diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c >index ce7f8a1..9016f06 100644 >--- a/drivers/char/tpm/tpm_tis_core.c >+++ b/drivers/char/tpm/tpm_tis_core.c >@@ -627,17 +627,7 @@ static int probe_itpm(struct tpm_chip *chip) > > static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status) > { >- struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); >- >- switch (priv->manufacturer_id) { >- case TPM_VID_WINBOND: >- return ((status == TPM_STS_VALID) || >- (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY))); >- case TPM_VID_STM: >- return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)); Stefan were these cases you found that were deviating from the spec? Wondering if dropping these will cause issues for these devices. >- default: >- return (status == TPM_STS_COMMAND_READY); >- } >+ return ((status & TPM_STS_COMMAND_READY) == TPM_STS_COMMAND_READY); > } > > static irqreturn_t tis_int_handler(int dummy, void *dev_id) >-- >2.7.4 >
On 11/10/19 1:00 PM, Jerry Snitselaar wrote: > On Sun Nov 10 19, amirmizi6@gmail.com wrote: >> From: Amir Mizinski <amirmizi6@gmail.com> >> >> using this function while read/write data resulted in aborted operation. >> after investigating according to TCG TPM Profile (PTP) Specifications, >> i found cancel should happen only if TPM_STS.commandReady bit is lit >> and couldn't find a case when the current condition is valid. >> also only cmdReady bit need to be compared instead of the full lower >> status register byte. >> >> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com> >> --- >> drivers/char/tpm/tpm_tis_core.c | 12 +----------- >> 1 file changed, 1 insertion(+), 11 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm_tis_core.c >> b/drivers/char/tpm/tpm_tis_core.c >> index ce7f8a1..9016f06 100644 >> --- a/drivers/char/tpm/tpm_tis_core.c >> +++ b/drivers/char/tpm/tpm_tis_core.c >> @@ -627,17 +627,7 @@ static int probe_itpm(struct tpm_chip *chip) >> >> static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status) >> { >> - struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); >> - >> - switch (priv->manufacturer_id) { >> - case TPM_VID_WINBOND: >> - return ((status == TPM_STS_VALID) || >> - (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY))); >> - case TPM_VID_STM: >> - return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)); > > Stefan were these cases you found that were deviating from the spec? > Wondering > if dropping these will cause issues for these devices. I believe these devices needed special handling of the status register as they didn't behave as the 'other' devices, so I would expect issues. Stefan
On Sun, Nov 10, 2019 at 06:21:34PM +0200, amirmizi6@gmail.com wrote: > From: Amir Mizinski <amirmizi6@gmail.com> > > The current principles: > - When sending command: > 1. Host writes TPM_STS.commandReady > 2. Host writes command > 3. Host checks TPM received data correctly > 4. if not go to step 1 You are probably talking about steps, right? Please check the grammar and punctation e.g. "The current steps are roughly done when sending a command". > - When receiving data: > 1. Host check TPM_STS.dataAvail is set > 2. Host get data > 3. Host check received data are correct. > 4. if not Host write TPM_STS.responseRetry and go to step 1. > > this commit is based on previous work by Christophe Richard Sentences in English start with a capital letter and end with a full stop. This is completely lacking the description what the commit does. /Jarkko
From: Amir Mizinski <amirmizi6@gmail.com> *This patch set adds support for TPM devices that implement the I2C interface defined by TCG PTP specification: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_2.0_r1.03_v22.pdf The driver was tested on Raspberry-Pie 3, using Nuvoton NPCT75X TPM. interupts are not implemented yet, preparing it for the next patch. this patch is based on initial work by oshri Alkoby, Alexander Steffen and Christophe Ricard Amir Mizinski (5): char: tpm: Make implementation of read16 read32 write32 optional char: tpm: Add check_data handle to tpm_tis_phy_ops in order to check data integrity char: tpm: rewrite "tpm_tis_req_canceled()" dt-bindings: tpm: Add the TPM TIS I2C device tree binding documentaion char: tpm: add tpm_tis_i2c driver .../bindings/security/tpm/tpm_tis_i2c.txt | 24 ++ drivers/char/tpm/Kconfig | 12 + drivers/char/tpm/Makefile | 1 + drivers/char/tpm/tpm_tis_core.c | 109 +++++---- drivers/char/tpm/tpm_tis_core.h | 41 +++- drivers/char/tpm/tpm_tis_i2c.c | 272 +++++++++++++++++++++ drivers/char/tpm/tpm_tis_spi.c | 41 ---- 7 files changed, 407 insertions(+), 93 deletions(-) create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_tis_i2c.txt create mode 100644 drivers/char/tpm/tpm_tis_i2c.c