linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* tpm: Remainder of earlier clean up
@ 2013-11-04  3:38 Jason Gunthorpe
  2013-11-04  3:38 ` [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c Jason Gunthorpe
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

Here is the last five patches from the prior series I sent that didn't get
picked up yet.

There are no changes, these are just rebased onto 3.12rc7 + Peter's for-james
branch. (Peter: Note, there are TPM changes to the Xen driver in 3.12rc7 that
are not in your for-james branch)

The intent of these patches is to reduce the duplicated code that is present
in all the drivers by migrating it into the core.

I've placed the patches on my github:
 https://github.com/jgunthorpe/linux/commits/for-tpm

 drivers/char/tpm/Makefile           |    2 +-
 drivers/char/tpm/tpm-dev.c          |  213 +++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-interface.c    |  487 +++++-------------------------------------------------------------------------------
 drivers/char/tpm/tpm-sysfs.c        |  318 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h              |   83 +++++++--------
 drivers/char/tpm/tpm_atmel.c        |   28 +----
 drivers/char/tpm/tpm_i2c_atmel.c    |   42 +-------
 drivers/char/tpm/tpm_i2c_infineon.c |   42 +-------
 drivers/char/tpm/tpm_i2c_nuvoton.c  |   42 +-------
 drivers/char/tpm/tpm_i2c_stm_st33.c |   41 +------
 drivers/char/tpm/tpm_ibmvtpm.c      |   40 +------
 drivers/char/tpm/tpm_infineon.c     |   28 +----
 drivers/char/tpm/tpm_nsc.c          |   28 +----
 drivers/char/tpm/tpm_tis.c          |   49 +--------
 drivers/char/tpm/xen-tpmfront.c     |   45 +-------
 include/linux/tpm.h                 |   12 +++
 16 files changed, 624 insertions(+), 876 deletions(-)


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

* [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
@ 2013-11-04  3:38 ` Jason Gunthorpe
  2013-11-16 22:18   ` [tpmdd-devel] " Ashley Lai
  2013-11-04  3:38 ` [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c Jason Gunthorpe
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

CLASS-dev.c is a common idiom for Linux subsystems

This pulls all the code related to the miscdev into tpm-dev.c and makes it
static. The identical file_operation structs in the drivers are purged and the
tpm common code unconditionally creates the miscdev.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 drivers/char/tpm/Makefile           |   2 +-
 drivers/char/tpm/tpm-dev.c          | 199 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-interface.c    | 178 ++------------------------------
 drivers/char/tpm/tpm.h              |  11 +-
 drivers/char/tpm/tpm_atmel.c        |  10 --
 drivers/char/tpm/tpm_i2c_atmel.c    |  10 --
 drivers/char/tpm/tpm_i2c_infineon.c |  10 --
 drivers/char/tpm/tpm_i2c_nuvoton.c  |  10 --
 drivers/char/tpm/tpm_i2c_stm_st33.c |  10 --
 drivers/char/tpm/tpm_ibmvtpm.c      |  10 --
 drivers/char/tpm/tpm_infineon.c     |  10 --
 drivers/char/tpm/tpm_nsc.c          |  10 --
 drivers/char/tpm/tpm_tis.c          |  11 --
 drivers/char/tpm/xen-tpmfront.c     |  12 ---
 14 files changed, 216 insertions(+), 277 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-dev.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index b80a400..d835e87 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o
+tpm-y := tpm-interface.o tpm-dev.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
new file mode 100644
index 0000000..8d94e97
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Copyright (C) 2013 Obsidian Reearch Corp
+ * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
+ *
+ * Device file system interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/miscdevice.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include "tpm.h"
+
+static void user_reader_timeout(unsigned long ptr)
+{
+	struct tpm_chip *chip = (struct tpm_chip *) ptr;
+
+	schedule_work(&chip->work);
+}
+
+static void timeout_work(struct work_struct *work)
+{
+	struct tpm_chip *chip = container_of(work, struct tpm_chip, work);
+
+	mutex_lock(&chip->buffer_mutex);
+	atomic_set(&chip->data_pending, 0);
+	memset(chip->data_buffer, 0, TPM_BUFSIZE);
+	mutex_unlock(&chip->buffer_mutex);
+}
+
+static int tpm_open(struct inode *inode, struct file *file)
+{
+	struct miscdevice *misc = file->private_data;
+	struct tpm_chip *chip = container_of(misc, struct tpm_chip,
+					     vendor.miscdev);
+
+	/* It's assured that the chip will be opened just once,
+	 * by the check of is_open variable, which is protected
+	 * by driver_lock. */
+	if (test_and_set_bit(0, &chip->is_open)) {
+		dev_dbg(chip->dev, "Another process owns this TPM\n");
+		return -EBUSY;
+	}
+
+	chip->data_buffer = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+	if (chip->data_buffer == NULL) {
+		clear_bit(0, &chip->is_open);
+		return -ENOMEM;
+	}
+
+	atomic_set(&chip->data_pending, 0);
+
+	file->private_data = chip;
+	get_device(chip->dev);
+	return 0;
+}
+
+static ssize_t tpm_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off)
+{
+	struct tpm_chip *chip = file->private_data;
+	ssize_t ret_size;
+	int rc;
+
+	del_singleshot_timer_sync(&chip->user_read_timer);
+	flush_work(&chip->work);
+	ret_size = atomic_read(&chip->data_pending);
+	if (ret_size > 0) {	/* relay data */
+		ssize_t orig_ret_size = ret_size;
+		if (size < ret_size)
+			ret_size = size;
+
+		mutex_lock(&chip->buffer_mutex);
+		rc = copy_to_user(buf, chip->data_buffer, ret_size);
+		memset(chip->data_buffer, 0, orig_ret_size);
+		if (rc)
+			ret_size = -EFAULT;
+
+		mutex_unlock(&chip->buffer_mutex);
+	}
+
+	atomic_set(&chip->data_pending, 0);
+
+	return ret_size;
+}
+
+static ssize_t tpm_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off)
+{
+	struct tpm_chip *chip = file->private_data;
+	size_t in_size = size;
+	ssize_t out_size;
+
+	/* cannot perform a write until the read has cleared
+	   either via tpm_read or a user_read_timer timeout.
+	   This also prevents splitted buffered writes from blocking here.
+	*/
+	if (atomic_read(&chip->data_pending) != 0)
+		return -EBUSY;
+
+	if (in_size > TPM_BUFSIZE)
+		return -E2BIG;
+
+	mutex_lock(&chip->buffer_mutex);
+
+	if (copy_from_user
+	    (chip->data_buffer, (void __user *) buf, in_size)) {
+		mutex_unlock(&chip->buffer_mutex);
+		return -EFAULT;
+	}
+
+	/* atomic tpm command send and result receive */
+	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
+	if (out_size < 0) {
+		mutex_unlock(&chip->buffer_mutex);
+		return out_size;
+	}
+
+	atomic_set(&chip->data_pending, out_size);
+	mutex_unlock(&chip->buffer_mutex);
+
+	/* Set a timeout by which the reader must come claim the result */
+	mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
+
+	return in_size;
+}
+
+/*
+ * Called on file close
+ */
+static int tpm_release(struct inode *inode, struct file *file)
+{
+	struct tpm_chip *chip = file->private_data;
+
+	del_singleshot_timer_sync(&chip->user_read_timer);
+	flush_work(&chip->work);
+	file->private_data = NULL;
+	atomic_set(&chip->data_pending, 0);
+	kzfree(chip->data_buffer);
+	clear_bit(0, &chip->is_open);
+	put_device(chip->dev);
+	return 0;
+}
+
+static const struct file_operations tpm_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpm_open,
+	.read = tpm_read,
+	.write = tpm_write,
+	.release = tpm_release,
+};
+
+int tpm_dev_add_device(struct tpm_chip *chip)
+{
+	int rc;
+
+	mutex_init(&chip->buffer_mutex);
+	INIT_WORK(&chip->work, timeout_work);
+
+	setup_timer(&chip->user_read_timer, user_reader_timeout,
+			(unsigned long)chip);
+
+	chip->vendor.miscdev.fops = &tpm_fops;
+	if (chip->dev_num == 0)
+		chip->vendor.miscdev.minor = TPM_MINOR;
+	else
+		chip->vendor.miscdev.minor = MISC_DYNAMIC_MINOR;
+
+	chip->vendor.miscdev.name = chip->devname;
+	chip->vendor.miscdev.parent = chip->dev;
+
+	rc = misc_register(&chip->vendor.miscdev);
+	if (rc) {
+		chip->vendor.miscdev.name = NULL;
+		dev_warn(chip->dev,
+			"unable to misc_register %s, minor %d err=%d\n",
+			 chip->vendor.miscdev.name,
+			 chip->vendor.miscdev.minor, rc);
+	}
+	return rc;
+}
+
+void tpm_dev_del_device(struct tpm_chip *chip)
+{
+	if (chip->vendor.miscdev.name)
+		misc_deregister(&chip->vendor.miscdev);
+}
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 6ae41d3..9b9bb7b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -312,23 +312,6 @@ static const u8 tpm_ordinal_duration[TPM_MAX_ORDINAL] = {
 	TPM_MEDIUM,
 };
 
-static void user_reader_timeout(unsigned long ptr)
-{
-	struct tpm_chip *chip = (struct tpm_chip *) ptr;
-
-	schedule_work(&chip->work);
-}
-
-static void timeout_work(struct work_struct *work)
-{
-	struct tpm_chip *chip = container_of(work, struct tpm_chip, work);
-
-	mutex_lock(&chip->buffer_mutex);
-	atomic_set(&chip->data_pending, 0);
-	memset(chip->data_buffer, 0, TPM_BUFSIZE);
-	mutex_unlock(&chip->buffer_mutex);
-}
-
 /*
  * Returns max number of jiffies to wait
  */
@@ -355,8 +338,8 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 /*
  * Internal kernel interface to transmit TPM commands
  */
-static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
-			    size_t bufsiz)
+ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+		     size_t bufsiz)
 {
 	ssize_t rc;
 	u32 count, ordinal;
@@ -1151,127 +1134,6 @@ again:
 	return -ETIME;
 }
 EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
-/*
- * Device file system interface to the TPM
- *
- * It's assured that the chip will be opened just once,
- * by the check of is_open variable, which is protected
- * by driver_lock.
- */
-int tpm_open(struct inode *inode, struct file *file)
-{
-	struct miscdevice *misc = file->private_data;
-	struct tpm_chip *chip = container_of(misc, struct tpm_chip,
-					     vendor.miscdev);
-
-	if (test_and_set_bit(0, &chip->is_open)) {
-		dev_dbg(chip->dev, "Another process owns this TPM\n");
-		return -EBUSY;
-	}
-
-	chip->data_buffer = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
-	if (chip->data_buffer == NULL) {
-		clear_bit(0, &chip->is_open);
-		return -ENOMEM;
-	}
-
-	atomic_set(&chip->data_pending, 0);
-
-	file->private_data = chip;
-	get_device(chip->dev);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(tpm_open);
-
-/*
- * Called on file close
- */
-int tpm_release(struct inode *inode, struct file *file)
-{
-	struct tpm_chip *chip = file->private_data;
-
-	del_singleshot_timer_sync(&chip->user_read_timer);
-	flush_work(&chip->work);
-	file->private_data = NULL;
-	atomic_set(&chip->data_pending, 0);
-	kzfree(chip->data_buffer);
-	clear_bit(0, &chip->is_open);
-	put_device(chip->dev);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(tpm_release);
-
-ssize_t tpm_write(struct file *file, const char __user *buf,
-		  size_t size, loff_t *off)
-{
-	struct tpm_chip *chip = file->private_data;
-	size_t in_size = size;
-	ssize_t out_size;
-
-	/* cannot perform a write until the read has cleared
-	   either via tpm_read or a user_read_timer timeout.
-	   This also prevents splitted buffered writes from blocking here.
-	*/
-	if (atomic_read(&chip->data_pending) != 0)
-		return -EBUSY;
-
-	if (in_size > TPM_BUFSIZE)
-		return -E2BIG;
-
-	mutex_lock(&chip->buffer_mutex);
-
-	if (copy_from_user
-	    (chip->data_buffer, (void __user *) buf, in_size)) {
-		mutex_unlock(&chip->buffer_mutex);
-		return -EFAULT;
-	}
-
-	/* atomic tpm command send and result receive */
-	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
-	if (out_size < 0) {
-		mutex_unlock(&chip->buffer_mutex);
-		return out_size;
-	}
-
-	atomic_set(&chip->data_pending, out_size);
-	mutex_unlock(&chip->buffer_mutex);
-
-	/* Set a timeout by which the reader must come claim the result */
-	mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
-
-	return in_size;
-}
-EXPORT_SYMBOL_GPL(tpm_write);
-
-ssize_t tpm_read(struct file *file, char __user *buf,
-		 size_t size, loff_t *off)
-{
-	struct tpm_chip *chip = file->private_data;
-	ssize_t ret_size;
-	int rc;
-
-	del_singleshot_timer_sync(&chip->user_read_timer);
-	flush_work(&chip->work);
-	ret_size = atomic_read(&chip->data_pending);
-	if (ret_size > 0) {	/* relay data */
-		ssize_t orig_ret_size = ret_size;
-		if (size < ret_size)
-			ret_size = size;
-
-		mutex_lock(&chip->buffer_mutex);
-		rc = copy_to_user(buf, chip->data_buffer, ret_size);
-		memset(chip->data_buffer, 0, orig_ret_size);
-		if (rc)
-			ret_size = -EFAULT;
-
-		mutex_unlock(&chip->buffer_mutex);
-	}
-
-	atomic_set(&chip->data_pending, 0);
-
-	return ret_size;
-}
-EXPORT_SYMBOL_GPL(tpm_read);
 
 void tpm_remove_hardware(struct device *dev)
 {
@@ -1287,7 +1149,7 @@ void tpm_remove_hardware(struct device *dev)
 	spin_unlock(&driver_lock);
 	synchronize_rcu();
 
-	misc_deregister(&chip->vendor.miscdev);
+	tpm_dev_del_device(chip);
 	sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
 	tpm_remove_ppi(&dev->kobj);
 	tpm_bios_log_teardown(chip->bios_dir);
@@ -1480,15 +1342,9 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 	if (chip == NULL)
 		return NULL;
 
-	mutex_init(&chip->buffer_mutex);
 	mutex_init(&chip->tpm_mutex);
 	INIT_LIST_HEAD(&chip->list);
 
-	INIT_WORK(&chip->work, timeout_work);
-
-	setup_timer(&chip->user_read_timer, user_reader_timeout,
-			(unsigned long)chip);
-
 	memcpy(&chip->vendor, entry, sizeof(struct tpm_vendor_specific));
 
 	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
@@ -1496,40 +1352,26 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 	if (chip->dev_num >= TPM_NUM_DEVICES) {
 		dev_err(dev, "No available tpm device numbers\n");
 		goto out_free;
-	} else if (chip->dev_num == 0)
-		chip->vendor.miscdev.minor = TPM_MINOR;
-	else
-		chip->vendor.miscdev.minor = MISC_DYNAMIC_MINOR;
+	}
 
 	set_bit(chip->dev_num, dev_mask);
 
 	scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
 		  chip->dev_num);
-	chip->vendor.miscdev.name = chip->devname;
 
-	chip->vendor.miscdev.parent = dev;
 	chip->dev = get_device(dev);
 	chip->release = dev->release;
 	dev->release = tpm_dev_release;
 	dev_set_drvdata(dev, chip);
 
-	if (misc_register(&chip->vendor.miscdev)) {
-		dev_err(chip->dev,
-			"unable to misc_register %s, minor %d\n",
-			chip->vendor.miscdev.name,
-			chip->vendor.miscdev.minor);
+	if (tpm_dev_add_device(chip))
 		goto put_device;
-	}
 
-	if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group)) {
-		misc_deregister(&chip->vendor.miscdev);
-		goto put_device;
-	}
+	if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group))
+		goto del_misc;
 
-	if (tpm_add_ppi(&dev->kobj)) {
-		misc_deregister(&chip->vendor.miscdev);
-		goto put_device;
-	}
+	if (tpm_add_ppi(&dev->kobj))
+		goto del_misc;
 
 	chip->bios_dir = tpm_bios_log_setup(chip->devname);
 
@@ -1540,6 +1382,8 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 
 	return chip;
 
+del_misc:
+	tpm_dev_del_device(chip);
 put_device:
 	put_device(chip->dev);
 out_free:
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f328478..496228c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -323,25 +323,24 @@ struct tpm_cmd_t {
 
 ssize_t	tpm_getcap(struct device *, __be32, cap_t *, const char *);
 
+ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
+		     size_t bufsiz);
 extern int tpm_get_timeouts(struct tpm_chip *);
 extern void tpm_gen_interrupt(struct tpm_chip *);
 extern int tpm_do_selftest(struct tpm_chip *);
 extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
 extern struct tpm_chip* tpm_register_hardware(struct device *,
 				 const struct tpm_vendor_specific *);
-extern int tpm_open(struct inode *, struct file *);
-extern int tpm_release(struct inode *, struct file *);
-extern void tpm_dev_release(struct device *dev);
 extern void tpm_dev_vendor_release(struct tpm_chip *);
-extern ssize_t tpm_write(struct file *, const char __user *, size_t,
-			 loff_t *);
-extern ssize_t tpm_read(struct file *, char __user *, size_t, loff_t *);
 extern void tpm_remove_hardware(struct device *);
 extern int tpm_pm_suspend(struct device *);
 extern int tpm_pm_resume(struct device *);
 extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
 			     wait_queue_head_t *, bool);
 
+int tpm_dev_add_device(struct tpm_chip *chip);
+void tpm_dev_del_device(struct tpm_chip *chip);
+
 #ifdef CONFIG_ACPI
 extern int tpm_add_ppi(struct kobject *);
 extern void tpm_remove_ppi(struct kobject *);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index c9a528d..9692e2f 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -121,15 +121,6 @@ static bool tpm_atml_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == ATML_STATUS_READY);
 }
 
-static const struct file_operations atmel_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
@@ -154,7 +145,6 @@ static const struct tpm_vendor_specific tpm_atmel = {
 	.req_complete_val = ATML_STATUS_DATA_AVAIL,
 	.req_canceled = tpm_atml_req_canceled,
 	.attr_group = &atmel_attr_grp,
-	.miscdev = { .fops = &atmel_ops, },
 };
 
 static struct platform_device *pdev;
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index af6805b..079c19d 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -135,15 +135,6 @@ static u8 i2c_atmel_read_status(struct tpm_chip *chip)
 	return ATMEL_STS_OK;
 }
 
-static const struct file_operations i2c_atmel_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -187,7 +178,6 @@ static const struct tpm_vendor_specific i2c_atmel = {
 	.req_complete_val = ATMEL_STS_OK,
 	.req_canceled = i2c_atmel_req_canceled,
 	.attr_group = &i2c_atmel_attr_grp,
-	.miscdev.fops = &i2c_atmel_ops,
 };
 
 static int i2c_atmel_probe(struct i2c_client *client,
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index fefd2aa..c1ba7fa 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -566,15 +566,6 @@ static bool tpm_tis_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static const struct file_operations tis_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -613,7 +604,6 @@ static struct tpm_vendor_specific tpm_tis_i2c = {
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_i2c_req_canceled,
 	.attr_group = &tis_attr_grp,
-	.miscdev.fops = &tis_ops,
 };
 
 static int tpm_tis_i2c_init(struct device *dev)
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 6276fea..3766fe7 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -456,15 +456,6 @@ static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static const struct file_operations i2c_nuvoton_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -503,7 +494,6 @@ static const struct tpm_vendor_specific tpm_i2c = {
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = i2c_nuvoton_req_canceled,
 	.attr_group = &i2c_nuvoton_attr_grp,
-	.miscdev.fops = &i2c_nuvoton_ops,
 };
 
 /* The only purpose for the handler is to signal to any waiting threads that
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index cf68403..e519e68 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -574,15 +574,6 @@ static bool tpm_st33_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static const struct file_operations tpm_st33_i2c_fops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.read = tpm_read,
-	.write = tpm_write,
-	.open = tpm_open,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -616,7 +607,6 @@ static struct tpm_vendor_specific st_i2c_tpm = {
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_st33_i2c_req_canceled,
 	.attr_group = &stm_tpm_attr_grp,
-	.miscdev = {.fops = &tpm_st33_i2c_fops,},
 };
 
 static int interrupts;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 42ff798..14af0c1 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -404,15 +404,6 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == 0);
 }
 
-static const struct file_operations ibmvtpm_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -449,7 +440,6 @@ static const struct tpm_vendor_specific tpm_ibmvtpm = {
 	.req_complete_val = 0,
 	.req_canceled = tpm_ibmvtpm_req_canceled,
 	.attr_group = &ibmvtpm_attr_grp,
-	.miscdev = { .fops = &ibmvtpm_ops, },
 };
 
 static const struct dev_pm_ops tpm_ibmvtpm_pm_ops = {
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 2b480c2..c75c10c 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -386,15 +386,6 @@ static struct attribute *inf_attrs[] = {
 
 static struct attribute_group inf_attr_grp = {.attrs = inf_attrs };
 
-static const struct file_operations inf_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static const struct tpm_vendor_specific tpm_inf = {
 	.recv = tpm_inf_recv,
 	.send = tpm_inf_send,
@@ -403,7 +394,6 @@ static const struct tpm_vendor_specific tpm_inf = {
 	.req_complete_mask = 0,
 	.req_complete_val = 0,
 	.attr_group = &inf_attr_grp,
-	.miscdev = {.fops = &inf_ops,},
 };
 
 static const struct pnp_device_id tpm_inf_pnp_tbl[] = {
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 770c46f..a4acac9 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -232,15 +232,6 @@ static bool tpm_nsc_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == NSC_STATUS_RDY);
 }
 
-static const struct file_operations nsc_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
@@ -265,7 +256,6 @@ static const struct tpm_vendor_specific tpm_nsc = {
 	.req_complete_val = NSC_STATUS_OBF,
 	.req_canceled = tpm_nsc_req_canceled,
 	.attr_group = &nsc_attr_grp,
-	.miscdev = { .fops = &nsc_ops, },
 };
 
 static struct platform_device *pdev = NULL;
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 0732a1e..c606ca1 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -432,15 +432,6 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 	}
 }
 
-static const struct file_operations tis_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -479,8 +470,6 @@ static struct tpm_vendor_specific tpm_tis = {
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_req_canceled,
 	.attr_group = &tis_attr_grp,
-	.miscdev = {
-		    .fops = &tis_ops,},
 };
 
 static irqreturn_t tis_int_probe(int irq, void *dev_id)
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index c8ff4df..e3c7c01 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -143,15 +143,6 @@ static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	return length;
 }
 
-static const struct file_operations vtpm_ops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.open = tpm_open,
-	.read = tpm_read,
-	.write = tpm_write,
-	.release = tpm_release,
-};
-
 static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
 static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
 static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
@@ -191,9 +182,6 @@ static const struct tpm_vendor_specific tpm_vtpm = {
 	.req_complete_val  = VTPM_STATUS_IDLE | VTPM_STATUS_RESULT,
 	.req_canceled      = vtpm_req_canceled,
 	.attr_group = &vtpm_attr_grp,
-	.miscdev = {
-		.fops = &vtpm_ops,
-	},
 };
 
 static irqreturn_t tpmif_interrupt(int dummy, void *dev_id)
-- 
1.8.1.2


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

* [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
  2013-11-04  3:38 ` [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c Jason Gunthorpe
@ 2013-11-04  3:38 ` Jason Gunthorpe
  2013-11-16 23:53   ` [tpmdd-devel] " Ashley Lai
  2013-11-04  3:38 ` [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers Jason Gunthorpe
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

CLASS-sysfs.c is a common idiom for linux subsystems.

This pulls all the sysfs attribute functions and related code
into tpm-sysfs.c. To support this change some constants are moved
from tpm.c to tpm.h and __tpm_pcr_read is made non-static and is
called tpm_pcr_read_dev.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 drivers/char/tpm/Makefile           |   2 +-
 drivers/char/tpm/tpm-interface.c    | 281 +------------------------------
 drivers/char/tpm/tpm-sysfs.c        | 318 ++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm.h              |  54 +++---
 drivers/char/tpm/tpm_atmel.c        |  16 --
 drivers/char/tpm/tpm_i2c_atmel.c    |  30 ----
 drivers/char/tpm/tpm_i2c_infineon.c |  30 ----
 drivers/char/tpm/tpm_i2c_nuvoton.c  |  30 ----
 drivers/char/tpm/tpm_i2c_stm_st33.c |  25 ---
 drivers/char/tpm/tpm_ibmvtpm.c      |  28 ----
 drivers/char/tpm/tpm_infineon.c     |  16 --
 drivers/char/tpm/tpm_nsc.c          |  16 --
 drivers/char/tpm/tpm_tis.c          |  30 ----
 drivers/char/tpm/xen-tpmfront.c     |  31 ----
 14 files changed, 356 insertions(+), 551 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-sysfs.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index d835e87..4d85dd6 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the kernel tpm device drivers.
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
-tpm-y := tpm-interface.o tpm-dev.o
+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 9b9bb7b..5ab3caa 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -32,13 +32,6 @@
 #include "tpm.h"
 #include "tpm_eventlog.h"
 
-enum tpm_duration {
-	TPM_SHORT = 0,
-	TPM_MEDIUM = 1,
-	TPM_LONG = 2,
-	TPM_UNDEFINED,
-};
-
 #define TPM_MAX_ORDINAL 243
 #define TSC_MAX_ORDINAL 12
 #define TPM_PROTECTED_COMMAND 0x00
@@ -405,24 +398,6 @@ out:
 #define TPM_DIGEST_SIZE 20
 #define TPM_RET_CODE_IDX 6
 
-enum tpm_capabilities {
-	TPM_CAP_FLAG = cpu_to_be32(4),
-	TPM_CAP_PROP = cpu_to_be32(5),
-	CAP_VERSION_1_1 = cpu_to_be32(0x06),
-	CAP_VERSION_1_2 = cpu_to_be32(0x1A)
-};
-
-enum tpm_sub_capabilities {
-	TPM_CAP_PROP_PCR = cpu_to_be32(0x101),
-	TPM_CAP_PROP_MANUFACTURER = cpu_to_be32(0x103),
-	TPM_CAP_FLAG_PERM = cpu_to_be32(0x108),
-	TPM_CAP_FLAG_VOL = cpu_to_be32(0x109),
-	TPM_CAP_PROP_OWNER = cpu_to_be32(0x111),
-	TPM_CAP_PROP_TIS_TIMEOUT = cpu_to_be32(0x115),
-	TPM_CAP_PROP_TIS_DURATION = cpu_to_be32(0x120),
-
-};
-
 static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
 			    int len, const char *desc)
 {
@@ -442,7 +417,6 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
 }
 
 #define TPM_INTERNAL_RESULT_SIZE 200
-#define TPM_TAG_RQU_COMMAND cpu_to_be16(193)
 #define TPM_ORD_GET_CAP cpu_to_be32(101)
 #define TPM_ORD_GET_RANDOM cpu_to_be32(70)
 
@@ -642,70 +616,6 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	return rc;
 }
 
-ssize_t tpm_show_enabled(struct device *dev, struct device_attribute *attr,
-			char *buf)
-{
-	cap_t cap;
-	ssize_t rc;
-
-	rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
-			 "attempting to determine the permanent enabled state");
-	if (rc)
-		return 0;
-
-	rc = sprintf(buf, "%d\n", !cap.perm_flags.disable);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_show_enabled);
-
-ssize_t tpm_show_active(struct device *dev, struct device_attribute *attr,
-			char *buf)
-{
-	cap_t cap;
-	ssize_t rc;
-
-	rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
-			 "attempting to determine the permanent active state");
-	if (rc)
-		return 0;
-
-	rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_show_active);
-
-ssize_t tpm_show_owned(struct device *dev, struct device_attribute *attr,
-			char *buf)
-{
-	cap_t cap;
-	ssize_t rc;
-
-	rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
-			 "attempting to determine the owner state");
-	if (rc)
-		return 0;
-
-	rc = sprintf(buf, "%d\n", cap.owned);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_show_owned);
-
-ssize_t tpm_show_temp_deactivated(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	cap_t cap;
-	ssize_t rc;
-
-	rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
-			 "attempting to determine the temporary state");
-	if (rc)
-		return 0;
-
-	rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_show_temp_deactivated);
-
 /*
  * tpm_chip_find_get - return tpm_chip for given chip number
  */
@@ -735,7 +645,7 @@ static struct tpm_input_header pcrread_header = {
 	.ordinal = TPM_ORDINAL_PCRREAD
 };
 
-static int __tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
+int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 {
 	int rc;
 	struct tpm_cmd_t cmd;
@@ -770,7 +680,7 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
 	chip = tpm_chip_find_get(chip_num);
 	if (chip == NULL)
 		return -ENODEV;
-	rc = __tpm_pcr_read(chip, pcr_idx, res_buf);
+	rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
 	tpm_chip_put(chip);
 	return rc;
 }
@@ -894,189 +804,8 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 }
 EXPORT_SYMBOL_GPL(tpm_send);
 
-ssize_t tpm_show_pcrs(struct device *dev, struct device_attribute *attr,
-		      char *buf)
-{
-	cap_t cap;
-	u8 digest[TPM_DIGEST_SIZE];
-	ssize_t rc;
-	int i, j, num_pcrs;
-	char *str = buf;
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
-			"attempting to determine the number of PCRS");
-	if (rc)
-		return 0;
-
-	num_pcrs = be32_to_cpu(cap.num_pcrs);
-	for (i = 0; i < num_pcrs; i++) {
-		rc = __tpm_pcr_read(chip, i, digest);
-		if (rc)
-			break;
-		str += sprintf(str, "PCR-%02d: ", i);
-		for (j = 0; j < TPM_DIGEST_SIZE; j++)
-			str += sprintf(str, "%02X ", digest[j]);
-		str += sprintf(str, "\n");
-	}
-	return str - buf;
-}
-EXPORT_SYMBOL_GPL(tpm_show_pcrs);
-
-#define  READ_PUBEK_RESULT_SIZE 314
-#define TPM_ORD_READPUBEK cpu_to_be32(124)
-static struct tpm_input_header tpm_readpubek_header = {
-	.tag = TPM_TAG_RQU_COMMAND,
-	.length = cpu_to_be32(30),
-	.ordinal = TPM_ORD_READPUBEK
-};
-
-ssize_t tpm_show_pubek(struct device *dev, struct device_attribute *attr,
-		       char *buf)
-{
-	u8 *data;
-	struct tpm_cmd_t tpm_cmd;
-	ssize_t err;
-	int i, rc;
-	char *str = buf;
-
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	tpm_cmd.header.in = tpm_readpubek_header;
-	err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
-			"attempting to read the PUBEK");
-	if (err)
-		goto out;
-
-	/*
-	   ignore header 10 bytes
-	   algorithm 32 bits (1 == RSA )
-	   encscheme 16 bits
-	   sigscheme 16 bits
-	   parameters (RSA 12->bytes: keybit, #primes, expbit)
-	   keylenbytes 32 bits
-	   256 byte modulus
-	   ignore checksum 20 bytes
-	 */
-	data = tpm_cmd.params.readpubek_out_buffer;
-	str +=
-	    sprintf(str,
-		    "Algorithm: %02X %02X %02X %02X\n"
-		    "Encscheme: %02X %02X\n"
-		    "Sigscheme: %02X %02X\n"
-		    "Parameters: %02X %02X %02X %02X "
-		    "%02X %02X %02X %02X "
-		    "%02X %02X %02X %02X\n"
-		    "Modulus length: %d\n"
-		    "Modulus:\n",
-		    data[0], data[1], data[2], data[3],
-		    data[4], data[5],
-		    data[6], data[7],
-		    data[12], data[13], data[14], data[15],
-		    data[16], data[17], data[18], data[19],
-		    data[20], data[21], data[22], data[23],
-		    be32_to_cpu(*((__be32 *) (data + 24))));
-
-	for (i = 0; i < 256; i++) {
-		str += sprintf(str, "%02X ", data[i + 28]);
-		if ((i + 1) % 16 == 0)
-			str += sprintf(str, "\n");
-	}
-out:
-	rc = str - buf;
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_show_pubek);
-
-
-ssize_t tpm_show_caps(struct device *dev, struct device_attribute *attr,
-		      char *buf)
-{
-	cap_t cap;
-	ssize_t rc;
-	char *str = buf;
-
-	rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
-			"attempting to determine the manufacturer");
-	if (rc)
-		return 0;
-	str += sprintf(str, "Manufacturer: 0x%x\n",
-		       be32_to_cpu(cap.manufacturer_id));
-
-	/* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
-	rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
-			 "attempting to determine the 1.2 version");
-	if (!rc) {
-		str += sprintf(str,
-			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
-			       cap.tpm_version_1_2.Major,
-			       cap.tpm_version_1_2.Minor,
-			       cap.tpm_version_1_2.revMajor,
-			       cap.tpm_version_1_2.revMinor);
-	} else {
-		/* Otherwise just use TPM_STRUCT_VER */
-		rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
-				"attempting to determine the 1.1 version");
-		if (rc)
-			return 0;
-		str += sprintf(str,
-			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
-			       cap.tpm_version.Major,
-			       cap.tpm_version.Minor,
-			       cap.tpm_version.revMajor,
-			       cap.tpm_version.revMinor);
-	}
-
-	return str - buf;
-}
-EXPORT_SYMBOL_GPL(tpm_show_caps);
-
-ssize_t tpm_show_durations(struct device *dev, struct device_attribute *attr,
-			  char *buf)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	if (chip->vendor.duration[TPM_LONG] == 0)
-		return 0;
-
-	return sprintf(buf, "%d %d %d [%s]\n",
-		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
-		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
-		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
-		       chip->vendor.duration_adjusted
-		       ? "adjusted" : "original");
-}
-EXPORT_SYMBOL_GPL(tpm_show_durations);
-
-ssize_t tpm_show_timeouts(struct device *dev, struct device_attribute *attr,
-			  char *buf)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-
-	return sprintf(buf, "%d %d %d %d [%s]\n",
-		       jiffies_to_usecs(chip->vendor.timeout_a),
-		       jiffies_to_usecs(chip->vendor.timeout_b),
-		       jiffies_to_usecs(chip->vendor.timeout_c),
-		       jiffies_to_usecs(chip->vendor.timeout_d),
-		       chip->vendor.timeout_adjusted
-		       ? "adjusted" : "original");
-}
-EXPORT_SYMBOL_GPL(tpm_show_timeouts);
-
-ssize_t tpm_store_cancel(struct device *dev, struct device_attribute *attr,
-			const char *buf, size_t count)
-{
-	struct tpm_chip *chip = dev_get_drvdata(dev);
-	if (chip == NULL)
-		return 0;
-
-	chip->vendor.cancel(chip);
-	return count;
-}
-EXPORT_SYMBOL_GPL(tpm_store_cancel);
-
 static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
-					bool check_cancel, bool *canceled)
+				   bool check_cancel, bool *canceled)
 {
 	u8 status = chip->vendor.status(chip);
 
@@ -1150,7 +879,7 @@ void tpm_remove_hardware(struct device *dev)
 	synchronize_rcu();
 
 	tpm_dev_del_device(chip);
-	sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
+	tpm_sysfs_del_device(chip);
 	tpm_remove_ppi(&dev->kobj);
 	tpm_bios_log_teardown(chip->bios_dir);
 
@@ -1367,7 +1096,7 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 	if (tpm_dev_add_device(chip))
 		goto put_device;
 
-	if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group))
+	if (tpm_sysfs_add_device(chip))
 		goto del_misc;
 
 	if (tpm_add_ppi(&dev->kobj))
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
new file mode 100644
index 0000000..3bcfed0
--- /dev/null
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -0,0 +1,318 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Copyright (C) 2013 Obsidian Reearch Corp
+ * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
+ *
+ * sysfs filesystem inspection interface to the TPM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ */
+#include <linux/device.h>
+#include "tpm.h"
+
+/* XXX for now this helper is duplicated in tpm.c */
+static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
+			    int len, const char *desc)
+{
+	int err;
+
+	len = tpm_transmit(chip, (u8 *) cmd, len);
+	if (len <  0)
+		return len;
+	else if (len < TPM_HEADER_SIZE)
+		return -EFAULT;
+
+	err = be32_to_cpu(cmd->header.out.return_code);
+	if (err != 0 && desc)
+		dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
+
+	return err;
+}
+
+#define	 READ_PUBEK_RESULT_SIZE 314
+#define TPM_ORD_READPUBEK cpu_to_be32(124)
+static struct tpm_input_header tpm_readpubek_header = {
+	.tag = TPM_TAG_RQU_COMMAND,
+	.length = cpu_to_be32(30),
+	.ordinal = TPM_ORD_READPUBEK
+};
+static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	u8 *data;
+	struct tpm_cmd_t tpm_cmd;
+	ssize_t err;
+	int i, rc;
+	char *str = buf;
+
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	tpm_cmd.header.in = tpm_readpubek_header;
+	err = transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
+			   "attempting to read the PUBEK");
+	if (err)
+		goto out;
+
+	/*
+	   ignore header 10 bytes
+	   algorithm 32 bits (1 == RSA )
+	   encscheme 16 bits
+	   sigscheme 16 bits
+	   parameters (RSA 12->bytes: keybit, #primes, expbit)
+	   keylenbytes 32 bits
+	   256 byte modulus
+	   ignore checksum 20 bytes
+	 */
+	data = tpm_cmd.params.readpubek_out_buffer;
+	str +=
+	    sprintf(str,
+		    "Algorithm: %02X %02X %02X %02X\n"
+		    "Encscheme: %02X %02X\n"
+		    "Sigscheme: %02X %02X\n"
+		    "Parameters: %02X %02X %02X %02X "
+		    "%02X %02X %02X %02X "
+		    "%02X %02X %02X %02X\n"
+		    "Modulus length: %d\n"
+		    "Modulus:\n",
+		    data[0], data[1], data[2], data[3],
+		    data[4], data[5],
+		    data[6], data[7],
+		    data[12], data[13], data[14], data[15],
+		    data[16], data[17], data[18], data[19],
+		    data[20], data[21], data[22], data[23],
+		    be32_to_cpu(*((__be32 *) (data + 24))));
+
+	for (i = 0; i < 256; i++) {
+		str += sprintf(str, "%02X ", data[i + 28]);
+		if ((i + 1) % 16 == 0)
+			str += sprintf(str, "\n");
+	}
+out:
+	rc = str - buf;
+	return rc;
+}
+static DEVICE_ATTR_RO(pubek);
+
+static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	cap_t cap;
+	u8 digest[TPM_DIGEST_SIZE];
+	ssize_t rc;
+	int i, j, num_pcrs;
+	char *str = buf;
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	rc = tpm_getcap(dev, TPM_CAP_PROP_PCR, &cap,
+			"attempting to determine the number of PCRS");
+	if (rc)
+		return 0;
+
+	num_pcrs = be32_to_cpu(cap.num_pcrs);
+	for (i = 0; i < num_pcrs; i++) {
+		rc = tpm_pcr_read_dev(chip, i, digest);
+		if (rc)
+			break;
+		str += sprintf(str, "PCR-%02d: ", i);
+		for (j = 0; j < TPM_DIGEST_SIZE; j++)
+			str += sprintf(str, "%02X ", digest[j]);
+		str += sprintf(str, "\n");
+	}
+	return str - buf;
+}
+static DEVICE_ATTR_RO(pcrs);
+
+static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
+		     char *buf)
+{
+	cap_t cap;
+	ssize_t rc;
+
+	rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
+			 "attempting to determine the permanent enabled state");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", !cap.perm_flags.disable);
+	return rc;
+}
+static DEVICE_ATTR_RO(enabled);
+
+ssize_t active_show(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	cap_t cap;
+	ssize_t rc;
+
+	rc = tpm_getcap(dev, TPM_CAP_FLAG_PERM, &cap,
+			 "attempting to determine the permanent active state");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated);
+	return rc;
+}
+static DEVICE_ATTR_RO(active);
+
+static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	cap_t cap;
+	ssize_t rc;
+
+	rc = tpm_getcap(dev, TPM_CAP_PROP_OWNER, &cap,
+			 "attempting to determine the owner state");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", cap.owned);
+	return rc;
+}
+static DEVICE_ATTR_RO(owned);
+
+static ssize_t temp_deactivated_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	cap_t cap;
+	ssize_t rc;
+
+	rc = tpm_getcap(dev, TPM_CAP_FLAG_VOL, &cap,
+			 "attempting to determine the temporary state");
+	if (rc)
+		return 0;
+
+	rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated);
+	return rc;
+}
+static DEVICE_ATTR_RO(temp_deactivated);
+
+static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	cap_t cap;
+	ssize_t rc;
+	char *str = buf;
+
+	rc = tpm_getcap(dev, TPM_CAP_PROP_MANUFACTURER, &cap,
+			"attempting to determine the manufacturer");
+	if (rc)
+		return 0;
+	str += sprintf(str, "Manufacturer: 0x%x\n",
+		       be32_to_cpu(cap.manufacturer_id));
+
+	/* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
+	rc = tpm_getcap(dev, CAP_VERSION_1_2, &cap,
+			 "attempting to determine the 1.2 version");
+	if (!rc) {
+		str += sprintf(str,
+			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
+			       cap.tpm_version_1_2.Major,
+			       cap.tpm_version_1_2.Minor,
+			       cap.tpm_version_1_2.revMajor,
+			       cap.tpm_version_1_2.revMinor);
+	} else {
+		/* Otherwise just use TPM_STRUCT_VER */
+		rc = tpm_getcap(dev, CAP_VERSION_1_1, &cap,
+				"attempting to determine the 1.1 version");
+		if (rc)
+			return 0;
+		str += sprintf(str,
+			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
+			       cap.tpm_version.Major,
+			       cap.tpm_version.Minor,
+			       cap.tpm_version.revMajor,
+			       cap.tpm_version.revMinor);
+	}
+
+	return str - buf;
+}
+static DEVICE_ATTR_RO(caps);
+
+static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+	if (chip == NULL)
+		return 0;
+
+	chip->vendor.cancel(chip);
+	return count;
+}
+static DEVICE_ATTR_WO(cancel);
+
+static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	if (chip->vendor.duration[TPM_LONG] == 0)
+		return 0;
+
+	return sprintf(buf, "%d %d %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
+		       jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
+		       chip->vendor.duration_adjusted
+		       ? "adjusted" : "original");
+}
+static DEVICE_ATTR_RO(durations);
+
+static ssize_t timeouts_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct tpm_chip *chip = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d %d %d %d [%s]\n",
+		       jiffies_to_usecs(chip->vendor.timeout_a),
+		       jiffies_to_usecs(chip->vendor.timeout_b),
+		       jiffies_to_usecs(chip->vendor.timeout_c),
+		       jiffies_to_usecs(chip->vendor.timeout_d),
+		       chip->vendor.timeout_adjusted
+		       ? "adjusted" : "original");
+}
+static DEVICE_ATTR_RO(timeouts);
+
+static struct attribute *tpm_dev_attrs[] = {
+	&dev_attr_pubek.attr,
+	&dev_attr_pcrs.attr,
+	&dev_attr_enabled.attr,
+	&dev_attr_active.attr,
+	&dev_attr_owned.attr,
+	&dev_attr_temp_deactivated.attr,
+	&dev_attr_caps.attr,
+	&dev_attr_cancel.attr,
+	&dev_attr_durations.attr,
+	&dev_attr_timeouts.attr,
+	NULL,
+};
+
+static const struct attribute_group tpm_dev_group = {
+	.attrs = tpm_dev_attrs,
+};
+
+int tpm_sysfs_add_device(struct tpm_chip *chip)
+{
+	int err;
+	err = sysfs_create_group(&chip->dev->kobj,
+				 &tpm_dev_group);
+
+	if (err)
+		dev_err(chip->dev,
+			"failed to create sysfs attributes, %d\n", err);
+	return err;
+}
+
+void tpm_sysfs_del_device(struct tpm_chip *chip)
+{
+	sysfs_remove_group(&chip->dev->kobj, &tpm_dev_group);
+}
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 496228c..14ba162 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -46,6 +46,14 @@ enum tpm_addr {
 	TPM_ADDR = 0x4E,
 };
 
+/* Indexes the duration array */
+enum tpm_duration {
+	TPM_SHORT = 0,
+	TPM_MEDIUM = 1,
+	TPM_LONG = 2,
+	TPM_UNDEFINED,
+};
+
 #define TPM_WARN_RETRY          0x800
 #define TPM_WARN_DOING_SELFTEST 0x802
 #define TPM_ERR_DEACTIVATED     0x6
@@ -53,27 +61,6 @@ enum tpm_addr {
 #define TPM_ERR_INVALID_POSTINIT 38
 
 #define TPM_HEADER_SIZE		10
-extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_show_pcrs(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_show_caps(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_store_cancel(struct device *, struct device_attribute *attr,
-				const char *, size_t);
-extern ssize_t tpm_show_enabled(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_show_active(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_show_owned(struct device *, struct device_attribute *attr,
-				char *);
-extern ssize_t tpm_show_temp_deactivated(struct device *,
-					 struct device_attribute *attr, char *);
-extern ssize_t tpm_show_durations(struct device *,
-				  struct device_attribute *attr, char *);
-extern ssize_t tpm_show_timeouts(struct device *,
-				 struct device_attribute *attr, char *);
-
 struct tpm_chip;
 
 struct tpm_vendor_specific {
@@ -95,7 +82,6 @@ struct tpm_vendor_specific {
 	u8 (*status) (struct tpm_chip *);
 	void (*release) (struct device *);
 	struct miscdevice miscdev;
-	struct attribute_group *attr_group;
 	struct list_head list;
 	int locality;
 	unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* jiffies */
@@ -171,6 +157,8 @@ struct tpm_output_header {
 	__be32	return_code;
 } __packed;
 
+#define TPM_TAG_RQU_COMMAND cpu_to_be16(193)
+
 struct	stclear_flags_t {
 	__be16	tag;
 	u8	deactivated;
@@ -244,6 +232,24 @@ typedef union {
 	struct duration_t duration;
 } cap_t;
 
+enum tpm_capabilities {
+	TPM_CAP_FLAG = cpu_to_be32(4),
+	TPM_CAP_PROP = cpu_to_be32(5),
+	CAP_VERSION_1_1 = cpu_to_be32(0x06),
+	CAP_VERSION_1_2 = cpu_to_be32(0x1A)
+};
+
+enum tpm_sub_capabilities {
+	TPM_CAP_PROP_PCR = cpu_to_be32(0x101),
+	TPM_CAP_PROP_MANUFACTURER = cpu_to_be32(0x103),
+	TPM_CAP_FLAG_PERM = cpu_to_be32(0x108),
+	TPM_CAP_FLAG_VOL = cpu_to_be32(0x109),
+	TPM_CAP_PROP_OWNER = cpu_to_be32(0x111),
+	TPM_CAP_PROP_TIS_TIMEOUT = cpu_to_be32(0x115),
+	TPM_CAP_PROP_TIS_DURATION = cpu_to_be32(0x120),
+
+};
+
 struct	tpm_getcap_params_in {
 	__be32	cap;
 	__be32	subcap_size;
@@ -340,6 +346,10 @@ extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
 
 int tpm_dev_add_device(struct tpm_chip *chip);
 void tpm_dev_del_device(struct tpm_chip *chip);
+int tpm_sysfs_add_device(struct tpm_chip *chip);
+void tpm_sysfs_del_device(struct tpm_chip *chip);
+
+int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 
 #ifdef CONFIG_ACPI
 extern int tpm_add_ppi(struct kobject *);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 9692e2f..7e665d0 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -121,21 +121,6 @@ static bool tpm_atml_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == ATML_STATUS_READY);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR |S_IWGRP, NULL, tpm_store_cancel);
-
-static struct attribute* atmel_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	NULL,
-};
-
-static struct attribute_group atmel_attr_grp = { .attrs = atmel_attrs };
-
 static const struct tpm_vendor_specific tpm_atmel = {
 	.recv = tpm_atml_recv,
 	.send = tpm_atml_send,
@@ -144,7 +129,6 @@ static const struct tpm_vendor_specific tpm_atmel = {
 	.req_complete_mask = ATML_STATUS_BUSY | ATML_STATUS_DATA_AVAIL,
 	.req_complete_val = ATML_STATUS_DATA_AVAIL,
 	.req_canceled = tpm_atml_req_canceled,
-	.attr_group = &atmel_attr_grp,
 };
 
 static struct platform_device *pdev;
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 079c19d..fe8bdce 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -135,35 +135,6 @@ static u8 i2c_atmel_read_status(struct tpm_chip *chip)
 	return ATMEL_STS_OK;
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *i2c_atmel_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
-	NULL,
-};
-
-static struct attribute_group i2c_atmel_attr_grp = {
-	.attrs = i2c_atmel_attrs
-};
-
 static bool i2c_atmel_req_canceled(struct tpm_chip *chip, u8 status)
 {
 	return false;
@@ -177,7 +148,6 @@ static const struct tpm_vendor_specific i2c_atmel = {
 	.req_complete_mask = ATMEL_STS_OK,
 	.req_complete_val = ATMEL_STS_OK,
 	.req_canceled = i2c_atmel_req_canceled,
-	.attr_group = &i2c_atmel_attr_grp,
 };
 
 static int i2c_atmel_probe(struct i2c_client *client,
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index c1ba7fa..ac1218f 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -566,35 +566,6 @@ static bool tpm_tis_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *tis_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
-	NULL,
-};
-
-static struct attribute_group tis_attr_grp = {
-	.attrs = tis_attrs
-};
-
 static struct tpm_vendor_specific tpm_tis_i2c = {
 	.status = tpm_tis_i2c_status,
 	.recv = tpm_tis_i2c_recv,
@@ -603,7 +574,6 @@ static struct tpm_vendor_specific tpm_tis_i2c = {
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_i2c_req_canceled,
-	.attr_group = &tis_attr_grp,
 };
 
 static int tpm_tis_i2c_init(struct device *dev)
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 3766fe7..5cb9670 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -456,35 +456,6 @@ static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *i2c_nuvoton_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
-	NULL,
-};
-
-static struct attribute_group i2c_nuvoton_attr_grp = {
-	.attrs = i2c_nuvoton_attrs
-};
-
 static const struct tpm_vendor_specific tpm_i2c = {
 	.status = i2c_nuvoton_read_status,
 	.recv = i2c_nuvoton_recv,
@@ -493,7 +464,6 @@ static const struct tpm_vendor_specific tpm_i2c = {
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = i2c_nuvoton_req_canceled,
-	.attr_group = &i2c_nuvoton_attr_grp,
 };
 
 /* The only purpose for the handler is to signal to any waiting threads that
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index e519e68..6902f7b 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -574,30 +574,6 @@ static bool tpm_st33_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-
-static struct attribute *stm_tpm_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr, NULL,
-};
-
-static struct attribute_group stm_tpm_attr_grp = {
-	.attrs = stm_tpm_attrs
-};
-
 static struct tpm_vendor_specific st_i2c_tpm = {
 	.send = tpm_stm_i2c_send,
 	.recv = tpm_stm_i2c_recv,
@@ -606,7 +582,6 @@ static struct tpm_vendor_specific st_i2c_tpm = {
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_st33_i2c_req_canceled,
-	.attr_group = &stm_tpm_attr_grp,
 };
 
 static int interrupts;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 14af0c1..1300e38 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -404,33 +404,6 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == 0);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated,
-		   NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *ibmvtpm_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr, NULL,
-};
-
-static struct attribute_group ibmvtpm_attr_grp = { .attrs = ibmvtpm_attrs };
-
 static const struct tpm_vendor_specific tpm_ibmvtpm = {
 	.recv = tpm_ibmvtpm_recv,
 	.send = tpm_ibmvtpm_send,
@@ -439,7 +412,6 @@ static const struct tpm_vendor_specific tpm_ibmvtpm = {
 	.req_complete_mask = 0,
 	.req_complete_val = 0,
 	.req_canceled = tpm_ibmvtpm_req_canceled,
-	.attr_group = &ibmvtpm_attr_grp,
 };
 
 static const struct dev_pm_ops tpm_ibmvtpm_pm_ops = {
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index c75c10c..9525be5 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -371,21 +371,6 @@ static u8 tpm_inf_status(struct tpm_chip *chip)
 	return tpm_data_in(STAT);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-
-static struct attribute *inf_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	NULL,
-};
-
-static struct attribute_group inf_attr_grp = {.attrs = inf_attrs };
-
 static const struct tpm_vendor_specific tpm_inf = {
 	.recv = tpm_inf_recv,
 	.send = tpm_inf_send,
@@ -393,7 +378,6 @@ static const struct tpm_vendor_specific tpm_inf = {
 	.status = tpm_inf_status,
 	.req_complete_mask = 0,
 	.req_complete_val = 0,
-	.attr_group = &inf_attr_grp,
 };
 
 static const struct pnp_device_id tpm_inf_pnp_tbl[] = {
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index a4acac9..ad980be 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -232,21 +232,6 @@ static bool tpm_nsc_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == NSC_STATUS_RDY);
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR|S_IWGRP, NULL, tpm_store_cancel);
-
-static struct attribute * nsc_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	NULL,
-};
-
-static struct attribute_group nsc_attr_grp = { .attrs = nsc_attrs };
-
 static const struct tpm_vendor_specific tpm_nsc = {
 	.recv = tpm_nsc_recv,
 	.send = tpm_nsc_send,
@@ -255,7 +240,6 @@ static const struct tpm_vendor_specific tpm_nsc = {
 	.req_complete_mask = NSC_STATUS_OBF,
 	.req_complete_val = NSC_STATUS_OBF,
 	.req_canceled = tpm_nsc_req_canceled,
-	.attr_group = &nsc_attr_grp,
 };
 
 static struct platform_device *pdev = NULL;
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index c606ca1..b5d876b 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -432,35 +432,6 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 	}
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated,
-		   NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *tis_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr, NULL,
-};
-
-static struct attribute_group tis_attr_grp = {
-	.attrs = tis_attrs
-};
-
 static struct tpm_vendor_specific tpm_tis = {
 	.status = tpm_tis_status,
 	.recv = tpm_tis_recv,
@@ -469,7 +440,6 @@ static struct tpm_vendor_specific tpm_tis = {
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_req_canceled,
-	.attr_group = &tis_attr_grp,
 };
 
 static irqreturn_t tis_int_probe(int irq, void *dev_id)
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index e3c7c01..d4f6b521 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -143,36 +143,6 @@ static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	return length;
 }
 
-static DEVICE_ATTR(pubek, S_IRUGO, tpm_show_pubek, NULL);
-static DEVICE_ATTR(pcrs, S_IRUGO, tpm_show_pcrs, NULL);
-static DEVICE_ATTR(enabled, S_IRUGO, tpm_show_enabled, NULL);
-static DEVICE_ATTR(active, S_IRUGO, tpm_show_active, NULL);
-static DEVICE_ATTR(owned, S_IRUGO, tpm_show_owned, NULL);
-static DEVICE_ATTR(temp_deactivated, S_IRUGO, tpm_show_temp_deactivated,
-		NULL);
-static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps, NULL);
-static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
-static DEVICE_ATTR(durations, S_IRUGO, tpm_show_durations, NULL);
-static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
-
-static struct attribute *vtpm_attrs[] = {
-	&dev_attr_pubek.attr,
-	&dev_attr_pcrs.attr,
-	&dev_attr_enabled.attr,
-	&dev_attr_active.attr,
-	&dev_attr_owned.attr,
-	&dev_attr_temp_deactivated.attr,
-	&dev_attr_caps.attr,
-	&dev_attr_cancel.attr,
-	&dev_attr_durations.attr,
-	&dev_attr_timeouts.attr,
-	NULL,
-};
-
-static struct attribute_group vtpm_attr_grp = {
-	.attrs = vtpm_attrs,
-};
-
 static const struct tpm_vendor_specific tpm_vtpm = {
 	.status = vtpm_status,
 	.recv = vtpm_recv,
@@ -181,7 +151,6 @@ static const struct tpm_vendor_specific tpm_vtpm = {
 	.req_complete_mask = VTPM_STATUS_IDLE | VTPM_STATUS_RESULT,
 	.req_complete_val  = VTPM_STATUS_IDLE | VTPM_STATUS_RESULT,
 	.req_canceled      = vtpm_req_canceled,
-	.attr_group = &vtpm_attr_grp,
 };
 
 static irqreturn_t tpmif_interrupt(int dummy, void *dev_id)
-- 
1.8.1.2


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

* [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
  2013-11-04  3:38 ` [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c Jason Gunthorpe
  2013-11-04  3:38 ` [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c Jason Gunthorpe
@ 2013-11-04  3:38 ` Jason Gunthorpe
  2013-11-17  0:57   ` [tpmdd-devel] " Ashley Lai
  2013-11-04  3:38 ` [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific Jason Gunthorpe
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

This replaces the static initialization of a tpm_vendor_specific
structure in the drivers with the standard Linux idiom of providing
a const structure of function pointers.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-interface.c    | 10 ++++++++--
 drivers/char/tpm/tpm.h              |  6 +++---
 drivers/char/tpm/tpm_atmel.c        |  2 +-
 drivers/char/tpm/tpm_i2c_atmel.c    |  2 +-
 drivers/char/tpm/tpm_i2c_infineon.c |  2 +-
 drivers/char/tpm/tpm_i2c_nuvoton.c  |  2 +-
 drivers/char/tpm/tpm_i2c_stm_st33.c |  2 +-
 drivers/char/tpm/tpm_ibmvtpm.c      |  2 +-
 drivers/char/tpm/tpm_infineon.c     |  2 +-
 drivers/char/tpm/tpm_nsc.c          |  2 +-
 drivers/char/tpm/tpm_tis.c          |  2 +-
 drivers/char/tpm/xen-tpmfront.c     |  2 +-
 include/linux/tpm.h                 | 12 ++++++++++++
 13 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 5ab3caa..0662b70 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1061,7 +1061,7 @@ EXPORT_SYMBOL_GPL(tpm_dev_release);
  * pci_disable_device
  */
 struct tpm_chip *tpm_register_hardware(struct device *dev,
-					const struct tpm_vendor_specific *entry)
+				       const struct tpm_class_ops *ops)
 {
 	struct tpm_chip *chip;
 
@@ -1074,7 +1074,13 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 	mutex_init(&chip->tpm_mutex);
 	INIT_LIST_HEAD(&chip->list);
 
-	memcpy(&chip->vendor, entry, sizeof(struct tpm_vendor_specific));
+	chip->vendor.req_complete_mask = ops->req_complete_mask;
+	chip->vendor.req_complete_val = ops->req_complete_val;
+	chip->vendor.req_canceled = ops->req_canceled;
+	chip->vendor.recv = ops->recv;
+	chip->vendor.send = ops->send;
+	chip->vendor.cancel = ops->cancel;
+	chip->vendor.status = ops->status;
 
 	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 14ba162..a56af2c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -64,8 +64,8 @@ enum tpm_duration {
 struct tpm_chip;
 
 struct tpm_vendor_specific {
-	const u8 req_complete_mask;
-	const u8 req_complete_val;
+	u8 req_complete_mask;
+	u8 req_complete_val;
 	bool (*req_canceled)(struct tpm_chip *chip, u8 status);
 	void __iomem *iobase;		/* ioremapped address */
 	unsigned long base;		/* TPM base address */
@@ -336,7 +336,7 @@ extern void tpm_gen_interrupt(struct tpm_chip *);
 extern int tpm_do_selftest(struct tpm_chip *);
 extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
 extern struct tpm_chip* tpm_register_hardware(struct device *,
-				 const struct tpm_vendor_specific *);
+					      const struct tpm_class_ops *ops);
 extern void tpm_dev_vendor_release(struct tpm_chip *);
 extern void tpm_remove_hardware(struct device *);
 extern int tpm_pm_suspend(struct device *);
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 7e665d0..6069d13 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -121,7 +121,7 @@ static bool tpm_atml_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == ATML_STATUS_READY);
 }
 
-static const struct tpm_vendor_specific tpm_atmel = {
+static const struct tpm_class_ops tpm_atmel = {
 	.recv = tpm_atml_recv,
 	.send = tpm_atml_send,
 	.cancel = tpm_atml_cancel,
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index fe8bdce..7727292 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -140,7 +140,7 @@ static bool i2c_atmel_req_canceled(struct tpm_chip *chip, u8 status)
 	return false;
 }
 
-static const struct tpm_vendor_specific i2c_atmel = {
+static const struct tpm_class_ops i2c_atmel = {
 	.status = i2c_atmel_read_status,
 	.recv = i2c_atmel_recv,
 	.send = i2c_atmel_send,
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index ac1218f..52b9b2b 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -566,7 +566,7 @@ static bool tpm_tis_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static struct tpm_vendor_specific tpm_tis_i2c = {
+static const struct tpm_class_ops tpm_tis_i2c = {
 	.status = tpm_tis_i2c_status,
 	.recv = tpm_tis_i2c_recv,
 	.send = tpm_tis_i2c_send,
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 5cb9670..ea60aa3 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -456,7 +456,7 @@ static bool i2c_nuvoton_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static const struct tpm_vendor_specific tpm_i2c = {
+static const struct tpm_class_ops tpm_i2c = {
 	.status = i2c_nuvoton_read_status,
 	.recv = i2c_nuvoton_recv,
 	.send = i2c_nuvoton_send,
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index 6902f7b..874b3be 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -574,7 +574,7 @@ static bool tpm_st33_i2c_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == TPM_STS_COMMAND_READY);
 }
 
-static struct tpm_vendor_specific st_i2c_tpm = {
+static const struct tpm_class_ops st_i2c_tpm = {
 	.send = tpm_stm_i2c_send,
 	.recv = tpm_stm_i2c_recv,
 	.cancel = tpm_stm_i2c_cancel,
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 1300e38..3341410 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -404,7 +404,7 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == 0);
 }
 
-static const struct tpm_vendor_specific tpm_ibmvtpm = {
+static const struct tpm_class_ops tpm_ibmvtpm = {
 	.recv = tpm_ibmvtpm_recv,
 	.send = tpm_ibmvtpm_send,
 	.cancel = tpm_ibmvtpm_cancel,
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 9525be5..dc0a255 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -371,7 +371,7 @@ static u8 tpm_inf_status(struct tpm_chip *chip)
 	return tpm_data_in(STAT);
 }
 
-static const struct tpm_vendor_specific tpm_inf = {
+static const struct tpm_class_ops tpm_inf = {
 	.recv = tpm_inf_recv,
 	.send = tpm_inf_send,
 	.cancel = tpm_inf_cancel,
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index ad980be..3179ec9 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -232,7 +232,7 @@ static bool tpm_nsc_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == NSC_STATUS_RDY);
 }
 
-static const struct tpm_vendor_specific tpm_nsc = {
+static const struct tpm_class_ops tpm_nsc = {
 	.recv = tpm_nsc_recv,
 	.send = tpm_nsc_send,
 	.cancel = tpm_nsc_cancel,
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index b5d876b..a9ed227 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -432,7 +432,7 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
 	}
 }
 
-static struct tpm_vendor_specific tpm_tis = {
+static const struct tpm_class_ops tpm_tis = {
 	.status = tpm_tis_status,
 	.recv = tpm_tis_recv,
 	.send = tpm_tis_send,
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index d4f6b521..92b0970 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -143,7 +143,7 @@ static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	return length;
 }
 
-static const struct tpm_vendor_specific tpm_vtpm = {
+static const struct tpm_class_ops tpm_vtpm = {
 	.status = vtpm_status,
 	.recv = vtpm_recv,
 	.send = vtpm_send,
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 9a9051b..fff1d09 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -29,6 +29,18 @@
  */
 #define	TPM_ANY_NUM 0xFFFF
 
+struct tpm_chip;
+
+struct tpm_class_ops {
+	const u8 req_complete_mask;
+	const u8 req_complete_val;
+	bool (*req_canceled)(struct tpm_chip *chip, u8 status);
+	int (*recv) (struct tpm_chip *chip, u8 *buf, size_t len);
+	int (*send) (struct tpm_chip *chip, u8 *buf, size_t len);
+	void (*cancel) (struct tpm_chip *chip);
+	u8 (*status) (struct tpm_chip *chip);
+};
+
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);
-- 
1.8.1.2


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

* [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
                   ` (2 preceding siblings ...)
  2013-11-04  3:38 ` [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers Jason Gunthorpe
@ 2013-11-04  3:38 ` Jason Gunthorpe
  2013-11-17  1:09   ` [tpmdd-devel] " Ashley Lai
  2013-11-04  3:38 ` [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure Jason Gunthorpe
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

This builds on the last commit to use the ops structure in the core
and reduce the size of tpm_vendor_specific.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-interface.c    | 34 ++++++++++++----------------------
 drivers/char/tpm/tpm-sysfs.c        |  2 +-
 drivers/char/tpm/tpm.h              |  9 +--------
 drivers/char/tpm/tpm_i2c_stm_st33.c |  4 ++--
 4 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0662b70..5a2807c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -353,7 +353,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 
 	mutex_lock(&chip->tpm_mutex);
 
-	rc = chip->vendor.send(chip, (u8 *) buf, count);
+	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
 		dev_err(chip->dev,
 			"tpm_transmit: tpm_send: error %zd\n", rc);
@@ -365,12 +365,12 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 
 	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
 	do {
-		u8 status = chip->vendor.status(chip);
-		if ((status & chip->vendor.req_complete_mask) ==
-		    chip->vendor.req_complete_val)
+		u8 status = chip->ops->status(chip);
+		if ((status & chip->ops->req_complete_mask) ==
+		    chip->ops->req_complete_val)
 			goto out_recv;
 
-		if (chip->vendor.req_canceled(chip, status)) {
+		if (chip->ops->req_canceled(chip, status)) {
 			dev_err(chip->dev, "Operation Canceled\n");
 			rc = -ECANCELED;
 			goto out;
@@ -380,13 +380,13 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 		rmb();
 	} while (time_before(jiffies, stop));
 
-	chip->vendor.cancel(chip);
+	chip->ops->cancel(chip);
 	dev_err(chip->dev, "Operation Timed out\n");
 	rc = -ETIME;
 	goto out;
 
 out_recv:
-	rc = chip->vendor.recv(chip, (u8 *) buf, bufsiz);
+	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
 	if (rc < 0)
 		dev_err(chip->dev,
 			"tpm_transmit: tpm_recv: error %zd\n", rc);
@@ -807,12 +807,12 @@ EXPORT_SYMBOL_GPL(tpm_send);
 static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
 				   bool check_cancel, bool *canceled)
 {
-	u8 status = chip->vendor.status(chip);
+	u8 status = chip->ops->status(chip);
 
 	*canceled = false;
 	if ((status & mask) == mask)
 		return true;
-	if (check_cancel && chip->vendor.req_canceled(chip, status)) {
+	if (check_cancel && chip->ops->req_canceled(chip, status)) {
 		*canceled = true;
 		return true;
 	}
@@ -828,7 +828,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
 	bool canceled = false;
 
 	/* check current status */
-	status = chip->vendor.status(chip);
+	status = chip->ops->status(chip);
 	if ((status & mask) == mask)
 		return 0;
 
@@ -855,7 +855,7 @@ again:
 	} else {
 		do {
 			msleep(TPM_TIMEOUT);
-			status = chip->vendor.status(chip);
+			status = chip->ops->status(chip);
 			if ((status & mask) == mask)
 				return 0;
 		} while (time_before(jiffies, stop));
@@ -1027,9 +1027,6 @@ void tpm_dev_vendor_release(struct tpm_chip *chip)
 	if (!chip)
 		return;
 
-	if (chip->vendor.release)
-		chip->vendor.release(chip->dev);
-
 	clear_bit(chip->dev_num, dev_mask);
 }
 EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
@@ -1074,14 +1071,7 @@ struct tpm_chip *tpm_register_hardware(struct device *dev,
 	mutex_init(&chip->tpm_mutex);
 	INIT_LIST_HEAD(&chip->list);
 
-	chip->vendor.req_complete_mask = ops->req_complete_mask;
-	chip->vendor.req_complete_val = ops->req_complete_val;
-	chip->vendor.req_canceled = ops->req_canceled;
-	chip->vendor.recv = ops->recv;
-	chip->vendor.send = ops->send;
-	chip->vendor.cancel = ops->cancel;
-	chip->vendor.status = ops->status;
-
+	chip->ops = ops;
 	chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
 
 	if (chip->dev_num >= TPM_NUM_DEVICES) {
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 3bcfed0..d2a8bca 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -245,7 +245,7 @@ static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
 	if (chip == NULL)
 		return 0;
 
-	chip->vendor.cancel(chip);
+	chip->ops->cancel(chip);
 	return count;
 }
 static DEVICE_ATTR_WO(cancel);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a56af2c..7b0a46e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -64,9 +64,6 @@ enum tpm_duration {
 struct tpm_chip;
 
 struct tpm_vendor_specific {
-	u8 req_complete_mask;
-	u8 req_complete_val;
-	bool (*req_canceled)(struct tpm_chip *chip, u8 status);
 	void __iomem *iobase;		/* ioremapped address */
 	unsigned long base;		/* TPM base address */
 
@@ -76,11 +73,6 @@ struct tpm_vendor_specific {
 	int region_size;
 	int have_region;
 
-	int (*recv) (struct tpm_chip *, u8 *, size_t);
-	int (*send) (struct tpm_chip *, u8 *, size_t);
-	void (*cancel) (struct tpm_chip *);
-	u8 (*status) (struct tpm_chip *);
-	void (*release) (struct device *);
 	struct miscdevice miscdev;
 	struct list_head list;
 	int locality;
@@ -104,6 +96,7 @@ struct tpm_vendor_specific {
 
 struct tpm_chip {
 	struct device *dev;	/* Device stuff */
+	const struct tpm_class_ops *ops;
 
 	int dev_num;		/* /dev/tpm# */
 	char devname[7];
diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
index 874b3be..5b0dd8e 100644
--- a/drivers/char/tpm/tpm_i2c_stm_st33.c
+++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
@@ -564,7 +564,7 @@ static int tpm_stm_i2c_recv(struct tpm_chip *chip, unsigned char *buf,
 	}
 
 out:
-	chip->vendor.cancel(chip);
+	chip->ops->cancel(chip);
 	release_locality(chip);
 	return size;
 }
@@ -807,7 +807,7 @@ static int tpm_st33_i2c_pm_resume(struct device *dev)
 	if (power_mgt) {
 		gpio_set_value(pin_infos->io_lpcpd, 1);
 		ret = wait_for_serirq_timeout(chip,
-					  (chip->vendor.status(chip) &
+					  (chip->ops->status(chip) &
 					  TPM_STS_VALID) == TPM_STS_VALID,
 					  chip->vendor.timeout_b);
 	} else {
-- 
1.8.1.2


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

* [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
                   ` (3 preceding siblings ...)
  2013-11-04  3:38 ` [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific Jason Gunthorpe
@ 2013-11-04  3:38 ` Jason Gunthorpe
  2013-11-17  2:01   ` [tpmdd-devel] " Ashley Lai
  2013-11-08 16:54 ` [tpmdd-devel] tpm: Remainder of earlier clean up Ashley Lai
  2013-12-07 13:21 ` Peter Hüwe
  6 siblings, 1 reply; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-04  3:38 UTC (permalink / raw)
  To: tpmdd-devel, Peter Huewe; +Cc: linux-kernel, Ashley Lai

This consolidates everything that is only used within tpm-dev.c
into tpm-dev.c and out of the publicly visible struct tpm_chip.

The per-file allocation lays the ground work for someday fixing the
strange forced O_EXCL behaviour of the current code.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm-dev.c | 100 ++++++++++++++++++++++++++-------------------
 drivers/char/tpm/tpm.h     |   7 ----
 2 files changed, 57 insertions(+), 50 deletions(-)

diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 8d94e97..f492eb6 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -22,21 +22,34 @@
 #include <linux/uaccess.h>
 #include "tpm.h"
 
+struct file_priv {
+	struct tpm_chip *chip;
+
+	/* Data passed to and from the tpm via the read/write calls */
+	atomic_t data_pending;
+	struct mutex buffer_mutex;
+
+	struct timer_list user_read_timer;      /* user needs to claim result */
+	struct work_struct work;
+
+	u8 data_buffer[TPM_BUFSIZE];
+};
+
 static void user_reader_timeout(unsigned long ptr)
 {
-	struct tpm_chip *chip = (struct tpm_chip *) ptr;
+	struct file_priv *priv = (struct file_priv *)ptr;
 
-	schedule_work(&chip->work);
+	schedule_work(&priv->work);
 }
 
 static void timeout_work(struct work_struct *work)
 {
-	struct tpm_chip *chip = container_of(work, struct tpm_chip, work);
+	struct file_priv *priv = container_of(work, struct file_priv, work);
 
-	mutex_lock(&chip->buffer_mutex);
-	atomic_set(&chip->data_pending, 0);
-	memset(chip->data_buffer, 0, TPM_BUFSIZE);
-	mutex_unlock(&chip->buffer_mutex);
+	mutex_lock(&priv->buffer_mutex);
+	atomic_set(&priv->data_pending, 0);
+	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
+	mutex_unlock(&priv->buffer_mutex);
 }
 
 static int tpm_open(struct inode *inode, struct file *file)
@@ -44,6 +57,7 @@ static int tpm_open(struct inode *inode, struct file *file)
 	struct miscdevice *misc = file->private_data;
 	struct tpm_chip *chip = container_of(misc, struct tpm_chip,
 					     vendor.miscdev);
+	struct file_priv *priv;
 
 	/* It's assured that the chip will be opened just once,
 	 * by the check of is_open variable, which is protected
@@ -53,15 +67,20 @@ static int tpm_open(struct inode *inode, struct file *file)
 		return -EBUSY;
 	}
 
-	chip->data_buffer = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
-	if (chip->data_buffer == NULL) {
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv == NULL) {
 		clear_bit(0, &chip->is_open);
 		return -ENOMEM;
 	}
 
-	atomic_set(&chip->data_pending, 0);
+	priv->chip = chip;
+	atomic_set(&priv->data_pending, 0);
+	mutex_init(&priv->buffer_mutex);
+	setup_timer(&priv->user_read_timer, user_reader_timeout,
+			(unsigned long)priv);
+	INIT_WORK(&priv->work, timeout_work);
 
-	file->private_data = chip;
+	file->private_data = priv;
 	get_device(chip->dev);
 	return 0;
 }
@@ -69,28 +88,28 @@ static int tpm_open(struct inode *inode, struct file *file)
 static ssize_t tpm_read(struct file *file, char __user *buf,
 			size_t size, loff_t *off)
 {
-	struct tpm_chip *chip = file->private_data;
+	struct file_priv *priv = file->private_data;
 	ssize_t ret_size;
 	int rc;
 
-	del_singleshot_timer_sync(&chip->user_read_timer);
-	flush_work(&chip->work);
-	ret_size = atomic_read(&chip->data_pending);
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	ret_size = atomic_read(&priv->data_pending);
 	if (ret_size > 0) {	/* relay data */
 		ssize_t orig_ret_size = ret_size;
 		if (size < ret_size)
 			ret_size = size;
 
-		mutex_lock(&chip->buffer_mutex);
-		rc = copy_to_user(buf, chip->data_buffer, ret_size);
-		memset(chip->data_buffer, 0, orig_ret_size);
+		mutex_lock(&priv->buffer_mutex);
+		rc = copy_to_user(buf, priv->data_buffer, ret_size);
+		memset(priv->data_buffer, 0, orig_ret_size);
 		if (rc)
 			ret_size = -EFAULT;
 
-		mutex_unlock(&chip->buffer_mutex);
+		mutex_unlock(&priv->buffer_mutex);
 	}
 
-	atomic_set(&chip->data_pending, 0);
+	atomic_set(&priv->data_pending, 0);
 
 	return ret_size;
 }
@@ -98,7 +117,7 @@ static ssize_t tpm_read(struct file *file, char __user *buf,
 static ssize_t tpm_write(struct file *file, const char __user *buf,
 			 size_t size, loff_t *off)
 {
-	struct tpm_chip *chip = file->private_data;
+	struct file_priv *priv = file->private_data;
 	size_t in_size = size;
 	ssize_t out_size;
 
@@ -106,32 +125,33 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 	   either via tpm_read or a user_read_timer timeout.
 	   This also prevents splitted buffered writes from blocking here.
 	*/
-	if (atomic_read(&chip->data_pending) != 0)
+	if (atomic_read(&priv->data_pending) != 0)
 		return -EBUSY;
 
 	if (in_size > TPM_BUFSIZE)
 		return -E2BIG;
 
-	mutex_lock(&chip->buffer_mutex);
+	mutex_lock(&priv->buffer_mutex);
 
 	if (copy_from_user
-	    (chip->data_buffer, (void __user *) buf, in_size)) {
-		mutex_unlock(&chip->buffer_mutex);
+	    (priv->data_buffer, (void __user *) buf, in_size)) {
+		mutex_unlock(&priv->buffer_mutex);
 		return -EFAULT;
 	}
 
 	/* atomic tpm command send and result receive */
-	out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
+	out_size = tpm_transmit(priv->chip, priv->data_buffer,
+				sizeof(priv->data_buffer));
 	if (out_size < 0) {
-		mutex_unlock(&chip->buffer_mutex);
+		mutex_unlock(&priv->buffer_mutex);
 		return out_size;
 	}
 
-	atomic_set(&chip->data_pending, out_size);
-	mutex_unlock(&chip->buffer_mutex);
+	atomic_set(&priv->data_pending, out_size);
+	mutex_unlock(&priv->buffer_mutex);
 
 	/* Set a timeout by which the reader must come claim the result */
-	mod_timer(&chip->user_read_timer, jiffies + (60 * HZ));
+	mod_timer(&priv->user_read_timer, jiffies + (60 * HZ));
 
 	return in_size;
 }
@@ -141,15 +161,15 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
  */
 static int tpm_release(struct inode *inode, struct file *file)
 {
-	struct tpm_chip *chip = file->private_data;
+	struct file_priv *priv = file->private_data;
 
-	del_singleshot_timer_sync(&chip->user_read_timer);
-	flush_work(&chip->work);
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
 	file->private_data = NULL;
-	atomic_set(&chip->data_pending, 0);
-	kzfree(chip->data_buffer);
-	clear_bit(0, &chip->is_open);
-	put_device(chip->dev);
+	atomic_set(&priv->data_pending, 0);
+	clear_bit(0, &priv->chip->is_open);
+	put_device(priv->chip->dev);
+	kfree(priv);
 	return 0;
 }
 
@@ -166,12 +186,6 @@ int tpm_dev_add_device(struct tpm_chip *chip)
 {
 	int rc;
 
-	mutex_init(&chip->buffer_mutex);
-	INIT_WORK(&chip->work, timeout_work);
-
-	setup_timer(&chip->user_read_timer, user_reader_timeout,
-			(unsigned long)chip);
-
 	chip->vendor.miscdev.fops = &tpm_fops;
 	if (chip->dev_num == 0)
 		chip->vendor.miscdev.minor = TPM_MINOR;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7b0a46e..e4d0888 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -103,13 +103,6 @@ struct tpm_chip {
 	unsigned long is_open;	/* only one allowed */
 	int time_expired;
 
-	/* Data passed to and from the tpm via the read/write calls */
-	u8 *data_buffer;
-	atomic_t data_pending;
-	struct mutex buffer_mutex;
-
-	struct timer_list user_read_timer;	/* user needs to claim result */
-	struct work_struct work;
 	struct mutex tpm_mutex;	/* tpm is processing */
 
 	struct tpm_vendor_specific vendor;
-- 
1.8.1.2


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

* Re: [tpmdd-devel] tpm: Remainder of earlier clean up
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
                   ` (4 preceding siblings ...)
  2013-11-04  3:38 ` [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure Jason Gunthorpe
@ 2013-11-08 16:54 ` Ashley Lai
  2013-11-08 17:23   ` Aw: " Peter Huewe
  2013-12-07 13:21 ` Peter Hüwe
  6 siblings, 1 reply; 17+ messages in thread
From: Ashley Lai @ 2013-11-08 16:54 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

Hi Jason,
 Just want to let you know that I'm not forgetting about reviewing these
patches.  It is on my to do list, I will get to it by this or next
weekend.

--Ashley Lai

On Sun, 2013-11-03 at 20:38 -0700, Jason Gunthorpe wrote:
> Here is the last five patches from the prior series I sent that didn't get
> picked up yet.
> 
> There are no changes, these are just rebased onto 3.12rc7 + Peter's for-james
> branch. (Peter: Note, there are TPM changes to the Xen driver in 3.12rc7 that
> are not in your for-james branch)
> 
> The intent of these patches is to reduce the duplicated code that is present
> in all the drivers by migrating it into the core.
> 
> I've placed the patches on my github:
>  https://github.com/jgunthorpe/linux/commits/for-tpm
> 
>  drivers/char/tpm/Makefile           |    2 +-
>  drivers/char/tpm/tpm-dev.c          |  213 +++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm-interface.c    |  487 +++++-------------------------------------------------------------------------------
>  drivers/char/tpm/tpm-sysfs.c        |  318 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm.h              |   83 +++++++--------
>  drivers/char/tpm/tpm_atmel.c        |   28 +----
>  drivers/char/tpm/tpm_i2c_atmel.c    |   42 +-------
>  drivers/char/tpm/tpm_i2c_infineon.c |   42 +-------
>  drivers/char/tpm/tpm_i2c_nuvoton.c  |   42 +-------
>  drivers/char/tpm/tpm_i2c_stm_st33.c |   41 +------
>  drivers/char/tpm/tpm_ibmvtpm.c      |   40 +------
>  drivers/char/tpm/tpm_infineon.c     |   28 +----
>  drivers/char/tpm/tpm_nsc.c          |   28 +----
>  drivers/char/tpm/tpm_tis.c          |   49 +--------
>  drivers/char/tpm/xen-tpmfront.c     |   45 +-------
>  include/linux/tpm.h                 |   12 +++
>  16 files changed, 624 insertions(+), 876 deletions(-)
> 
> 
> ------------------------------------------------------------------------------
> Android is increasing in popularity, but the open development platform that
> developers love is also attractive to malware creators. Download this white
> paper to learn more about secure code signing practices that can help keep
> Android apps secure.
> http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
> 



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

* Aw: Re: [tpmdd-devel] tpm: Remainder of earlier clean up
  2013-11-08 16:54 ` [tpmdd-devel] tpm: Remainder of earlier clean up Ashley Lai
@ 2013-11-08 17:23   ` Peter Huewe
  0 siblings, 0 replies; 17+ messages in thread
From: Peter Huewe @ 2013-11-08 17:23 UTC (permalink / raw)
  To: Ashley Lai; +Cc: Jason Gunthorpe, tpmdd-devel, Peter Huewe, linux-kernel


Hi Jason, Ashley
> Just want to let you know that I'm not forgetting about reviewing these
> patches. It is on my to do list, I will get to it by this or next
> weekend.

I'm also on it ;)
But of course it's quite a lot

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

* Re: [tpmdd-devel] [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c
  2013-11-04  3:38 ` [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c Jason Gunthorpe
@ 2013-11-16 22:18   ` Ashley Lai
  2013-11-17  0:12     ` Jason Gunthorpe
  0 siblings, 1 reply; 17+ messages in thread
From: Ashley Lai @ 2013-11-16 22:18 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

Nice clean up!!! The code looks much more clean after removing the
duplicated code in all drivers.  Thanks.


> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> new file mode 100644
> index 0000000..8d94e97
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-dev.c
> + * Copyright (C) 2013 Obsidian Reearch Corp

Typo Research?

> + * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> + *

> +
> +int tpm_dev_add_device(struct tpm_chip *chip)
> +{

> +	rc = misc_register(&chip->vendor.miscdev);
> +	if (rc) {
> +		chip->vendor.miscdev.name = NULL;
> +		dev_warn(chip->dev,

Any reason why we use dev_warn here instead of dev_err?

> +			"unable to misc_register %s, minor %d err=%d\n",
> +			 chip->vendor.miscdev.name,
> +			 chip->vendor.miscdev.minor, rc);
> +	}
> +	return rc;
> +}
> +

Reviewed-by: Ashley Lai <adlai@linux.vnet.ibm.com>

Regards,
--Ashley Lai


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

* Re: [tpmdd-devel] [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c
  2013-11-04  3:38 ` [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c Jason Gunthorpe
@ 2013-11-16 23:53   ` Ashley Lai
  2013-11-17  0:24     ` Jason Gunthorpe
  0 siblings, 1 reply; 17+ messages in thread
From: Ashley Lai @ 2013-11-16 23:53 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel


> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> new file mode 100644
> index 0000000..3bcfed0
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -0,0 +1,318 @@
> + * Copyright (C) 2013 Obsidian Reearch Corp
Typo Research?
> + * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> + *

> +#include "tpm.h"
> +
> +/* XXX for now this helper is duplicated in tpm.c */

I think you mean this is duplicated in tpm-interface.c.  Is there a
reason why we cannot add this to tpm.h to avoid this duplication?

> +static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> +			    int len, const char *desc)
> +{


> +static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)

Most of the functions in this file are moved from tpm-interface.c
without any modification to the code.  Why do we need to change the
function names in this file?  Unless there is a good reason for it
otherwise I would prefer to keep the same function names.  This is for
easy maintaining (for me at least :)) in case there are issues in the
future and we need to go back we can easily find out where they came
from.

Thanks,
--Ashley Lai


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

* Re: [tpmdd-devel] [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c
  2013-11-16 22:18   ` [tpmdd-devel] " Ashley Lai
@ 2013-11-17  0:12     ` Jason Gunthorpe
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-17  0:12 UTC (permalink / raw)
  To: Ashley Lai; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

On Sat, Nov 16, 2013 at 04:18:55PM -0600, Ashley Lai wrote:
> Nice clean up!!! The code looks much more clean after removing the
> duplicated code in all drivers.  Thanks.

> > +++ b/drivers/char/tpm/tpm-dev.c
> > + * Copyright (C) 2013 Obsidian Reearch Corp
> 
> Typo Research?

Yes, thanks :)
 
> > + * Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > + *
> 
> > +
> > +int tpm_dev_add_device(struct tpm_chip *chip)
> > +{
> 
> > +	rc = misc_register(&chip->vendor.miscdev);
> > +	if (rc) {
> > +		chip->vendor.miscdev.name = NULL;
> > +		dev_warn(chip->dev,
> 
> Any reason why we use dev_warn here instead of dev_err?

Indeed, it was dev_err before I moved it. 

Hmm, it has been so long.. I think an earlier draft had
tpm_dev_add_device return void (like in other subsystems), so warn
made more sense. However I eventually changed it to propogate errors,
and didn't catch the flip.

Lets put it back.

Thanks!
Jason

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

* Re: [tpmdd-devel] [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c
  2013-11-16 23:53   ` [tpmdd-devel] " Ashley Lai
@ 2013-11-17  0:24     ` Jason Gunthorpe
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2013-11-17  0:24 UTC (permalink / raw)
  To: Ashley Lai; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

On Sat, Nov 16, 2013 at 05:53:19PM -0600, Ashley Lai wrote:

> > +/* XXX for now this helper is duplicated in tpm.c */
> 
> I think you mean this is duplicated in tpm-interface.c.  Is there a
> reason why we cannot add this to tpm.h to avoid this duplication?

Indeed, it was called tpm.c when I wrote it :)

I left it because I think there should be a public API to send a
command that makes sense, and transmit_cmd is not good. But, that can
come later, so lets share them for now.

> > +static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> > +			    int len, const char *desc)
> > +{
> 
> 
> > +static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
> > +			  char *buf)
> 
> Most of the functions in this file are moved from tpm-interface.c
> without any modification to the code.  Why do we need to change the
> function names in this file?  Unless there is a good reason for it
> otherwise I would prefer to keep the same function names.  

When I did the sysfs attribute lists in this file I used the modern
sysfs macros:

>> +static DEVICE_ATTR_RO(pubek);

Which are safer to use, but do require the function name to match
the sysfs file name.

So all sysfs functions must be in the form <FILENAME>_show.

Jason

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

* Re: [tpmdd-devel] [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers
  2013-11-04  3:38 ` [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers Jason Gunthorpe
@ 2013-11-17  0:57   ` Ashley Lai
  0 siblings, 0 replies; 17+ messages in thread
From: Ashley Lai @ 2013-11-17  0:57 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

On Sun, 2013-11-03 at 20:38 -0700, Jason Gunthorpe wrote:
> This replaces the static initialization of a tpm_vendor_specific
> structure in the drivers with the standard Linux idiom of providing
> a const structure of function pointers.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>

Reviewed-by: Ashley Lai <adlai@linux.vnet.ibm.com>
Acked-by: Ashley Lai <adlai@linux.vnet.ibm.com>

Thanks,
--Ashley Lai


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

* Re: [tpmdd-devel] [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific
  2013-11-04  3:38 ` [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific Jason Gunthorpe
@ 2013-11-17  1:09   ` Ashley Lai
  0 siblings, 0 replies; 17+ messages in thread
From: Ashley Lai @ 2013-11-17  1:09 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

On Sun, 2013-11-03 at 20:38 -0700, Jason Gunthorpe wrote:
> This builds on the last commit to use the ops structure in the core
> and reduce the size of tpm_vendor_specific.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Reviewed-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> ---

Reviewed-by: Ashley Lai <adlai@linux.vnet.ibm.com>
Acked-by: Ashley Lai <adlai@linux.vnet.ibm.com>

Thanks,
--Ashley Lai


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

* Re: [tpmdd-devel] [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure
  2013-11-04  3:38 ` [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure Jason Gunthorpe
@ 2013-11-17  2:01   ` Ashley Lai
  0 siblings, 0 replies; 17+ messages in thread
From: Ashley Lai @ 2013-11-17  2:01 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, Peter Huewe, linux-kernel

On Sun, 2013-11-03 at 20:38 -0700, Jason Gunthorpe wrote:
> This consolidates everything that is only used within tpm-dev.c
> into tpm-dev.c and out of the publicly visible struct tpm_chip.
> 
> The per-file allocation lays the ground work for someday fixing the
> strange forced O_EXCL behaviour of the current code.

Reviewed-by: Ashley Lai <adlai@linux.vnet.ibm.com>
Acked-by: Ashley Lai <adlai@linux.vnet.ibm.com>

Thanks,
--Ashley Lai


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

* Re: tpm: Remainder of earlier clean up
  2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
                   ` (5 preceding siblings ...)
  2013-11-08 16:54 ` [tpmdd-devel] tpm: Remainder of earlier clean up Ashley Lai
@ 2013-12-07 13:21 ` Peter Hüwe
  2013-12-10  1:33   ` Jason Gunthorpe
  6 siblings, 1 reply; 17+ messages in thread
From: Peter Hüwe @ 2013-12-07 13:21 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: tpmdd-devel, linux-kernel, Ashley Lai

Hi Jason

did push everything to 
	https://github.com/PeterHuewe/linux-tpmdd/tree/testing-and-review
against latest rc, with alls signed-offs and reviewed-bys

I'll move it to for-james after giving it a final test run.

Thanks,
Peter

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

* Re: tpm: Remainder of earlier clean up
  2013-12-07 13:21 ` Peter Hüwe
@ 2013-12-10  1:33   ` Jason Gunthorpe
  0 siblings, 0 replies; 17+ messages in thread
From: Jason Gunthorpe @ 2013-12-10  1:33 UTC (permalink / raw)
  To: Peter Hüwe; +Cc: tpmdd-devel, linux-kernel, Ashley Lai

On Sat, Dec 07, 2013 at 02:21:43PM +0100, Peter Hüwe wrote:
> Hi Jason
> 
> did push everything to 
> 	https://github.com/PeterHuewe/linux-tpmdd/tree/testing-and-review
> against latest rc, with alls signed-offs and reviewed-bys
> 
> I'll move it to for-james after giving it a final test run.

Thanks!

Jason

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

end of thread, other threads:[~2013-12-10  1:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-04  3:38 tpm: Remainder of earlier clean up Jason Gunthorpe
2013-11-04  3:38 ` [PATCH 1/5 v2] tpm: Pull everything related to /dev/tpmX into tpm-dev.c Jason Gunthorpe
2013-11-16 22:18   ` [tpmdd-devel] " Ashley Lai
2013-11-17  0:12     ` Jason Gunthorpe
2013-11-04  3:38 ` [PATCH 2/5 v2] tpm: Pull everything related to sysfs into tpm-sysfs.c Jason Gunthorpe
2013-11-16 23:53   ` [tpmdd-devel] " Ashley Lai
2013-11-17  0:24     ` Jason Gunthorpe
2013-11-04  3:38 ` [PATCH 3/5 v2] tpm: Create a tpm_class_ops structure and use it in the drivers Jason Gunthorpe
2013-11-17  0:57   ` [tpmdd-devel] " Ashley Lai
2013-11-04  3:38 ` [PATCH 4/5 v2] tpm: Use the ops structure instead of a copy in tpm_vendor_specific Jason Gunthorpe
2013-11-17  1:09   ` [tpmdd-devel] " Ashley Lai
2013-11-04  3:38 ` [PATCH 5/5 v2] tpm: Make tpm-dev allocate a per-file structure Jason Gunthorpe
2013-11-17  2:01   ` [tpmdd-devel] " Ashley Lai
2013-11-08 16:54 ` [tpmdd-devel] tpm: Remainder of earlier clean up Ashley Lai
2013-11-08 17:23   ` Aw: " Peter Huewe
2013-12-07 13:21 ` Peter Hüwe
2013-12-10  1:33   ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).