Skip to contentSkip to conversation

tpm: updates for new hardware

PATCH In review Kylene Jo Hall 3 participants 4 messages Nov 11, 2005 – Nov 16, 2005

Subsystems (2)
Arm/Microchip (AT91) SoC support TPM device driver
lore
Share & tools
lore

Kylene Jo Hall

author
#0 lore raw

PATCH 1/1 tpm: updates for new hardware

This is the patch to support TPMs on power ppc hardware. It has been reworked as requested to remove the need for messing with the io page mask by just using ioremap.

Signed-off-by Kylene Hall <[email protected]>

---

drivers/char/tpm/tpm_atmel.c +37 -71
diff -uprN --exclude='*.o' --exclude='*.ko' --exclude='.*' --exclude='*~' --exclude='*infineon*' --exclude='*nsc*' --exclude='*mod*' --exclude='*.rej' --exclude='*.orig' linux-2.6.14/drivers/char/tpm/tpm_atmel.c linux-2.6.14-rc4-tpm/drivers/char/tpm/tpm_atmel.c
@@ -19,14 +19,8 @@
  * 
  */
 
-#include <linux/platform_device.h>
 #include "tpm.h"
-
-/* Atmel definitions */
-enum tpm_atmel_addr {
-	TPM_ATMEL_BASE_ADDR_LO = 0x08,
-	TPM_ATMEL_BASE_ADDR_HI = 0x09
-};
+#include "tpm_atmel.h"
 
 /* write status bits */
 enum tpm_atmel_write_status {

@@ -53,13 +47,13 @@ static int tpm_atml_recv(struct tpm_chip
 		return -EIO;
 
 	for (i = 0; i < 6; i++) {
-		status = inb(chip->vendor->base + 1);
+		status = atmel_getb(chip, 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
 			dev_err(chip->dev,
 				"error reading header\n");
 			return -EIO;
 		}
-		*buf++ = inb(chip->vendor->base);
+		*buf++ = atmel_getb(chip, 0);
 	}
 
 	/* size of the data received */

@@ -70,7 +64,7 @@ static int tpm_atml_recv(struct tpm_chip
 		dev_err(chip->dev,
 			"Recv size(%d) less than available space\n", size);
 		for (; i < size; i++) {	/* clear the waiting data anyway */
-			status = inb(chip->vendor->base + 1);
+			status = atmel_getb(chip, 1);
 			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
 				dev_err(chip->dev,
 					"error reading data\n");

@@ -82,17 +76,17 @@ static int tpm_atml_recv(struct tpm_chip
 
 	/* read all the data available */
 	for (; i < size; i++) {
-		status = inb(chip->vendor->base + 1);
+		status = atmel_getb(chip, 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
 			dev_err(chip->dev,
 				"error reading data\n");
 			return -EIO;
 		}
-		*buf++ = inb(chip->vendor->base);
+		*buf++ = atmel_getb(chip, 0);
 	}
 
 	/* make sure data available is gone */
-	status = inb(chip->vendor->base + 1);
+	status = atmel_getb(chip, 1);
 	if (status & ATML_STATUS_DATA_AVAIL) {
 		dev_err(chip->dev, "data available is stuck\n");
 		return -EIO;

@@ -108,7 +102,7 @@ static int tpm_atml_send(struct tpm_chip
 	dev_dbg(chip->dev, "tpm_atml_send:\n");
 	for (i = 0; i < count; i++) {
 		dev_dbg(chip->dev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
-		outb(buf[i], chip->vendor->base);
+		atmel_putb(buf[i], chip, 0);
 	}
 
 	return count;

@@ -116,12 +110,12 @@ static int tpm_atml_send(struct tpm_chip
 
 static void tpm_atml_cancel(struct tpm_chip *chip)
 {
-	outb(ATML_STATUS_ABORT, chip->vendor->base + 1);
+	atmel_putb(ATML_STATUS_ABORT, chip, 1);
 }
 
 static u8 tpm_atml_status(struct tpm_chip *chip)
 {
-	return inb(chip->vendor->base + 1);
+	return atmel_getb(chip, 1);
 }
 
 static struct file_operations atmel_ops = {

@@ -162,12 +156,16 @@ static struct tpm_vendor_specific tpm_at
 
 static struct platform_device *pdev;
 
-static void __devexit tpm_atml_remove(struct device *dev)
+static void atml_plat_remove(void)
 {
-	struct tpm_chip *chip = dev_get_drvdata(dev);
+	struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
+
 	if (chip) {
-		release_region(chip->vendor->base, 2);
+		if (chip->vendor->have_region)
+			atmel_release_region(chip->vendor->base, chip->vendor->region_size);
+		atmel_put_base_addr(chip->vendor);	
 		tpm_remove_hardware(chip->dev);
+		platform_device_unregister(pdev);
 	}
 }
 

@@ -182,72 +180,40 @@ static struct device_driver atml_drv = {
 static int __init init_atmel(void)
 {
 	int rc = 0;
-	int lo, hi;
 
 	driver_register(&atml_drv);
 
-	lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO);
-	hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI);
-
-	tpm_atmel.base = (hi<<8)|lo;
-
-	/* verify that it is an Atmel part */
-	if (tpm_read_index(TPM_ADDR, 4) != 'A' || tpm_read_index(TPM_ADDR, 5) != 'T'
-	    || tpm_read_index(TPM_ADDR, 6) != 'M' || tpm_read_index(TPM_ADDR, 7) != 'L') {
-		return -ENODEV;
+	if (atmel_get_base_addr(&tpm_atmel) != 0) {
+		rc = -ENODEV;
+		goto err_unreg_drv;
 	}
 
-	/* verify chip version number is 1.1 */
-	if (	(tpm_read_index(TPM_ADDR, 0x00) != 0x01) ||
-		(tpm_read_index(TPM_ADDR, 0x01) != 0x01 ))
-		return -ENODEV;
-
-	pdev = kzalloc(sizeof(struct platform_device), GFP_KERNEL);
-	if ( !pdev )
-		return -ENOMEM;
-
-	pdev->name = "tpm_atmel0";
-	pdev->id = -1;
-	pdev->num_resources = 0;
-	pdev->dev.release = tpm_atml_remove;
-	pdev->dev.driver = &atml_drv;
+	tpm_atmel.have_region = (atmel_request_region( tpm_atmel.base, tpm_atmel.region_size, "tpm_atmel0") == NULL) ? 0 : 1;
 
-	if ((rc = platform_device_register(pdev)) < 0) {
-		kfree(pdev);
-		pdev = NULL;
-		return rc;
+	if (IS_ERR(pdev = platform_device_register_simple("tpm_atmel", -1, NULL, 0 ))) {
+		rc = PTR_ERR(pdev);
+		goto err_rel_reg;
 	}
 
-	if (request_region(tpm_atmel.base, 2, "tpm_atmel0") == NULL ) {
-		platform_device_unregister(pdev);
-		kfree(pdev);
-		pdev = NULL;
-		return -EBUSY;
-	}
-
-	if ((rc = tpm_register_hardware(&pdev->dev, &tpm_atmel)) < 0) {
-		release_region(tpm_atmel.base, 2);
-		platform_device_unregister(pdev);
-		kfree(pdev);
-		pdev = NULL;
-		return rc;
-	}
-
-	dev_info(&pdev->dev, "Atmel TPM 1.1, Base Address: 0x%x\n",
-			tpm_atmel.base);
+	if ((rc = tpm_register_hardware(&pdev->dev, &tpm_atmel)) < 0) 
+		goto err_unreg_dev;
 	return 0;
+
+err_unreg_dev:
+	platform_device_unregister(pdev);
+err_rel_reg:
+	if (tpm_atmel.have_region)
+		atmel_release_region(tpm_atmel.base, tpm_atmel.region_size);
+	atmel_put_base_addr(&tpm_atmel);
+err_unreg_drv:
+	driver_unregister(&atml_drv);
+	return rc;
 }
 
 static void __exit cleanup_atmel(void)
 {
-	if (pdev) {
-		tpm_atml_remove(&pdev->dev);
-		platform_device_unregister(pdev);
-		kfree(pdev);
-		pdev = NULL;
-	}
-
 	driver_unregister(&atml_drv);
+	atml_plat_remove();
 }
 
 module_init(init_atmel);
drivers/char/tpm/tpm_atmel.h +129 -0

129 lines not on this page · raw message

drivers/char/tpm/tpm.h +5 -1
diff -uprN --exclude='*.o' --exclude='*.ko' --exclude='.*' --exclude='*~' --exclude='*infineon*' --exclude='*nsc*' --exclude='*mod*' --exclude='*.rej' --exclude='*.orig' linux-2.6.14/drivers/char/tpm/tpm.h linux-2.6.14-rc4-tpm/drivers/char/tpm/tpm.h
@@ -50,7 +50,11 @@ struct tpm_vendor_specific {
 	u8 req_complete_mask;
 	u8 req_complete_val;
 	u8 req_canceled;
-	u16 base;		/* TPM base address */
+	void __iomem *iobase;		/* ioremapped address */
+	unsigned long base;		/* TPM base address */
+
+	int region_size;
+	int have_region;
 
 	int (*recv) (struct tpm_chip *, u8 *, size_t);
 	int (*send) (struct tpm_chip *, u8 *, size_t);
#1 lore raw

drivers/char/tpm/tpm_atmel.h line 79

Quoted 7 lines · Kylene Jo Hall
drivers/char/tpm/tpm_atmel.h +6 -0
+#ifdef CONFIG_PPC64
+#define atmel_getb(chip, offset) readb(chip->vendor->iobase + offset);
+#define atmel_putb(val, chip, offset) writeb(val, chip->vendor->iobase + offset)
...
+#else
+#define atmel_getb(chip, offset) inb(chip->vendor->base + offset)
+#define atmel_putb(val, chip, offset) outb(val, chip->vendor->base + offset)

Why don't you use ioread8() instead of defining atmel_getb()?

You'd still need something PPC64-specific to initialize the iomem cookie, but the accessors would go away.

Unfortunately, ioread8() and associated interfaces aren't mentioned under Documentation/, but there are some hints in lib/iomap.c.

#2 lore raw
Quoted 8 lines

Why don't you use ioread8() instead of defining atmel_getb()?

You'd still need something PPC64-specific to initialize the iomem cookie, but the accessors would go away.

Unfortunately, ioread8() and associated interfaces aren't mentioned under Documentation/, but there are some hints in lib/iomap.c.

Yes, I was about to reply the same thing :)

Ben.

author
#3 lore raw
In reply to Bjorn Helgaas

Patch to use ioread8 and iowrite8 as suggested.

Signed-off-by Kylene Hall <[email protected]>
Quoted 11 lines · Bjorn Helgaas

Review

Quoted 7 lines · Kylene Jo Hall
(quoted diff) +6 -0
+#ifdef CONFIG_PPC64
+#define atmel_getb(chip, offset) readb(chip->vendor->iobase + offset);
+#define atmel_putb(val, chip, offset) writeb(val, chip->vendor->iobase + offset)
...
+#else
+#define atmel_getb(chip, offset) inb(chip->vendor->base + offset)
+#define atmel_putb(val, chip, offset) outb(val, chip->vendor->base + offset)

Why don't you use ioread8() instead of defining atmel_getb()?

Quoted 6 lines

You'd still need something PPC64-specific to initialize the iomem cookie, but the accessors would go away.

Unfortunately, ioread8() and associated interfaces aren't mentioned under Documentation/, but there are some hints in lib/iomap.c.

---

drivers/char/tpm/tpm_atmel.c +27 -22
@@ -47,13 +47,12 @@ static int tpm_atml_recv(struct tpm_chip
 		return -EIO;
 
 	for (i = 0; i < 6; i++) {
-		status = atmel_getb(chip, 1);
+		status = ioread8(chip->vendor->iobase + 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-			dev_err(chip->dev,
-				"error reading header\n");
+			dev_err(chip->dev, "error reading header\n");
 			return -EIO;
 		}
-		*buf++ = atmel_getb(chip, 0);
+		*buf++ = ioread8(chip->vendor->iobase);
 	}
 
 	/* size of the data received */

@@ -64,10 +63,9 @@ static int tpm_atml_recv(struct tpm_chip
 		dev_err(chip->dev,
 			"Recv size(%d) less than available space\n", size);
 		for (; i < size; i++) {	/* clear the waiting data anyway */
-			status = atmel_getb(chip, 1);
+			status = ioread8(chip->vendor->iobase + 1);
 			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-				dev_err(chip->dev,
-					"error reading data\n");
+				dev_err(chip->dev, "error reading data\n");
 				return -EIO;
 			}
 		}

@@ -76,17 +74,17 @@ static int tpm_atml_recv(struct tpm_chip
 
 	/* read all the data available */
 	for (; i < size; i++) {
-		status = atmel_getb(chip, 1);
+		status = ioread8(chip->vendor->iobase + 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-			dev_err(chip->dev,
-				"error reading data\n");
+			dev_err(chip->dev, "error reading data\n");
 			return -EIO;
 		}
-		*buf++ = atmel_getb(chip, 0);
+		*buf++ = ioread8(chip->vendor->iobase);
 	}
 
 	/* make sure data available is gone */
-	status = atmel_getb(chip, 1);
+	status = ioread8(chip->vendor->iobase + 1);
+
 	if (status & ATML_STATUS_DATA_AVAIL) {
 		dev_err(chip->dev, "data available is stuck\n");
 		return -EIO;

@@ -102,7 +100,7 @@ static int tpm_atml_send(struct tpm_chip
 	dev_dbg(chip->dev, "tpm_atml_send:\n");
 	for (i = 0; i < count; i++) {
 		dev_dbg(chip->dev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
-		atmel_putb(buf[i], chip, 0);
+ 		iowrite8(buf[i], chip->vendor->iobase);
 	}
 
 	return count;

@@ -110,12 +108,12 @@ static int tpm_atml_send(struct tpm_chip
 
 static void tpm_atml_cancel(struct tpm_chip *chip)
 {
-	atmel_putb(ATML_STATUS_ABORT, chip, 1);
+	iowrite8(ATML_STATUS_ABORT, chip->vendor->iobase + 1);
 }
 
 static u8 tpm_atml_status(struct tpm_chip *chip)
 {
-	return atmel_getb(chip, 1);
+	return ioread8(chip->vendor->iobase + 1);
 }
 
 static struct file_operations atmel_ops = {

@@ -162,7 +160,8 @@ static void atml_plat_remove(void)
 
 	if (chip) {
 		if (chip->vendor->have_region)
-			atmel_release_region(chip->vendor->base, chip->vendor->region_size);
+			atmel_release_region(chip->vendor->base,
+					     chip->vendor->region_size);
 		atmel_put_base_addr(chip->vendor);
 		tpm_remove_hardware(chip->dev);
 		platform_device_unregister(pdev);

@@ -183,14 +182,19 @@ static int __init init_atmel(void)
 
 	driver_register(&atml_drv);
 
-	if (atmel_get_base_addr(&tpm_atmel) != 0) {
+	if ((tpm_atmel.iobase = atmel_get_base_addr(&tpm_atmel)) == NULL) {
 		rc = -ENODEV;
 		goto err_unreg_drv;
 	}
 
-	tpm_atmel.have_region = (atmel_request_region( tpm_atmel.base, tpm_atmel.region_size, "tpm_atmel0") == NULL) ? 0 : 1;
-
-	if (IS_ERR(pdev = platform_device_register_simple("tpm_atmel", -1, NULL, 0 ))) {
+	tpm_atmel.have_region =
+	    (atmel_request_region
+	     (tpm_atmel.base, tpm_atmel.region_size,
+	      "tpm_atmel0") == NULL) ? 0 : 1;
+
+	if (IS_ERR
+	    (pdev =
+	     platform_device_register_simple("tpm_atmel", -1, NULL, 0))) {
 		rc = PTR_ERR(pdev);
 		goto err_rel_reg;
 	}

@@ -202,9 +206,10 @@ static int __init init_atmel(void)
 err_unreg_dev:
 	platform_device_unregister(pdev);
 err_rel_reg:
-	if (tpm_atmel.have_region)
-		atmel_release_region(tpm_atmel.base, tpm_atmel.region_size);
 	atmel_put_base_addr(&tpm_atmel);
+	if (tpm_atmel.have_region)
+		atmel_release_region(tpm_atmel.base,
+				     tpm_atmel.region_size);
 err_unreg_drv:
 	driver_unregister(&atml_drv);
 	return rc;
drivers/char/tpm/tpm_atmel.h +12 -10
@@ -27,12 +27,14 @@
 #define atmel_putb(val, chip, offset) writeb(val, chip->vendor->iobase + offset)
 #define atmel_request_region request_mem_region
 #define atmel_release_region release_mem_region
-static inline void atmel_put_base_addr(struct tpm_vendor_specific *vendor)
+
+static inline void atmel_put_base_addr(struct tpm_vendor_specific
+					 *vendor)
 {
 	iounmap(vendor->iobase);
 }
 
-static int atmel_get_base_addr(struct tpm_vendor_specific *vendor)
+static void __iomem * atmel_get_base_addr(struct tpm_vendor_specific *vendor)
 {
 	struct device_node *dn;
 	unsigned long address, size;

@@ -44,11 +46,11 @@ static int atmel_get_base_addr(struct tp
 	dn = of_find_node_by_name(NULL, "tpm");
 
 	if (!dn)
-		return 1;
+		return NULL;
 
 	if (!device_is_compatible(dn, "AT97SC3201")) {
 		of_node_put(dn);
-		return 1;
+		return NULL;
 	}
 
 	reg = (unsigned int *) get_property(dn, "reg", &reglen);

@@ -71,8 +73,7 @@ static int atmel_get_base_addr(struct tp
 
 	vendor->base = address;
 	vendor->region_size = size;
-	vendor->iobase = ioremap(address, size);
-	return 0;
+	return ioremap(vendor->base, vendor->region_size);
 }
 #else
 #define atmel_getb(chip, offset) inb(chip->vendor->base + offset)

@@ -105,18 +106,19 @@ static int atmel_verify_tpm11(void)
 	return 0;
 }
 
-static inline void atmel_put_base_addr(struct tpm_vendor_specific *vendor)
+static inline void atmel_put_base_addr(struct tpm_vendor_specific
+					 *vendor)
 {
 }
 
 /* Determine where to talk to device */
-static unsigned long atmel_get_base_addr(struct tpm_vendor_specific
+static void __iomem * atmel_get_base_addr(struct tpm_vendor_specific
 					 *vendor)
 {
 	int lo, hi;
 
 	if (atmel_verify_tpm11() != 0)
-		return 1;
+		return NULL;
 
 	lo = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_LO);
 	hi = tpm_read_index(TPM_ADDR, TPM_ATMEL_BASE_ADDR_HI);

@@ -124,6 +126,6 @@ static unsigned long atmel_get_base_addr
 	vendor->base = (hi << 8) | lo;
 	vendor->region_size = 2;
 
-	return 0;
+	return ioport_map(vendor->base, vendor->region_size);
 }
 #endif