On Tue, 25 Sep 2007 15:14:50 +0200 Richard MUSIL <[EMAIL PROTECTED]> wrote:
> 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. We prefer that contributors sign off their work as per
Documentation/SubmittingPatches. Please review that and if agrereable,
send a Signed-off-by: for this work.
> /*
> + * 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 */
That's not the most useful of comments ;)
> + if (chip->vendor.release)
> + chip->vendor.release(dev);
> +
> + /* it *should* be: chip->release != NULL */
And that one's actually wrong in the context of kernel coding practices.
But whatever.
> + if (likely(chip->release))
> + chip->release(dev);
>From my reading, neither of these fields can ever be NULL, so the tests
simply aren't needed?
> + 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)
-
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/
