PATCH: Google security chip, additional DID:VID

Commit Message

Jes Klinke April 4, 2022, 11 p.m. UTC

Hello Peter,

I am a firmware engineer working on the TPM chip in Chromebooks (known
as cr50).  As we plan to deploy a new codebase on our TPM chips, we
will have them present a new DID:VID value, but otherwise follow the
same protocol as the existing implementations.

The below patch has the effect of accepting one additional numerical
value, and logging one of two messages, depending on the value.
Unfortunately, I had to reverse the indentation, so the diff appears
to touch more lines than it needed to do.

I am unsure which procedure to follow, as this is my first kernel
patch.  Let me know if posting the patch inline like this is not the
way to go.

Regards
Jes Bodi Klinke

Comments

Eric Biggers April 4, 2022, 11:21 p.m. UTC | #1

On Mon, Apr 04, 2022 at 04:00:31PM -0700, Jes Klinke wrote:
> Hello Peter,
> 
> I am a firmware engineer working on the TPM chip in Chromebooks (known
> as cr50).  As we plan to deploy a new codebase on our TPM chips, we
> will have them present a new DID:VID value, but otherwise follow the
> same protocol as the existing implementations.
> 
> The below patch has the effect of accepting one additional numerical
> value, and logging one of two messages, depending on the value.
> Unfortunately, I had to reverse the indentation, so the diff appears
> to touch more lines than it needed to do.
> 
> I am unsure which procedure to follow, as this is my first kernel
> patch.  Let me know if posting the patch inline like this is not the
> way to go.

Can you take a look at Documentation/process/submitting-patches.rst?

- Eric

Jes Klinke April 4, 2022, 11:44 p.m. UTC | #2

I have sent a separate email to the list with the patch, in which I
have cut out the long explanation, added Signed-off-by, and added the
delimiter for the diff.  Let me know if there are anything else I have
overlooked.

Regards
Jes

On Mon, Apr 4, 2022 at 4:21 PM Eric Biggers <[e-mail]> wrote:
>
> On Mon, Apr 04, 2022 at 04:00:31PM -0700, Jes Klinke wrote:
> > Hello Peter,
> >
> > I am a firmware engineer working on the TPM chip in Chromebooks (known
> > as cr50).  As we plan to deploy a new codebase on our TPM chips, we
> > will have them present a new DID:VID value, but otherwise follow the
> > same protocol as the existing implementations.
> >
> > The below patch has the effect of accepting one additional numerical
> > value, and logging one of two messages, depending on the value.
> > Unfortunately, I had to reverse the indentation, so the diff appears
> > to touch more lines than it needed to do.
> >
> > I am unsure which procedure to follow, as this is my first kernel
> > patch.  Let me know if posting the patch inline like this is not the
> > way to go.
>
> Can you take a look at Documentation/process/submitting-patches.rst?
>
> - Eric

Eric Biggers April 4, 2022, 11:58 p.m. UTC | #3

On Mon, Apr 04, 2022 at 04:44:57PM -0700, Jes Klinke wrote:
> I have sent a separate email to the list with the patch, in which I
> have cut out the long explanation, added Signed-off-by, and added the
> delimiter for the diff.  Let me know if there are anything else I have
> overlooked.
> 
> Regards
> Jes

Please use 'git format-patch', as submitting-patches.rst recommends.  Also run
scripts/checkpatch.pl on your patch.  That will ensure that your patch is in the
correct format.  As-is, your patch doesn't apply.

- Eric

Patch

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c
b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index f6c0affbb4567..e5fb1ecc8fa2e 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -31,6 +31,7 @@ 
 #define TPM_CR50_TIMEOUT_SHORT_MS 2 /* Short timeout during transactions */
 #define TPM_CR50_TIMEOUT_NOIRQ_MS 20 /* Timeout for TPM ready without IRQ */
 #define TPM_CR50_I2C_DID_VID 0x00281ae0L /* Device and vendor ID reg value */
+#define TPM_TI50_I2C_DID_VID 0x504a6666L /* Device and vendor ID reg value */
 #define TPM_CR50_I2C_MAX_RETRIES 3 /* Max retries due to I2C errors */
 #define TPM_CR50_I2C_RETRY_DELAY_LO 55 /* Min usecs between retries on I2C */
 #define TPM_CR50_I2C_RETRY_DELAY_HI 65 /* Max usecs between retries on I2C */
@@ -742,16 +743,20 @@  static int tpm_cr50_i2c_probe(struct i2c_client *client)
  }

  vendor = le32_to_cpup((__le32 *)buf);
- if (vendor != TPM_CR50_I2C_DID_VID) {
- dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
- tpm_cr50_release_locality(chip, true);
- return -ENODEV;
+ if (vendor == TPM_CR50_I2C_DID_VID) {
+ dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
+ client->addr, client->irq, vendor >> 16);
+ return tpm_chip_register(chip);
+ }
+ if (vendor == TPM_TI50_I2C_DID_VID) {
+ dev_info(dev, "ti50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
+ client->addr, client->irq, vendor >> 16);
+ return tpm_chip_register(chip);
  }

- dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
- client->addr, client->irq, vendor >> 16);
-
- return tpm_chip_register(chip);
+ dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
+ tpm_cr50_release_locality(chip, true);
+ return -ENODEV;
 }

 /**