diff mbox

[tpmdd-devel,v8,0/8] TPM 2.0 support

Message ID 201412030021.07882.PeterHuewe@gmx.de
State Superseded, archived
Headers show

Commit Message

Peter Hüwe Dec. 2, 2014, 11:21 p.m. UTC
Am Mittwoch, 3. Dezember 2014, 00:16:19 schrieb Peter Hüwe:
> Am Dienstag, 2. Dezember 2014, 23:31:12 schrieb Jarkko Sakkinen:
> > This patch set enables TPM2 protocol and provides drivers for FIFO and
> > CRB interfaces. This patch set does not export any sysfs attributes for
> > TPM 2.0 because existing sysfs attributes have three non-trivial issues:
> > 
> > - They are associated with the platform device instead of character
> > 
> >   device.
> > 
> > - They are are not trivial key-value pairs but contain text that is
> > 
> >   not easily parsed by a computer.
> > 
> > - Raciness as described in
> > 
> > http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly
> > /
> > 
> > This is too big effort to be included into this patch set and requires
> > more discussion.
> > 
> > v2:
> > - Improved struct tpm_chip life-cycle by taking advantage of devres
> > 
> >   API.
> > 
> > - Refined sysfs attributes as simple key-values thereby not repeating
> > 
> >   mistakes in TPM1 sysfs attributes.
> > 
> > - Documented functions in tpm-chip.c and tpm2-cmd.c.
> > - Documented sysfs attributes.
> > 
> > v3:
> > - Lots of fixes in calling order in device drivers (thanks to Jason
> > 
> >   Gunthorpe for pointing these out!).
> > 
> > - Attach sysfs attributes to the misc device because it represents
> > 
> >   TPM device to the user space.
> > 
> > v4:
> > - Disable sysfs attibutes for TPM 2.0 for until we can sort out the
> > 
> >   best approach for them.
> > 
> > - Fixed all the style issues found with checkpatch.pl.
> > 
> > v5:
> > - missing EXPORT_SYMBOL_GPL()
> > - own class for TPM devices used for TPM 2.0 devices and onwards.
> > 
> > v6:
> > - Non-racy initialization for sysfs attributes using struct device's
> > 
> >   groups field.
> > 
> > - The class 'tpm' is used now for all TPM devices. For the first device
> > 
> >   node major MISC_MAJOR and minor TPM_MINOR is used in order to retain
> >   backwards compatability.
> > 
> > v7:
> > - Release device number and free struct tpm_chip memory inside
> > 
> >   tpm_dev_release callback.
> > 
> > - Moved code from tpm-interface.c and tpm_dev.c to tpm-chip.c.
> > 
> > v8:
> > - Cleaned up unneeded cast from tpm_transmit_cmd().
> > - Cleaned up redundant PPI_VERSION_LEN constant from tpm_ppi.c.
> > - Fixed tpm_tis to use tpm2_calc_ordinal_duration() for TPM2 devices.
> > - tpm_crb: in crb_recv, check that count can hold the TPM header at
> > 
> >   minimum.
> > 
> > - tpm_crb: add enumerations for bit flags in start and cancel fields
> > 
> >   of the control area.
> > 
> > - tpm_crb: use ioremap() for command and response buffer because
> > 
> >   they might be anywhere.
> > 
> > - tpm_crb: use IO access functions for reading ioremapped buffers
> > 
> >   because using direct pointers is not portable.
> > 
> > - tpm_crb: only apply ACPI start if start method reported by the
> > 
> >   TPM2 ACPI table allows it.
> > 
> > - In tpm2_pcr_read() just calculate index and bit and get rid of
> > 
> >   hacky loop.
> > 
> > - Do not add sysfs attributes for TPM 2.0 devices.
> > 
> > Jarkko Sakkinen (7):
> >   tpm: merge duplicate transmit_cmd() functions
> >   tpm: two-phase chip management functions
> >   tpm: fix raciness of PPI interface lookup
> >   tpm: rename chip->dev to chip->pdev
> >   tpm: device class for tpm
> >   tpm: TPM 2.0 baseline support
> >   tpm: TPM 2.0 CRB Interface
> > 
> > Will Arthur (1):
> >   tpm: TPM 2.0 FIFO Interface
> >  
> >  drivers/char/tpm/Kconfig            |   9 +
> >  drivers/char/tpm/Makefile           |   3 +-
> >  drivers/char/tpm/tpm-chip.c         | 251 ++++++++++++++++
> >  drivers/char/tpm/tpm-dev.c          |  42 +--
> >  drivers/char/tpm/tpm-interface.c    | 261 ++++++----------
> >  drivers/char/tpm/tpm-sysfs.c        |  29 +-
> >  drivers/char/tpm/tpm.h              | 113 ++++++-
> >  drivers/char/tpm/tpm2-cmd.c         | 571
> > 
> > ++++++++++++++++++++++++++++++++++++ drivers/char/tpm/tpm_atmel.c       
> > |
> > 
> >  25 +-
> >  drivers/char/tpm/tpm_crb.c          | 356 ++++++++++++++++++++++
> >  drivers/char/tpm/tpm_i2c_atmel.c    |  49 ++--
> >  drivers/char/tpm/tpm_i2c_infineon.c |  43 +--
> >  drivers/char/tpm/tpm_i2c_nuvoton.c  |  68 ++---
> 
> When applying to linux-v3.18-rc6 I get this new coccinelle warning:
> drivers/char/tpm/tpm_i2c_nuvoton.c:607:1-3: WARNING: end returns can be
> simpified
> 
> 
> make -C ../../../linux/ M=$(pwd) coccicheck
> 
Consider folding this patch to remove the warning

----

>From cb1b82859ba98f8573624905d2b3cc8d10f456b2 Mon Sep 17 00:00:00 2001
From: Peter Huewe <peterhuewe@gmx.de>
Date: Wed, 3 Dec 2014 00:18:52 +0100
Subject: [PATCH] tpm:tpm_i2c_nuvoton: simpyl return statements

if !rc evals to false it is 0
-> we can return rc in both cases

Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
 drivers/char/tpm/tpm_i2c_nuvoton.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Aaro Koskinen Dec. 2, 2014, 11:24 p.m. UTC | #1
Hi,

On Wed, Dec 03, 2014 at 12:21:07AM +0100, Peter Hüwe wrote:
> --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> @@ -605,10 +605,8 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
>  		return -ENODEV;
>  
>  	rc = tpm_chip_register(chip);
> -	if (rc)
> -		return rc;
>  
> -	return 0;
> +	return rc;

Maybe just return tpm_chip_register(chip)?

A.

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
Peter Hüwe Dec. 2, 2014, 11:33 p.m. UTC | #2
Am Mittwoch, 3. Dezember 2014, 00:24:43 schrieb Aaro Koskinen:
> Hi,
> 
> On Wed, Dec 03, 2014 at 12:21:07AM +0100, Peter Hüwe wrote:
> > --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> > +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> > @@ -605,10 +605,8 @@ static int i2c_nuvoton_probe(struct i2c_client
> > *client,
> > 
> >  		return -ENODEV;
> >  	
> >  	rc = tpm_chip_register(chip);
> > 
> > -	if (rc)
> > -		return rc;
> > 
> > -	return 0;
> > +	return rc;
> 
> Maybe just return tpm_chip_register(chip)?

Even better.

(I should go to bed now ;)
Peter

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
Joe Perches Dec. 2, 2014, 11:48 p.m. UTC | #3
On Wed, 2014-12-03 at 00:33 +0100, Peter Hüwe wrote:
> Am Mittwoch, 3. Dezember 2014, 00:24:43 schrieb Aaro Koskinen:
> > On Wed, Dec 03, 2014 at 12:21:07AM +0100, Peter Hüwe wrote:
> > > --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> > > +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> > > @@ -605,10 +605,8 @@ static int i2c_nuvoton_probe(struct i2c_client
> > > *client,
> > > 
> > >  		return -ENODEV;
> > >  	
> > >  	rc = tpm_chip_register(chip);
> > > 
> > > -	if (rc)
> > > -		return rc;
> > > 
> > > -	return 0;
> > > +	return rc;
> > 
> > Maybe just return tpm_chip_register(chip)?
> 
> Even better.

The pattern:

	foo = bar();
	if (foo)
		return foo;

	return 0;

is fairly common.

There are a few hundred in the kernel.

$ grep-2.5.4 -nrP --include=*.[ch] "\b(\w+)\s*=\s*[^;]*;\s*if\s*\(\s*\1\s*\)\s*return\s*\1\s*;\s*return\s*0;" * | \
  grep -oP '^.+:\d+:' | \
  wc -l
308



------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
Thomas Gleixner Dec. 3, 2014, 12:03 a.m. UTC | #4
On Wed, 3 Dec 2014, Peter Hüwe wrote:
> From: Peter Huewe <peterhuewe@gmx.de>
> Date: Wed, 3 Dec 2014 00:18:52 +0100
> Subject: [PATCH] tpm:tpm_i2c_nuvoton: simpyl return statements

simpyl?
 
> if !rc evals to false it is 0
> -> we can return rc in both cases

Why assigning rc and returning it when you can return the result of
tpm_chip_register() right away?
 
Thanks,

	tglx
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 14246e2..79f4fef 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -605,10 +605,8 @@  static int i2c_nuvoton_probe(struct i2c_client *client,
 		return -ENODEV;
 
 	rc = tpm_chip_register(chip);
-	if (rc)
-		return rc;
 
-	return 0;
+	return rc;
 }
 
 static int i2c_nuvoton_remove(struct i2c_client *client)