All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666
@ 2022-04-19 23:37 Jes B. Klinke
  2022-04-20  7:17 ` Jarkko Sakkinen
  2022-04-20 10:52 ` Paul Menzel
  0 siblings, 2 replies; 5+ messages in thread
From: Jes B. Klinke @ 2022-04-19 23:37 UTC (permalink / raw)
  To: linux-integrity
  Cc: Paul Menzel, Jes B. Klinke, Jarkko Sakkinen, Jason Gunthorpe,
	Peter Huewe, linux-kernel

Accept one additional numerical value of DID:VID for next generation
Google TPM with new firmware, to be used in future Chromebooks.

The TPM with the new firmware has the code name TI50, and is going to
use the same interfaces.

Signed-off-by: Jes B. Klinke <jbk@chromium.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---

 drivers/char/tpm/tpm_tis_i2c_cr50.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index f6c0affbb4567..4ddb8ff3a8569 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,15 +743,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
 	}
 
 	vendor = le32_to_cpup((__le32 *)buf);
-	if (vendor != TPM_CR50_I2C_DID_VID) {
+	if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) {
 		dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
 		tpm_cr50_release_locality(chip, true);
 		return -ENODEV;
 	}
 
-	dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
+	dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
+		 vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50",
 		 client->addr, client->irq, vendor >> 16);
-
 	return tpm_chip_register(chip);
 }
 
-- 
2.36.0.rc0.470.gd361397f0d-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666
  2022-04-19 23:37 [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666 Jes B. Klinke
@ 2022-04-20  7:17 ` Jarkko Sakkinen
  2022-04-20 10:52 ` Paul Menzel
  1 sibling, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2022-04-20  7:17 UTC (permalink / raw)
  To: Jes B. Klinke, linux-integrity
  Cc: Paul Menzel, Jason Gunthorpe, Peter Huewe, linux-kernel

On Tue, 2022-04-19 at 16:37 -0700, Jes B. Klinke wrote:
> Accept one additional numerical value of DID:VID for next generation
> Google TPM with new firmware, to be used in future Chromebooks.
> 
> The TPM with the new firmware has the code name TI50, and is going to
> use the same interfaces.
> 
> Signed-off-by: Jes B. Klinke <jbk@chromium.org>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> 
>  drivers/char/tpm/tpm_tis_i2c_cr50.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index f6c0affbb4567..4ddb8ff3a8569 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,15 +743,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
>         }
>  
>         vendor = le32_to_cpup((__le32 *)buf);
> -       if (vendor != TPM_CR50_I2C_DID_VID) {
> +       if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) {
>                 dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
>                 tpm_cr50_release_locality(chip, true);
>                 return -ENODEV;
>         }
>  
> -       dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> +       dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> +                vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50",
>                  client->addr, client->irq, vendor >> 16);
> -
>         return tpm_chip_register(chip);
>  }
>  

Thank you, I just picked this.

BR, Jarkko


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666
  2022-04-19 23:37 [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666 Jes B. Klinke
  2022-04-20  7:17 ` Jarkko Sakkinen
@ 2022-04-20 10:52 ` Paul Menzel
  2022-04-21 16:07   ` Jes Klinke
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Menzel @ 2022-04-20 10:52 UTC (permalink / raw)
  To: Jes B. Klinke
  Cc: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Huewe,
	linux-kernel

Dear Jes,


Am 20.04.22 um 01:37 schrieb Jes B. Klinke:
> Accept one additional numerical value of DID:VID for next generation
> Google TPM with new firmware, to be used in future Chromebooks.
> 
> The TPM with the new firmware has the code name TI50, and is going to
> use the same interfaces.
> 
> Signed-off-by: Jes B. Klinke <jbk@chromium.org>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> 
>   drivers/char/tpm/tpm_tis_i2c_cr50.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> index f6c0affbb4567..4ddb8ff3a8569 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,15 +743,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
>   	}
>   
>   	vendor = le32_to_cpup((__le32 *)buf);
> -	if (vendor != TPM_CR50_I2C_DID_VID) {
> +	if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) {
>   		dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
>   		tpm_cr50_release_locality(chip, true);
>   		return -ENODEV;
>   	}
>   
> -	dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> +	dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> +		 vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50",
>   		 client->addr, client->irq, vendor >> 16);
> -

Remove this blank line wasn’t needed, I gues.

>   	return tpm_chip_register(chip);
>   }
>   

Thank for addressing the other comments, and congratulations on getting 
your first commit into the Linux kernel.


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666
  2022-04-20 10:52 ` Paul Menzel
@ 2022-04-21 16:07   ` Jes Klinke
  2022-04-21 16:13     ` Jarkko Sakkinen
  0 siblings, 1 reply; 5+ messages in thread
From: Jes Klinke @ 2022-04-21 16:07 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Jes B. Klinke, linux-integrity, Jarkko Sakkinen, Jason Gunthorpe,
	Peter Huewe, linux-kernel

Dear Paul,

On Wed, Apr 20, 2022 at 3:52 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Jes,
>
>
> Am 20.04.22 um 01:37 schrieb Jes B. Klinke:
> > Accept one additional numerical value of DID:VID for next generation
> > Google TPM with new firmware, to be used in future Chromebooks.
> >
> > The TPM with the new firmware has the code name TI50, and is going to
> > use the same interfaces.
> >
> > Signed-off-by: Jes B. Klinke <jbk@chromium.org>
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> >
> >   drivers/char/tpm/tpm_tis_i2c_cr50.c | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> > index f6c0affbb4567..4ddb8ff3a8569 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,15 +743,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
> >       }
> >
> >       vendor = le32_to_cpup((__le32 *)buf);
> > -     if (vendor != TPM_CR50_I2C_DID_VID) {
> > +     if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) {
> >               dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
> >               tpm_cr50_release_locality(chip, true);
> >               return -ENODEV;
> >       }
> >
> > -     dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> > +     dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> > +              vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50",
> >                client->addr, client->irq, vendor >> 16);
> > -
>
> Remove this blank line wasn’t needed, I gues.

You are right, I should probably have left the blank line untouched.
It does not hurt the readability of the code much to remove it,
though.

> >       return tpm_chip_register(chip);
> >   }
> >
>
> Thank for addressing the other comments, and congratulations on getting
> your first commit into the Linux kernel.

When I think about it, I did propose a kernel patch in 2002, to
introduce /proc/sys/kernel/core_pattern, so this is my second patch.
All the mailing lists, and formal review process this time seemed
daunting at first, though, so thank you for guiding me through the
process.

Regards
Jes

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666
  2022-04-21 16:07   ` Jes Klinke
@ 2022-04-21 16:13     ` Jarkko Sakkinen
  0 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2022-04-21 16:13 UTC (permalink / raw)
  To: Jes Klinke
  Cc: Paul Menzel, Jes B. Klinke, linux-integrity, Jason Gunthorpe,
	Peter Huewe, linux-kernel

On Thu, Apr 21, 2022 at 09:07:21AM -0700, Jes Klinke wrote:
> Dear Paul,
> 
> On Wed, Apr 20, 2022 at 3:52 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >
> > Dear Jes,
> >
> >
> > Am 20.04.22 um 01:37 schrieb Jes B. Klinke:
> > > Accept one additional numerical value of DID:VID for next generation
> > > Google TPM with new firmware, to be used in future Chromebooks.
> > >
> > > The TPM with the new firmware has the code name TI50, and is going to
> > > use the same interfaces.
> > >
> > > Signed-off-by: Jes B. Klinke <jbk@chromium.org>
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > ---
> > >
> > >   drivers/char/tpm/tpm_tis_i2c_cr50.c | 7 ++++---
> > >   1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> > > index f6c0affbb4567..4ddb8ff3a8569 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,15 +743,15 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
> > >       }
> > >
> > >       vendor = le32_to_cpup((__le32 *)buf);
> > > -     if (vendor != TPM_CR50_I2C_DID_VID) {
> > > +     if (vendor != TPM_CR50_I2C_DID_VID && vendor != TPM_TI50_I2C_DID_VID) {
> > >               dev_err(dev, "Vendor ID did not match! ID was %08x\n", vendor);
> > >               tpm_cr50_release_locality(chip, true);
> > >               return -ENODEV;
> > >       }
> > >
> > > -     dev_info(dev, "cr50 TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> > > +     dev_info(dev, "%s TPM 2.0 (i2c 0x%02x irq %d id 0x%x)\n",
> > > +              vendor == TPM_TI50_I2C_DID_VID ? "ti50" : "cr50",
> > >                client->addr, client->irq, vendor >> 16);
> > > -
> >
> > Remove this blank line wasn’t needed, I gues.
> 
> You are right, I should probably have left the blank line untouched.
> It does not hurt the readability of the code much to remove it,
> though.

I don't mind removing it.

> 
> > >       return tpm_chip_register(chip);
> > >   }
> > >
> >
> > Thank for addressing the other comments, and congratulations on getting
> > your first commit into the Linux kernel.
> 
> When I think about it, I did propose a kernel patch in 2002, to
> introduce /proc/sys/kernel/core_pattern, so this is my second patch.
> All the mailing lists, and formal review process this time seemed
> daunting at first, though, so thank you for guiding me through the
> process.

It gets easier over time when your patch count increases.

Compared to many Github projects, I find actually contributing to kernel
easier than many of those, because form is so rigid, but this of course
happens over time. I.e. over time you can estimate a lot how likely is
patch get to be accepted.

Also it is good to remember that kernel maintainers might sound a bit
impolite, not because they want to be, but when you review dozens of
patches in a day, you have to be somewhat mechanic how you response,
and simply do not have time to be "verbosely polite".

> Regards
> Jes

BR, Jarkko

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-04-21 16:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 23:37 [PATCH v2] tpm: cr50: Add new device/vendor ID 0x504a6666 Jes B. Klinke
2022-04-20  7:17 ` Jarkko Sakkinen
2022-04-20 10:52 ` Paul Menzel
2022-04-21 16:07   ` Jes Klinke
2022-04-21 16:13     ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.