| Message ID | 1448485536-7282-5-git-send-email-jgunthorpe@obsidianresearch.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > Just pull it into tpm_get_timeouts. I want a bit more verbose commit message but can do it myself. No need to resubmit jusst for that. This is a pure clean up, right? I guess tpm_crb should call this too but it can be postponed (no need to resubmit for that change either). > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> /Jarkko > --- > drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++ > drivers/char/tpm/tpm_crb.c | 14 +++----------- > drivers/char/tpm/tpm_tis.c | 23 ++++++----------------- > 3 files changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index afdc83602e9f..e2fa89c88304 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -503,6 +503,21 @@ int tpm_get_timeouts(struct tpm_chip *chip) > struct duration_t *duration_cap; > ssize_t rc; > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > + /* Fixed timeouts for TPM2 */ > + chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > + chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > + chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > + chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > + chip->vendor.duration[TPM_SHORT] = > + msecs_to_jiffies(TPM2_DURATION_SHORT); > + chip->vendor.duration[TPM_MEDIUM] = > + msecs_to_jiffies(TPM2_DURATION_MEDIUM); > + chip->vendor.duration[TPM_LONG] = > + msecs_to_jiffies(TPM2_DURATION_LONG); > + return 0; > + } > + > tpm_cmd.header.in = tpm_getcap_header; > tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP; > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4); > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 4bb9727c1047..8342cf51ffdc 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -284,17 +284,9 @@ static int crb_acpi_add(struct acpi_device *device) > > chip->vendor.priv = priv; > > - /* Default timeouts and durations */ > - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > - chip->vendor.duration[TPM_SHORT] = > - msecs_to_jiffies(TPM2_DURATION_SHORT); > - chip->vendor.duration[TPM_MEDIUM] = > - msecs_to_jiffies(TPM2_DURATION_MEDIUM); > - chip->vendor.duration[TPM_LONG] = > - msecs_to_jiffies(TPM2_DURATION_LONG); > + rc = tpm_get_timeouts(chip); > + if (rc) > + return rc; > > chip->acpi_dev_handle = device->handle; > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index c07022a9c0d5..65a252c7ec5f 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -850,18 +850,13 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > iowrite8(irq_r, chip->vendor.iobase + > TPM_INT_VECTOR(chip->vendor.locality)); > > - if (chip->flags & TPM_CHIP_FLAG_TPM2) { > - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > - chip->vendor.duration[TPM_SHORT] = > - msecs_to_jiffies(TPM2_DURATION_SHORT); > - chip->vendor.duration[TPM_MEDIUM] = > - msecs_to_jiffies(TPM2_DURATION_MEDIUM); > - chip->vendor.duration[TPM_LONG] = > - msecs_to_jiffies(TPM2_DURATION_LONG); > + if (tpm_get_timeouts(chip)) { > + dev_err(dev, "Could not get TPM timeouts and durations\n"); > + rc = -ENODEV; > + goto out_err; > + } > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > rc = tpm2_do_selftest(chip); > if (rc == TPM2_RC_INITIALIZE) { > dev_warn(dev, "Firmware has not started TPM\n"); > @@ -877,12 +872,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > goto out_err; > } > } else { > - if (tpm_get_timeouts(chip)) { > - dev_err(dev, "Could not get TPM timeouts and durations\n"); > - rc = -ENODEV; > - goto out_err; > - } > - > if (tpm_do_selftest(chip)) { > dev_err(dev, "TPM self test failed\n"); > rc = -ENODEV; > -- > 2.1.4 > ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
On Mon, Nov 30, 2015 at 05:35:46PM +0200, Jarkko Sakkinen wrote: > On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > > Just pull it into tpm_get_timeouts. > > I want a bit more verbose commit message but can do it myself. No need > to resubmit jusst for that. This is a pure clean up, right? > > I guess tpm_crb should call this too but it can be postponed (no need > to resubmit for that change either). I did it too: > > drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++ > > drivers/char/tpm/tpm_crb.c | 14 +++----------- Jason ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
On Mon, Nov 30, 2015 at 10:29:53AM -0700, Jason Gunthorpe wrote: > On Mon, Nov 30, 2015 at 05:35:46PM +0200, Jarkko Sakkinen wrote: > > On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > > > Just pull it into tpm_get_timeouts. > > > > I want a bit more verbose commit message but can do it myself. No need > > to resubmit jusst for that. This is a pure clean up, right? > > > > I guess tpm_crb should call this too but it can be postponed (no need > > to resubmit for that change either). > > I did it too: > > > > drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++ > > > drivers/char/tpm/tpm_crb.c | 14 +++----------- Oops right. Thanks. > Jason /Jarkko ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > Just pull it into tpm_get_timeouts. I will squash this with the previous patch. Too high granularity, stuff is just moved from one place to another. Hard to make sense for a reviewer when add and remove parts in different patches. > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> /Jarkko > --- > drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++ > drivers/char/tpm/tpm_crb.c | 14 +++----------- > drivers/char/tpm/tpm_tis.c | 23 ++++++----------------- > 3 files changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index afdc83602e9f..e2fa89c88304 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -503,6 +503,21 @@ int tpm_get_timeouts(struct tpm_chip *chip) > struct duration_t *duration_cap; > ssize_t rc; > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > + /* Fixed timeouts for TPM2 */ > + chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > + chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > + chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > + chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > + chip->vendor.duration[TPM_SHORT] = > + msecs_to_jiffies(TPM2_DURATION_SHORT); > + chip->vendor.duration[TPM_MEDIUM] = > + msecs_to_jiffies(TPM2_DURATION_MEDIUM); > + chip->vendor.duration[TPM_LONG] = > + msecs_to_jiffies(TPM2_DURATION_LONG); > + return 0; > + } > + > tpm_cmd.header.in = tpm_getcap_header; > tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP; > tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4); > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 4bb9727c1047..8342cf51ffdc 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -284,17 +284,9 @@ static int crb_acpi_add(struct acpi_device *device) > > chip->vendor.priv = priv; > > - /* Default timeouts and durations */ > - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > - chip->vendor.duration[TPM_SHORT] = > - msecs_to_jiffies(TPM2_DURATION_SHORT); > - chip->vendor.duration[TPM_MEDIUM] = > - msecs_to_jiffies(TPM2_DURATION_MEDIUM); > - chip->vendor.duration[TPM_LONG] = > - msecs_to_jiffies(TPM2_DURATION_LONG); > + rc = tpm_get_timeouts(chip); > + if (rc) > + return rc; > > chip->acpi_dev_handle = device->handle; > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index c07022a9c0d5..65a252c7ec5f 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -850,18 +850,13 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > iowrite8(irq_r, chip->vendor.iobase + > TPM_INT_VECTOR(chip->vendor.locality)); > > - if (chip->flags & TPM_CHIP_FLAG_TPM2) { > - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); > - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); > - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); > - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); > - chip->vendor.duration[TPM_SHORT] = > - msecs_to_jiffies(TPM2_DURATION_SHORT); > - chip->vendor.duration[TPM_MEDIUM] = > - msecs_to_jiffies(TPM2_DURATION_MEDIUM); > - chip->vendor.duration[TPM_LONG] = > - msecs_to_jiffies(TPM2_DURATION_LONG); > + if (tpm_get_timeouts(chip)) { > + dev_err(dev, "Could not get TPM timeouts and durations\n"); > + rc = -ENODEV; > + goto out_err; > + } > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > rc = tpm2_do_selftest(chip); > if (rc == TPM2_RC_INITIALIZE) { > dev_warn(dev, "Firmware has not started TPM\n"); > @@ -877,12 +872,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, > goto out_err; > } > } else { > - if (tpm_get_timeouts(chip)) { > - dev_err(dev, "Could not get TPM timeouts and durations\n"); > - rc = -ENODEV; > - goto out_err; > - } > - > if (tpm_do_selftest(chip)) { > dev_err(dev, "TPM self test failed\n"); > rc = -ENODEV; > -- > 2.1.4 > ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
On Mon, Nov 30, 2015 at 11:51:37PM +0200, Jarkko Sakkinen wrote: > On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > > Just pull it into tpm_get_timeouts. > > I will squash this with the previous patch. Too high granularity, stuff > is just moved from one place to another. Hard to make sense for a > reviewer when add and remove parts in different patches. I wouldn't. Again, this is pure code motion, and is totally stand alone. The prior patch is a functional change that could have an impact and has no relation at all to this code motion. It is best practice to keep those distinct things. Jason ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
On Mon, Nov 30, 2015 at 02:59:11PM -0700, Jason Gunthorpe wrote: > On Mon, Nov 30, 2015 at 11:51:37PM +0200, Jarkko Sakkinen wrote: > > On Wed, Nov 25, 2015 at 02:05:33PM -0700, Jason Gunthorpe wrote: > > > Just pull it into tpm_get_timeouts. > > > > I will squash this with the previous patch. Too high granularity, stuff > > is just moved from one place to another. Hard to make sense for a > > reviewer when add and remove parts in different patches. > > I wouldn't. Again, this is pure code motion, and is totally stand > alone. > > The prior patch is a functional change that could have an impact > and has no relation at all to this code motion. > > It is best practice to keep those distinct things. I keep my position here. For me it is easier to backtrack later on what was made and quicker to backport. Probably not the biggest concern for you either so no much harm done... > Jason /Jarkko ------------------------------------------------------------------------------ Go from Idea to Many App Stores Faster with Intel(R) XDK Give your users amazing mobile app experiences with Intel(R) XDK. Use one codebase in this all-in-one HTML5 development environment. Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs. http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index afdc83602e9f..e2fa89c88304 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -503,6 +503,21 @@ int tpm_get_timeouts(struct tpm_chip *chip) struct duration_t *duration_cap; ssize_t rc; + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + /* Fixed timeouts for TPM2 */ + chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); + chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); + chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); + chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); + chip->vendor.duration[TPM_SHORT] = + msecs_to_jiffies(TPM2_DURATION_SHORT); + chip->vendor.duration[TPM_MEDIUM] = + msecs_to_jiffies(TPM2_DURATION_MEDIUM); + chip->vendor.duration[TPM_LONG] = + msecs_to_jiffies(TPM2_DURATION_LONG); + return 0; + } + tpm_cmd.header.in = tpm_getcap_header; tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP; tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4); diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index 4bb9727c1047..8342cf51ffdc 100644 --- a/drivers/char/tpm/tpm_crb.c +++ b/drivers/char/tpm/tpm_crb.c @@ -284,17 +284,9 @@ static int crb_acpi_add(struct acpi_device *device) chip->vendor.priv = priv; - /* Default timeouts and durations */ - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); - chip->vendor.duration[TPM_SHORT] = - msecs_to_jiffies(TPM2_DURATION_SHORT); - chip->vendor.duration[TPM_MEDIUM] = - msecs_to_jiffies(TPM2_DURATION_MEDIUM); - chip->vendor.duration[TPM_LONG] = - msecs_to_jiffies(TPM2_DURATION_LONG); + rc = tpm_get_timeouts(chip); + if (rc) + return rc; chip->acpi_dev_handle = device->handle; diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index c07022a9c0d5..65a252c7ec5f 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -850,18 +850,13 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, iowrite8(irq_r, chip->vendor.iobase + TPM_INT_VECTOR(chip->vendor.locality)); - if (chip->flags & TPM_CHIP_FLAG_TPM2) { - chip->vendor.timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A); - chip->vendor.timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B); - chip->vendor.timeout_c = msecs_to_jiffies(TPM2_TIMEOUT_C); - chip->vendor.timeout_d = msecs_to_jiffies(TPM2_TIMEOUT_D); - chip->vendor.duration[TPM_SHORT] = - msecs_to_jiffies(TPM2_DURATION_SHORT); - chip->vendor.duration[TPM_MEDIUM] = - msecs_to_jiffies(TPM2_DURATION_MEDIUM); - chip->vendor.duration[TPM_LONG] = - msecs_to_jiffies(TPM2_DURATION_LONG); + if (tpm_get_timeouts(chip)) { + dev_err(dev, "Could not get TPM timeouts and durations\n"); + rc = -ENODEV; + goto out_err; + } + if (chip->flags & TPM_CHIP_FLAG_TPM2) { rc = tpm2_do_selftest(chip); if (rc == TPM2_RC_INITIALIZE) { dev_warn(dev, "Firmware has not started TPM\n"); @@ -877,12 +872,6 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info, goto out_err; } } else { - if (tpm_get_timeouts(chip)) { - dev_err(dev, "Could not get TPM timeouts and durations\n"); - rc = -ENODEV; - goto out_err; - } - if (tpm_do_selftest(chip)) { dev_err(dev, "TPM self test failed\n"); rc = -ENODEV;
Just pull it into tpm_get_timeouts. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/char/tpm/tpm-interface.c | 15 +++++++++++++++ drivers/char/tpm/tpm_crb.c | 14 +++----------- drivers/char/tpm/tpm_tis.c | 23 ++++++----------------- 3 files changed, 24 insertions(+), 28 deletions(-)