Hello all,

sometime ago I submitted patch to TPM layer, originally I thought this
patch could be accepted into kernel (see below). However,
since this did not happen, I wonder, if there are some problems with the
patch or whether I am expected to do/provide something else, in order to
have it accepted.

The patch follows even more below.

Thanks,
Richard

Greg KH wrote:
> On Thu, Aug 23, 2007 at 10:46:55AM +0200, Richard MUSIL wrote:
>> Dear all,
>>
>> I am currently writing virtual TPM device driver. This driver exposes
>> itself and behaves like regular TPM device (i.e. uses TPM layer which is
>> already present in kernel), but instead of talking to hardware it talks
>> to user space.
> 
> Heh, I like the idea, I can imagine what it could be used for :)
> 
>> What I present below is rather quickfix with least impact on other TPM
>> parts (drivers). The patch uses device->remove callback (of
>> platform_device device) and reroutes this to itself. In this
>> callback it eventually calls vendor callback and finally kfrees all
>> memory resources allocated on its own.
> 
> It looks sane to me, nice fixup.
> 
> thanks,
> 
> greg k-h
> 

>From bd80b63ca2e1edb761a3ffcf87bd86c30a44ca5f Mon Sep 17 00:00:00 2001
From: Richard Musil <[EMAIL PROTECTED]>
Date: Tue, 21 Aug 2007 16:46:06 +0200
Subject: [PATCH] Change in TPM module:

The clean up procedure now uses platform device "release" callback to
handle memory clean up. For this purpose "release" function callback was
added to struct tpm_vendor_specific, so hw device driver provider can get
called when it is safe to remove all allocated resources.

This is supposed to fix a bug in device removal, where device while in
receive function (waiting on timeout) was prone to segfault, if the
tpm_chip struct was unallocated before the timeout expired (in
tpm_remove_hardware).
---
 drivers/char/tpm/tpm.c |   46 +++++++++++++++++++++++++++++-----------------
 drivers/char/tpm/tpm.h |    2 ++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 9bb5429..41eba7e 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -1031,18 +1031,13 @@ void tpm_remove_hardware(struct device *dev)
 
        spin_unlock(&driver_lock);
 
-       dev_set_drvdata(dev, NULL);
        misc_deregister(&chip->vendor.miscdev);
-       kfree(chip->vendor.miscdev.name);
 
        sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
        tpm_bios_log_teardown(chip->bios_dir);
 
-       clear_bit(chip->dev_num, dev_mask);
-
-       kfree(chip);
-
-       put_device(dev);
+       /* write it this way to be explicit (chip->dev == dev) */
+       put_device(chip->dev);
 }
 EXPORT_SYMBOL_GPL(tpm_remove_hardware);
 
@@ -1083,6 +1078,28 @@ int tpm_pm_resume(struct device *dev)
 EXPORT_SYMBOL_GPL(tpm_pm_resume);
 
 /*
+ * Once all references to platform device are down to 0,
+ * release all allocated structures.
+ * In case vendor provided release function,
+ * call it too.
+ */
+static void tpm_dev_release(struct device *dev)
+{
+       struct tpm_chip *chip = dev_get_drvdata(dev);
+       /* call vendor release, if defined */
+       if (chip->vendor.release)
+               chip->vendor.release(dev);
+
+       /* it *should* be: chip->release != NULL */
+       if (likely(chip->release))
+               chip->release(dev);
+
+       clear_bit(chip->dev_num, dev_mask);
+       kfree(chip->vendor.miscdev.name);
+       kfree(chip);
+}
+
+/*
  * Called from tpm_<specific>.c probe function only for devices 
  * the driver has determined it should claim.  Prior to calling
  * this function the specific probe function has called pci_enable_device
@@ -1136,23 +1153,21 @@ struct tpm_chip *tpm_register_hardware(struct device 
*dev, const struct tpm_vend
 
        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);
-               put_device(dev);
-               clear_bit(chip->dev_num, dev_mask);
-               kfree(chip);
-               kfree(devname);
+               put_device(chip->dev);
                return NULL;
        }
 
        spin_lock(&driver_lock);
 
-       dev_set_drvdata(dev, chip);
-
        list_add(&chip->list, &tpm_chip_list);
 
        spin_unlock(&driver_lock);
@@ -1160,10 +1175,7 @@ struct tpm_chip *tpm_register_hardware(struct device 
*dev, const struct tpm_vend
        if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group)) {
                list_del(&chip->list);
                misc_deregister(&chip->vendor.miscdev);
-               put_device(dev);
-               clear_bit(chip->dev_num, dev_mask);
-               kfree(chip);
-               kfree(devname);
+               put_device(chip->dev);
                return NULL;
        }
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index b2e2b00..f1c265e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -74,6 +74,7 @@ struct tpm_vendor_specific {
        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 attribute_group *attr_group;
        struct list_head list;
@@ -106,6 +107,7 @@ struct tpm_chip {
        struct dentry **bios_dir;
 
        struct list_head list;
+       void (*release) (struct device *);
 };
 
 #define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
-- 1.5.3.rc5 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to