| Message ID | 20220405173741.4023216-1-jbk@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm: cr50: Add new device/vendor ID 0x504a6666 | expand |
On Tue, Apr 05, 2022 at 10:37:41AM -0700, Jes B. Klinke wrote: > Accept one additional numerical value of DID:VID for next generation > Google TPM, to be used in future Chromebooks. > > This patch touches more lines than may seem necessary, as a result of > the need to move the error case to sit after the two recognized cases. > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > --- > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > index f6c0affbb4567..bf54ebd6724b0 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; > } > > /** > -- > 2.35.1.1094.g7c7d902a7c-goog > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
Dear Jes, Thank you for your patch. Am 05.04.22 um 19:37 schrieb Jes B. Klinke: > Accept one additional numerical value of DID:VID for next generation > Google TPM, to be used in future Chromebooks. Maybe extend: … Google TPM with new firmware … The TPM with the new firmware has the code name TI50, and going to use the same interfaces. > This patch touches more lines than may seem necessary, as a result of > the need to move the error case to sit after the two recognized cases. > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > --- > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > index f6c0affbb4567..bf54ebd6724b0 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); > } Both branches are quite similar. Can a ternary operator be used? dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", (vendor == TPM_CR50_I2C_DID_VID) ? "cr50" : "ti50", client->addr, client->irq, vendor >> 16); return tpm_chip_register(chip); and the original flow be left? (A separate variable can also be added.) > > - 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; > } > > /** Kind regards, Paul
On Tue, Apr 05, 2022 at 10:37:41AM -0700, Jes B. Klinke wrote: > Accept one additional numerical value of DID:VID for next generation > Google TPM, to be used in future Chromebooks. > > This patch touches more lines than may seem necessary, as a result of > the need to move the error case to sit after the two recognized cases. > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > --- > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > index f6c0affbb4567..bf54ebd6724b0 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; > } > > /** > -- > 2.35.1.1094.g7c7d902a7c-goog > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On Thu, Apr 07, 2022 at 12:21:44PM +0200, Paul Menzel wrote: > Dear Jes, > > > Thank you for your patch. > > Am 05.04.22 um 19:37 schrieb Jes B. Klinke: > > Accept one additional numerical value of DID:VID for next generation > > Google TPM, to be used in future Chromebooks. > > Maybe extend: > > … Google TPM with new firmware … > > The TPM with the new firmware has the code name TI50, and going to use the > same interfaces. > > > This patch touches more lines than may seem necessary, as a result of > > the need to move the error case to sit after the two recognized cases. > > > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > > --- > > > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > > index f6c0affbb4567..bf54ebd6724b0 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); > > } > > Both branches are quite similar. Can a ternary operator be used? > > dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", > (vendor == TPM_CR50_I2C_DID_VID) ? "cr50" : "ti50", client->addr, > client->irq, vendor >> 16); > return tpm_chip_register(chip); > > and the original flow be left? (A separate variable can also be added.) > > > - 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; > > } > > /** > > > Kind regards, > > Paul OK, these are legit suggestions. Paul, can you do these changes and add my reviewed-by for the +1 version? BR, Jarkko
Dear Jarkko, Am 14.04.22 um 14:08 schrieb Jarkko Sakkinen: > On Thu, Apr 07, 2022 at 12:21:44PM +0200, Paul Menzel wrote: >> Thank you for your patch. >> >> Am 05.04.22 um 19:37 schrieb Jes B. Klinke: >>> Accept one additional numerical value of DID:VID for next generation >>> Google TPM, to be used in future Chromebooks. >> >> Maybe extend: >> >> … Google TPM with new firmware … >> >> The TPM with the new firmware has the code name TI50, and going to use the >> same interfaces. >> >>> This patch touches more lines than may seem necessary, as a result of >>> the need to move the error case to sit after the two recognized cases. >>> >>> Signed-off-by: Jes B. Klinke <jbk@chromium.org> >>> --- >>> >>> drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- >>> 1 file changed, 13 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c >>> index f6c0affbb4567..bf54ebd6724b0 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); >>> } >> >> Both branches are quite similar. Can a ternary operator be used? >> >> dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", >> (vendor == TPM_CR50_I2C_DID_VID) ? "cr50" : "ti50", client->addr, >> client->irq, vendor >> 16); >> return tpm_chip_register(chip); >> >> and the original flow be left? (A separate variable can also be added.) >> >>> - 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; >>> } >>> /** > OK, these are legit suggestions. Paul, can you do these changes and add my > reviewed-by for the +1 version? I guess you mean Jes? Kind regards, Paul
On Thu, 2022-04-14 at 17:06 +0200, Paul Menzel wrote: > Dear Jarkko, > > > Am 14.04.22 um 14:08 schrieb Jarkko Sakkinen: > > On Thu, Apr 07, 2022 at 12:21:44PM +0200, Paul Menzel wrote: > > > > Thank you for your patch. > > > > > > Am 05.04.22 um 19:37 schrieb Jes B. Klinke: > > > > Accept one additional numerical value of DID:VID for next generation > > > > Google TPM, to be used in future Chromebooks. > > > > > > Maybe extend: > > > > > > … Google TPM with new firmware … > > > > > > The TPM with the new firmware has the code name TI50, and going to use the > > > same interfaces. > > > > > > > This patch touches more lines than may seem necessary, as a result of > > > > the need to move the error case to sit after the two recognized cases. > > > > > > > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > > > > --- > > > > > > > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > > > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > > > > index f6c0affbb4567..bf54ebd6724b0 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); > > > > } > > > > > > Both branches are quite similar. Can a ternary operator be used? > > > > > > dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", > > > (vendor == TPM_CR50_I2C_DID_VID) ? "cr50" : "ti50", client->addr, > > > client->irq, vendor >> 16); > > > return tpm_chip_register(chip); > > > > > > and the original flow be left? (A separate variable can also be added.) > > > > > > > - 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; > > > > } > > > > /** > > > OK, these are legit suggestions. Paul, can you do these changes and add my > > reviewed-by for the +1 version? > > I guess you mean Jes? > > > Kind regards, Yeah, I meant Jes :-) /Jarkko
Thanks for your feedback, I have sent an updated patch to the list, (as far as I could tell, the convention is to prefix with: [PATCH v2]). Regards Jes On Thu, Apr 14, 2022 at 9:53 AM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Thu, 2022-04-14 at 17:06 +0200, Paul Menzel wrote: > > Dear Jarkko, > > > > > > Am 14.04.22 um 14:08 schrieb Jarkko Sakkinen: > > > On Thu, Apr 07, 2022 at 12:21:44PM +0200, Paul Menzel wrote: > > > > > > Thank you for your patch. > > > > > > > > Am 05.04.22 um 19:37 schrieb Jes B. Klinke: > > > > > Accept one additional numerical value of DID:VID for next generation > > > > > Google TPM, to be used in future Chromebooks. > > > > > > > > Maybe extend: > > > > > > > > … Google TPM with new firmware … > > > > > > > > The TPM with the new firmware has the code name TI50, and going to use the > > > > same interfaces. > > > > > > > > > This patch touches more lines than may seem necessary, as a result of > > > > > the need to move the error case to sit after the two recognized cases. > > > > > > > > > > Signed-off-by: Jes B. Klinke <jbk@chromium.org> > > > > > --- > > > > > > > > > > drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- > > > > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c > > > > > index f6c0affbb4567..bf54ebd6724b0 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); > > > > > } > > > > > > > > Both branches are quite similar. Can a ternary operator be used? > > > > > > > > dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n", > > > > (vendor == TPM_CR50_I2C_DID_VID) ? "cr50" : "ti50", client->addr, > > > > client->irq, vendor >> 16); > > > > return tpm_chip_register(chip); > > > > > > > > and the original flow be left? (A separate variable can also be added.) > > > > > > > > > - 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; > > > > > } > > > > > /** > > > > > OK, these are legit suggestions. Paul, can you do these changes and add my > > > reviewed-by for the +1 version? > > > > I guess you mean Jes? > > > > > > Kind regards, > > Yeah, I meant Jes :-) > > /Jarkko
diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c index f6c0affbb4567..bf54ebd6724b0 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; } /**
Accept one additional numerical value of DID:VID for next generation Google TPM, to be used in future Chromebooks. This patch touches more lines than may seem necessary, as a result of the need to move the error case to sit after the two recognized cases. Signed-off-by: Jes B. Klinke <jbk@chromium.org> --- drivers/char/tpm/tpm_tis_i2c_cr50.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)