Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751313AbaKZMeV (ORCPT ); Wed, 26 Nov 2014 07:34:21 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:42525 "EHLO e9.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750895AbaKZMeT (ORCPT ); Wed, 26 Nov 2014 07:34:19 -0500 Message-ID: <5475C8BF.8090209@linux.vnet.ibm.com> Date: Wed, 26 Nov 2014 07:34:07 -0500 From: Stefan Berger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Jarkko Sakkinen , Peter Huewe , Ashley Lai , Marcel Selhorst CC: christophe.ricard@gmail.com, josh.triplett@intel.com, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, tpmdd-devel@lists.sourceforge.net, jason.gunthorpe@obsidianresearch.com, trousers-tech@lists.sourceforge.net Subject: Re: [tpmdd-devel] [PATCH v7 05/10] tpm: device class for tpm References: <1415713513-16524-1-git-send-email-jarkko.sakkinen@linux.intel.com> <1415713513-16524-6-git-send-email-jarkko.sakkinen@linux.intel.com> In-Reply-To: <1415713513-16524-6-git-send-email-jarkko.sakkinen@linux.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14112612-0033-0000-0000-00000126FF91 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote: > Added own device class for TPM. Uses MISC_MAJOR:TPM_MINOR for the > first character device in order to retain backwards compatability. > Added tpm_dev_release() back attached to the character device. > devm now only calls put_device when the platform device is removed. > > Signed-off-by: Jarkko Sakkinen > --- > drivers/char/tpm/tpm-chip.c | 81 ++++++++++++++++++++++++++++++++++---- > drivers/char/tpm/tpm-dev.c | 36 ++--------------- > drivers/char/tpm/tpm-interface.c | 29 ++++++++++++++ > drivers/char/tpm/tpm.h | 12 ++++-- > drivers/char/tpm/tpm_i2c_nuvoton.c | 2 +- > drivers/char/tpm/tpm_tis.c | 4 +- > 6 files changed, 116 insertions(+), 48 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index 96ea8eb..df40eee 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include "tpm.h" > #include "tpm_eventlog.h" > > @@ -32,6 +33,9 @@ static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES); > static LIST_HEAD(tpm_chip_list); > static DEFINE_SPINLOCK(driver_lock); > > +struct class *tpm_class; > +dev_t tpm_devt; > + > /* > * tpm_chip_find_get - return tpm_chip for a given chip number > * @chip_num the device number for the chip > @@ -55,16 +59,24 @@ struct tpm_chip *tpm_chip_find_get(int chip_num) > } > > /** > - * tpmm_chip_remove() - free chip memory and device number > - * @data: points to struct tpm_chip instance > + * tpmm_put_device() - wrap put_device() for devm > + * @data: points to the device > + */ > +static void tpmm_put_device(void *data) > +{ > + struct device *dev = (struct device *) data; > + put_device(dev); > +} > + > +/** > + * tpm_dev_release() - free chip memory and the device number > + * @dev: the character device for the TPM chip > * > - * This is used internally by tpmm_chip_alloc() and called by devres > - * when the device is released. This function does the opposite of > - * tpmm_chip_alloc() freeing memory and the device number. > + * This is used as the release function for the character device. > */ > -static void tpmm_chip_remove(void *data) > +static void tpm_dev_release(struct device *dev) > { > - struct tpm_chip *chip = (struct tpm_chip *) data; > + struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev); > > spin_lock(&driver_lock); > clear_bit(chip->dev_num, dev_mask); > @@ -112,13 +124,66 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev, > chip->dev_num); > > chip->pdev = dev; > - devm_add_action(dev, tpmm_chip_remove, chip); > + > dev_set_drvdata(dev, chip); > > + chip->dev.class = tpm_class; > + chip->dev.release = tpm_dev_release; > + chip->dev.parent = chip->pdev; > + > + if (chip->dev_num == 0) > + chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR); > + else > + chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num); > + > + dev_set_name(&chip->dev, chip->devname); > + > + device_initialize(&chip->dev); > + > + chip->cdev.owner = chip->pdev->driver->owner; > + cdev_init(&chip->cdev, &tpm_fops); > + > + devm_add_action(dev, tpmm_put_device, chip); > return chip; > } > EXPORT_SYMBOL_GPL(tpmm_chip_alloc); > > +static int tpm_dev_add_device(struct tpm_chip *chip) > +{ > + int rc; > + > + rc = device_add(&chip->dev); > + if (rc) { > + dev_err(&chip->dev, > + "unable to device_register %s, major %d, minor %d " \ > + "err=%d\n", > + chip->devname, MAJOR(chip->dev.devt), > + MINOR(chip->dev.devt), rc); > + > + return rc; > + } > + > + rc = cdev_add(&chip->cdev, chip->dev.devt, 1); > + if (rc) { > + dev_err(&chip->dev, > + "unable to cdev_add %s, major %d, minor %d " \ > + "err=%d\n", > + chip->devname, MAJOR(chip->dev.devt), > + MINOR(chip->dev.devt), rc); > + > + device_unregister(&chip->dev); > + return rc; > + } > + > + return rc; > +} > + > +static void tpm_dev_del_device(struct tpm_chip *chip) > +{ > + cdev_del(&chip->cdev); > + device_unregister(&chip->dev); > +} > + > /* > * tpm_chip_register() - create a misc driver for the TPM chip > * @chip: TPM chip to use. > diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c > index 3568321..de0337e 100644 > --- a/drivers/char/tpm/tpm-dev.c > +++ b/drivers/char/tpm/tpm-dev.c > @@ -17,7 +17,6 @@ > * License. > * > */ > -#include > #include > #include > #include "tpm.h" > @@ -54,9 +53,8 @@ static void timeout_work(struct work_struct *work) > > 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 tpm_chip *chip = > + container_of(inode->i_cdev, struct tpm_chip, cdev); > struct file_priv *priv; > > /* It's assured that the chip will be opened just once, > @@ -173,7 +171,7 @@ static int tpm_release(struct inode *inode, struct file *file) > return 0; > } > > -static const struct file_operations tpm_fops = { > +const struct file_operations tpm_fops = { > .owner = THIS_MODULE, > .llseek = no_llseek, > .open = tpm_open, > @@ -182,32 +180,4 @@ static const struct file_operations tpm_fops = { > .release = tpm_release, > }; > > -int tpm_dev_add_device(struct tpm_chip *chip) > -{ > - int rc; > > - 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->pdev; > - > - rc = misc_register(&chip->vendor.miscdev); > - if (rc) { > - chip->vendor.miscdev.name = NULL; > - dev_err(chip->pdev, > - "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 e6b08bd..9e4ce4d 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -997,6 +997,35 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max) > } > EXPORT_SYMBOL_GPL(tpm_get_random); > > +static int __init tpm_init(void) > +{ > + int rc; > + > + tpm_class = class_create(THIS_MODULE, "tpm"); > + if (IS_ERR(tpm_class)) { > + pr_err("couldn't create tpm class\n"); > + return PTR_ERR(tpm_class); > + } > + > + rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm"); > + if (rc < 0) { > + pr_err("tpm: failed to allocate char dev region\n"); > + class_destroy(tpm_class); > + return rc; > + } > + > + return 0; > +} > + > +static void __exit tpm_exit(void) > +{ > + class_destroy(tpm_class); > + unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES); > +} > + > +subsys_initcall(tpm_init); > +module_exit(tpm_exit); > + > MODULE_AUTHOR("Leendert van Doorn (leendert@watson.ibm.com)"); > MODULE_DESCRIPTION("TPM Driver"); > MODULE_VERSION("2.0"); > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index b3a7c76..83103e0 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -23,11 +23,11 @@ > #include > #include > #include > -#include > #include > #include > #include > #include > +#include > > enum tpm_const { > TPM_MINOR = 224, /* officially assigned */ > @@ -74,7 +74,6 @@ struct tpm_vendor_specific { > int region_size; > int have_region; > > - struct miscdevice miscdev; > struct list_head list; > int locality; > unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /* jiffies */ > @@ -99,6 +98,9 @@ struct tpm_vendor_specific { > > struct tpm_chip { > struct device *pdev; /* Device stuff */ > + struct device dev; > + struct cdev cdev; > + > const struct tpm_class_ops *ops; > > int dev_num; /* /dev/tpm# */ > @@ -320,6 +322,10 @@ struct tpm_cmd_t { > tpm_cmd_params params; > } __packed; > > +extern struct class *tpm_class; > +extern dev_t tpm_devt; > +extern const struct file_operations tpm_fops; > + > 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); > @@ -340,8 +346,6 @@ extern struct tpm_chip *tpmm_chip_alloc(struct device *dev, > extern int tpm_chip_register(struct tpm_chip *chip); > extern void tpm_chip_unregister(struct tpm_chip *chip); > > -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); > > diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c > index 92ee9fa..14246e2 100644 > --- a/drivers/char/tpm/tpm_i2c_nuvoton.c > +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c > @@ -557,7 +557,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client, > rc = devm_request_irq(dev, chip->vendor.irq, > i2c_nuvoton_int_handler, > IRQF_TRIGGER_LOW, > - chip->vendor.miscdev.name, > + chip->devname, > chip); > if (rc) { > dev_err(dev, "%s() Unable to request irq: %d for use\n", > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index 660d9af..7a2c59b 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -661,7 +661,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle, > TPM_INT_VECTOR(chip->vendor.locality)); > if (devm_request_irq > (dev, i, tis_int_probe, IRQF_SHARED, > - chip->vendor.miscdev.name, chip) != 0) { > + chip->devname, chip) != 0) { > dev_info(chip->pdev, > "Unable to request irq: %d for probe\n", > i); > @@ -708,7 +708,7 @@ static int tpm_tis_init(struct device *dev, acpi_handle acpi_dev_handle, > TPM_INT_VECTOR(chip->vendor.locality)); > if (devm_request_irq > (dev, chip->vendor.irq, tis_int_handler, IRQF_SHARED, > - chip->vendor.miscdev.name, chip) != 0) { > + chip->devname, chip) != 0) { > dev_info(chip->pdev, > "Unable to request irq: %d for use\n", > chip->vendor.irq); This looks ok to me. Reviewed-by: Stefan Berger -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/