Here is info. -
This patch adds multiple locality support in tpm_tis driver.
drivers/char/tpm/tpm_tis.c | 83 ++++++++++++++++++++++++-----------
1 file changed, 57 insertions(+), 26 deletions(-)
--- pristine-linux-2.6.18/drivers/char/tpm/tpm_tis.c 2006-09-19
20:42:06.000000000 -0700
+++ linux-2.6.18-xen/drivers/char/tpm/tpm_tis.c 2007-10-11
13:28:38.000000000 -0700
@@ -56,29 +56,37 @@ enum tis_int_flags {
enum tis_defaults {
TIS_MEM_BASE = 0xFED40000,
- TIS_MEM_LEN = 0x5000,
+ TIS_LOCALITY_SHIFT = 12,
+ TIS_MEM_LEN = 0x1000,
TIS_SHORT_TIMEOUT = 750, /* ms */
TIS_LONG_TIMEOUT = 2000, /* 2 sec */
};
-#define TPM_ACCESS(l) (0x0000 | ((l) << 12))
-#define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12))
-#define TPM_INT_VECTOR(l) (0x000C | ((l) << 12))
-#define TPM_INT_STATUS(l) (0x0010 | ((l) << 12))
-#define TPM_INTF_CAPS(l) (0x0014 | ((l) << 12))
-#define TPM_STS(l) (0x0018 | ((l) << 12))
-#define TPM_DATA_FIFO(l) (0x0024 | ((l) << 12))
+#define TPM_ACCESS(l) (0x0000)
+#define TPM_INT_ENABLE(l) (0x0008)
+#define TPM_INT_VECTOR(l) (0x000C)
+#define TPM_INT_STATUS(l) (0x0010)
+#define TPM_INTF_CAPS(l) (0x0014)
+#define TPM_STS(l) (0x0018)
+#define TPM_DATA_FIFO(l) (0x0024)
-#define TPM_DID_VID(l) (0x0F00 | ((l) << 12))
-#define TPM_RID(l) (0x0F04 | ((l) << 12))
+#define TPM_DID_VID(l) (0x0F00)
+#define TPM_RID(l) (0x0F04)
static LIST_HEAD(tis_chips);
static DEFINE_SPINLOCK(tis_lock);
static int check_locality(struct tpm_chip *chip, int l)
{
- if ((ioread8(chip->vendor.iobase + TPM_ACCESS(l)) &
- (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
+ unsigned char tpm_access;
+
+ tpm_access = ioread8(chip->vendor.iobase + TPM_ACCESS(l));
+
+ /* check if locality is closed */
+ if(tpm_access == 0xFF)
+ return -1;
+
+ if ((tpm_access & (TPM_ACCESS_ACTIVE_LOCALITY |
TPM_ACCESS_VALID)) ==
(TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
return chip->vendor.locality = l;
@@ -251,10 +259,14 @@ static int tpm_tis_recv(struct tpm_chip
out:
tpm_tis_ready(chip);
- release_locality(chip, chip->vendor.locality, 0);
+ release_locality(chip, chip->vendor.locality, 1);
return size;
}
+static int locality = 0;
+module_param(locality, int, 0444);
+MODULE_PARM_DESC(locality, "TPM Locality To access");
+
/*
* If interrupts are used (signaled by an irq set in the vendor
structure)
* tpm.c can skip polling for the data to be available as the interrupt
is
@@ -266,7 +278,7 @@ static int tpm_tis_send(struct tpm_chip
size_t count = 0;
u32 ordinal;
- if (request_locality(chip, 0) < 0)
+ if (request_locality(chip, locality) < 0)
return -EBUSY;
status = tpm_tis_status(chip);
@@ -326,7 +338,7 @@ static int tpm_tis_send(struct tpm_chip
return len;
out_err:
tpm_tis_ready(chip);
- release_locality(chip, chip->vendor.locality, 0);
+ release_locality(chip, chip->vendor.locality, 1);
return rc;
}
@@ -401,7 +413,10 @@ static irqreturn_t tis_int_handler(int i
{
struct tpm_chip *chip = (struct tpm_chip *) dev_id;
u32 interrupt;
- int i;
+
+ /* check if interrupt is meant for this locality */
+ if(check_locality(chip, locality) < 0)
+ return IRQ_NONE;
interrupt = ioread32(chip->vendor.iobase +
TPM_INT_STATUS(chip->vendor.locality));
@@ -411,10 +426,6 @@ static irqreturn_t tis_int_handler(int i
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(&chip->vendor.read_queue);
- if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
- for (i = 0; i < 5; i++)
- if (check_locality(chip, i) >= 0)
- break;
if (interrupt &
(TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_STS_VALID_INT |
TPM_INTF_CMD_READY_INT))
@@ -440,7 +451,7 @@ static int tpm_tis_init(struct device *d
struct tpm_chip *chip;
if (!start)
- start = TIS_MEM_BASE;
+ start = TIS_MEM_BASE | (locality << TIS_LOCALITY_SHIFT);
if (!len)
len = TIS_MEM_LEN;
@@ -490,8 +501,9 @@ static int tpm_tis_init(struct device *d
if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
dev_dbg(dev, "\tData Avail Int Support\n");
- if (request_locality(chip, 0) != 0) {
- rc = -ENODEV;
+ if (request_locality(chip, locality) < 0) {
+ rc = -EBUSY;
+ printk("tpm_tis: failed request_locality %d\n",
locality);
goto out_err;
}
@@ -582,9 +594,11 @@ static int tpm_tis_init(struct device *d
tpm_get_timeouts(chip);
tpm_continue_selftest(chip);
+ release_locality(chip, chip->vendor.locality, 1);
return 0;
out_err:
+ release_locality(chip, chip->vendor.locality, 1);
if (chip->vendor.iobase)
iounmap(chip->vendor.iobase);
tpm_remove_hardware(chip->dev);
@@ -636,7 +650,7 @@ module_param_string(hid, tpm_pnp_tbl[TIS
MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to
probe");
static struct device_driver tis_drv = {
- .name = "tpm_tis",
+ .name = "",
.bus = &platform_bus_type,
.owner = THIS_MODULE,
.suspend = tpm_pm_suspend,
@@ -648,19 +662,34 @@ static struct platform_device *pdev;
static int force;
module_param(force, bool, 0444);
MODULE_PARM_DESC(force, "Force device probe rather than using ACPI
entry");
+
+static char *devname;
+
static int __init init_tis(void)
{
+#define DEVNAME_SIZE 10
+
int rc;
+ if ((locality < 0) || (locality > 4)) {
+ return PTR_ERR(pdev);
+ }
+
if (force) {
+ devname = kmalloc(DEVNAME_SIZE, GFP_KERNEL);
+ scnprintf(devname, DEVNAME_SIZE, "%s%d", "tpm_tis",
locality);
+
+ tis_drv.name = devname;
rc = driver_register(&tis_drv);
if (rc < 0)
return rc;
- if
(IS_ERR(pdev=platform_device_register_simple("tpm_tis", -1, NULL, 0)))
+
+ if (IS_ERR(pdev=platform_device_register_simple(devname,
-1, NULL, 0)))
return PTR_ERR(pdev);
if((rc=tpm_tis_init(&pdev->dev, 0, 0)) != 0) {
platform_device_unregister(pdev);
driver_unregister(&tis_drv);
+ kfree(devname);
}
return rc;
}
@@ -692,7 +721,9 @@ static void __exit cleanup_tis(void)
if (force) {
platform_device_unregister(pdev);
driver_unregister(&tis_drv);
- } else
+ kfree(devname);
+ }
+ else
pnp_unregister_driver(&tis_pnp_driver);
}
I don't have scripts/checkpatch.pl in my linux tree. Where can I get
one?
Devname is being used in 2 functions. That's why it is global. Locality
parameter is initialized with 0 just to be safe. Are all the global
variables in driver is guaranteed to be init 0? Even if it is it doesn't
hurt to init it.
-----Original Message-----
From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
Sent: Thursday, October 11, 2007 11:54 AM
To: Agarwal, Lomesh
Cc: Valdis.Kletnieks@vt.edu; linux-kernel@vger.kernel.org
Subject: Re: TPM driver changes to support multiple locality
On Thu, 11 Oct 2007 11:33:35 -0700 Agarwal, Lomesh wrote:
Observe/use kernel coding style.
Run the patch thru scripts/checkpatch.pl and check its suggestions.
Use "diffstat -p1 -w70" and put that summary near the top of the
patch (after the patch description).
Use -p option of diff to generate the diff so that reviewers
can see the function name that patch blocks apply to.
Use tabs instead of spaces for indenting.
More below.
Indent above/below the same amount (1 tab, not spaces).
if (
if ((
No need to init this to 0.
interrupt
Use same indenting as surrounding code.
Looks like all of those "<< 12"s (mostly in header file) could use
some helpers.
Use some prefix identity string that tells the use what module
failed here.
No need to init to 0 or NULL.
But why is devname global here?
could it be in the function below or is it used later?
A quick scan finds only local function use below...
---
~Randy
-
