[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5475E5F1.8080508@linux.vnet.ibm.com>
Date: Wed, 26 Nov 2014 09:38:41 -0500
From: Stefan Berger <stefanb@...ux.vnet.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
Peter Huewe <peterhuewe@....de>,
Ashley Lai <ashley@...leylai.com>,
Marcel Selhorst <tpmdd@...horst.net>
CC: christophe.ricard@...il.com, josh.triplett@...el.com,
linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
tpmdd-devel@...ts.sourceforge.net,
jason.gunthorpe@...idianresearch.com,
trousers-tech@...ts.sourceforge.net
Subject: Re: [tpmdd-devel] [PATCH v7 02/10] tpm: two-phase chip management
functions
On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> tpm_register_hardware() and tpm_remove_hardware() are called often
> before initializing the device. This is wrong order since it could
> be that main TPM driver needs a fully initialized chip to be able to
> do its job. For example, now it is impossible to move common startup
> functions such as tpm_do_selftest() to tpm_register_hardware().
>
> Added tpmm_chip_alloc() and tpm_chip_register() where tpm_chip_alloc()
> reserves memory resources and tpm_chip_register() initializes the
> device driver. This way it is possible to alter struct tpm_chip
> attributes and initialize the device driver before passing it to
> tpm_chip_register().
>
> The framework takes care of freeing struct tpm_chip by using devres
> API. The broken release callback has been wiped. For example, ACPI
> drivers do not ever get this callback.
>
> This is a interm step to get proper life-cycle for TPM device drivers.
> The next steps are adding proper ref counting and locking to tpm_chip
> that is used in every place in the TPM driver.
>
> Big thank you to Jason Gunthorpe for carefully reviewing this part
> of the code. Without his contribution reaching the best quality would
> not have been possible.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> ---
> drivers/char/tpm/Makefile | 2 +-
> drivers/char/tpm/tpm-chip.c | 196 ++++++++++++++++++++++++++++++++++++
> drivers/char/tpm/tpm-interface.c | 148 +--------------------------
> drivers/char/tpm/tpm.h | 11 +-
> drivers/char/tpm/tpm_atmel.c | 11 +-
> drivers/char/tpm/tpm_i2c_atmel.c | 33 ++----
> drivers/char/tpm/tpm_i2c_infineon.c | 37 ++-----
> drivers/char/tpm/tpm_i2c_nuvoton.c | 44 +++-----
> drivers/char/tpm/tpm_i2c_stm_st33.c | 38 +++----
> drivers/char/tpm/tpm_ibmvtpm.c | 17 ++--
> drivers/char/tpm/tpm_infineon.c | 29 +++---
> drivers/char/tpm/tpm_nsc.c | 14 ++-
> drivers/char/tpm/tpm_tis.c | 78 ++++++--------
> drivers/char/tpm/xen-tpmfront.c | 14 +--
> 14 files changed, 329 insertions(+), 343 deletions(-)
> create mode 100644 drivers/char/tpm/tpm-chip.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 4d85dd6..837da04 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -2,7 +2,7 @@
> # Makefile for the kernel tpm device drivers.
> #
> obj-$(CONFIG_TCG_TPM) += tpm.o
> -tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o
> +tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
> tpm-$(CONFIG_ACPI) += tpm_ppi.o
>
> ifdef CONFIG_ACPI
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> new file mode 100644
> index 0000000..cf3ad24
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + * Copyright (C) 2014 Intel Corporation
> + *
> + * Authors:
> + * Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
> + * Leendert van Doorn <leendert@...son.ibm.com>
> + * Dave Safford <safford@...son.ibm.com>
> + * Reiner Sailer <sailer@...son.ibm.com>
> + * Kylene Hall <kjhall@...ibm.com>
> + *
> + * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
> + *
> + * TPM chip management routines.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + *
> + */
> +
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <linux/freezer.h>
> +#include "tpm.h"
> +#include "tpm_eventlog.h"
> +
> +static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> +static LIST_HEAD(tpm_chip_list);
> +static DEFINE_SPINLOCK(driver_lock);
> +
> +/*
> + * tpm_chip_find_get - return tpm_chip for a given chip number
> + * @chip_num the device number for the chip
> + */
> +struct tpm_chip *tpm_chip_find_get(int chip_num)
> +{
> + struct tpm_chip *pos, *chip = NULL;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
> + if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
> + continue;
> +
> + if (try_module_get(pos->dev->driver->owner)) {
> + chip = pos;
> + break;
> + }
> + }
> + rcu_read_unlock();
> + return chip;
> +}
> +
> +/**
> + * tpmm_chip_remove() - free chip memory and device number
> + * @data: points to struct tpm_chip instance
> + *
> + * 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.
> + */
> +static void tpmm_chip_remove(void *data)
> +{
> + struct tpm_chip *chip = (struct tpm_chip *) data;
> +
> + spin_lock(&driver_lock);
> + clear_bit(chip->dev_num, dev_mask);
> + spin_unlock(&driver_lock);
> + kfree(chip);
> +}
> +
> +/**
> + * tpmm_chip_alloc() - allocate a new struct tpm_chip instance
> + * @dev: device to which the chip is associated
> + * @ops: struct tpm_class_ops instance
> + *
> + * Allocates a new struct tpm_chip instance and assigns a free
> + * device number for it. Caller does not have to worry about
> + * freeing the allocated resources. When the devices is removed
> + * devres calls tpmm_chip_remove() to do the job.
> + */
> +struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> + const struct tpm_class_ops *ops)
> +{
> + struct tpm_chip *chip;
> +
> + chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> + if (chip == NULL)
> + return ERR_PTR(-ENOMEM);
> +
> + mutex_init(&chip->tpm_mutex);
> + INIT_LIST_HEAD(&chip->list);
> +
> + chip->ops = ops;
> +
> + spin_lock(&driver_lock);
> + chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> + spin_unlock(&driver_lock);
> +
> + if (chip->dev_num >= TPM_NUM_DEVICES) {
> + dev_err(dev, "No available tpm device numbers\n");
> + kfree(chip);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + set_bit(chip->dev_num, dev_mask);
> +
> + scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
> + chip->dev_num);
nit picking here: "tpm%d", chip->dev_num would just be good enough
> +
> + chip->dev = dev;
> + devm_add_action(dev, tpmm_chip_remove, chip);
> + dev_set_drvdata(dev, chip);
> +
> + return chip;
> +}
> +EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
> +
> +/*
> + * tpm_chip_register() - create a misc driver for the TPM chip
> + * @chip: TPM chip to use.
> + *
> + * Creates a misc driver for the TPM chip and adds sysfs interfaces for
> + * the device, PPI and TCPA. As the last step this function adds the
> + * chip to the list of TPM chips available for use.
> + *
> + * NOTE: This function should be only called after the chip initialization
> + * is complete.
> + *
> + * 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
> + * upon errant exit from this function specific probe function should call
> + * pci_disable_device
> + */
> +int tpm_chip_register(struct tpm_chip *chip)
> +{
> + int rc;
> +
> + rc = tpm_dev_add_device(chip);
> + if (rc)
> + return rc;
> +
> + rc = tpm_sysfs_add_device(chip);
> + if (rc)
> + goto del_misc;
> +
> + rc = tpm_add_ppi(&chip->dev->kobj);
> + if (rc)
> + goto del_sysfs;
> +
> + chip->bios_dir = tpm_bios_log_setup(chip->devname);
> +
> + /* Make the chip available. */
> + spin_lock(&driver_lock);
> + list_add_rcu(&chip->list, &tpm_chip_list);
> + spin_unlock(&driver_lock);
> +
> + return 0;
> +del_sysfs:
> + tpm_sysfs_del_device(chip);
> +del_misc:
> + tpm_dev_del_device(chip);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(tpm_chip_register);
> +
> +/*
> + * tpm_chip_unregister() - release the TPM driver
> + * @chip: TPM chip to use.
> + *
> + * Takes the chip first away from the list of available TPM chips and then
> + * cleans up all the resources reserved by tpm_chip_register().
> + *
> + * NOTE: This function should be only called before deinitializing chip
> + * resources.
> + */
> +void tpm_chip_unregister(struct tpm_chip *chip)
> +{
> + spin_lock(&driver_lock);
> + list_del_rcu(&chip->list);
> + spin_unlock(&driver_lock);
> + synchronize_rcu();
> +
> + tpm_sysfs_del_device(chip);
> + tpm_remove_ppi(&chip->dev->kobj);
> +
> + if (chip->bios_dir)
> + tpm_bios_log_teardown(chip->bios_dir);
> +
> + tpm_dev_del_device(chip);
> +}
> +EXPORT_SYMBOL_GPL(tpm_chip_unregister);
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 0150b7c..915c610 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1,5 +1,6 @@
> /*
> * Copyright (C) 2004 IBM Corporation
> + * Copyright (C) 2014 Intel Corporation
> *
> * Authors:
> * Leendert van Doorn <leendert@...son.ibm.com>
> @@ -47,10 +48,6 @@ module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
> MODULE_PARM_DESC(suspend_pcr,
> "PCR to use for dummy writes to faciltate flush on suspend.");
>
> -static LIST_HEAD(tpm_chip_list);
> -static DEFINE_SPINLOCK(driver_lock);
> -static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> -
> /*
> * Array with one entry per ordinal defining the maximum amount
> * of time the chip could take to return the result. The ordinal
> @@ -639,27 +636,6 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
> return rc;
> }
>
> -/*
> - * tpm_chip_find_get - return tpm_chip for given chip number
> - */
> -static struct tpm_chip *tpm_chip_find_get(int chip_num)
> -{
> - struct tpm_chip *pos, *chip = NULL;
> -
> - rcu_read_lock();
> - list_for_each_entry_rcu(pos, &tpm_chip_list, list) {
> - if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
> - continue;
> -
> - if (try_module_get(pos->dev->driver->owner)) {
> - chip = pos;
> - break;
> - }
> - }
> - rcu_read_unlock();
> - return chip;
> -}
> -
> #define TPM_ORDINAL_PCRREAD cpu_to_be32(21)
> #define READ_PCR_RESULT_SIZE 30
> static struct tpm_input_header pcrread_header = {
> @@ -887,30 +863,6 @@ again:
> }
> EXPORT_SYMBOL_GPL(wait_for_tpm_stat);
>
> -void tpm_remove_hardware(struct device *dev)
> -{
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> -
> - if (chip == NULL) {
> - dev_err(dev, "No device data found\n");
> - return;
> - }
> -
> - spin_lock(&driver_lock);
> - list_del_rcu(&chip->list);
> - spin_unlock(&driver_lock);
> - synchronize_rcu();
> -
> - tpm_dev_del_device(chip);
> - tpm_sysfs_del_device(chip);
> - tpm_remove_ppi(&dev->kobj);
> - tpm_bios_log_teardown(chip->bios_dir);
> -
> - /* write it this way to be explicit (chip->dev == dev) */
> - put_device(chip->dev);
> -}
> -EXPORT_SYMBOL_GPL(tpm_remove_hardware);
> -
> #define TPM_ORD_SAVESTATE cpu_to_be32(152)
> #define SAVESTATE_RESULT_SIZE 10
>
> @@ -1044,104 +996,6 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> }
> EXPORT_SYMBOL_GPL(tpm_get_random);
>
> -/* In case vendor provided release function, call it too.*/
> -
> -void tpm_dev_vendor_release(struct tpm_chip *chip)
> -{
> - if (!chip)
> - return;
> -
> - clear_bit(chip->dev_num, dev_mask);
> -}
> -EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
> -
> -
> -/*
> - * Once all references to platform device are down to 0,
> - * release all allocated structures.
> - */
> -static void tpm_dev_release(struct device *dev)
> -{
> - struct tpm_chip *chip = dev_get_drvdata(dev);
> -
> - if (!chip)
> - return;
> -
> - tpm_dev_vendor_release(chip);
> -
> - chip->release(dev);
> - 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
> - * upon errant exit from this function specific probe function should call
> - * pci_disable_device
> - */
> -struct tpm_chip *tpm_register_hardware(struct device *dev,
> - const struct tpm_class_ops *ops)
> -{
> - struct tpm_chip *chip;
> -
> - /* Driver specific per-device data */
> - chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> -
> - if (chip == NULL)
> - return NULL;
> -
> - mutex_init(&chip->tpm_mutex);
> - INIT_LIST_HEAD(&chip->list);
> -
> - chip->ops = ops;
> - chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> -
> - if (chip->dev_num >= TPM_NUM_DEVICES) {
> - dev_err(dev, "No available tpm device numbers\n");
> - goto out_free;
> - }
> -
> - set_bit(chip->dev_num, dev_mask);
> -
> - scnprintf(chip->devname, sizeof(chip->devname), "%s%d", "tpm",
> - chip->dev_num);
> -
> - chip->dev = get_device(dev);
> - chip->release = dev->release;
> - dev->release = tpm_dev_release;
> - dev_set_drvdata(dev, chip);
> -
> - if (tpm_dev_add_device(chip))
> - goto put_device;
> -
> - if (tpm_sysfs_add_device(chip))
> - goto del_misc;
> -
> - if (tpm_add_ppi(&dev->kobj))
> - goto del_sysfs;
> -
> - chip->bios_dir = tpm_bios_log_setup(chip->devname);
> -
> - /* Make chip available */
> - spin_lock(&driver_lock);
> - list_add_rcu(&chip->list, &tpm_chip_list);
> - spin_unlock(&driver_lock);
> -
> - return chip;
> -
> -del_sysfs:
> - tpm_sysfs_del_device(chip);
> -del_misc:
> - tpm_dev_del_device(chip);
> -put_device:
> - put_device(chip->dev);
> -out_free:
> - kfree(chip);
> - return NULL;
> -}
> -EXPORT_SYMBOL_GPL(tpm_register_hardware);
> -
> MODULE_AUTHOR("Leendert van Doorn (leendert@...son.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 e638eb0..9880681 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -110,7 +110,6 @@ 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)
> @@ -322,15 +321,17 @@ extern int tpm_get_timeouts(struct tpm_chip *);
> extern void tpm_gen_interrupt(struct tpm_chip *);
> extern int tpm_do_selftest(struct tpm_chip *);
> extern unsigned long tpm_calc_ordinal_duration(struct tpm_chip *, u32);
> -extern struct tpm_chip* tpm_register_hardware(struct device *,
> - const struct tpm_class_ops *ops);
> -extern void tpm_dev_vendor_release(struct tpm_chip *);
> -extern void tpm_remove_hardware(struct device *);
> extern int tpm_pm_suspend(struct device *);
> extern int tpm_pm_resume(struct device *);
> extern int wait_for_tpm_stat(struct tpm_chip *, u8, unsigned long,
> wait_queue_head_t *, bool);
>
> +struct tpm_chip *tpm_chip_find_get(int chip_num);
> +extern struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> + const struct tpm_class_ops *ops);
> +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);
> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> index 6069d13..8e2576a 100644
> --- a/drivers/char/tpm/tpm_atmel.c
> +++ b/drivers/char/tpm/tpm_atmel.c
> @@ -138,11 +138,11 @@ static void atml_plat_remove(void)
> struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
>
> if (chip) {
> + tpm_chip_unregister(chip);
> if (chip->vendor.have_region)
> atmel_release_region(chip->vendor.base,
> chip->vendor.region_size);
> atmel_put_base_addr(chip->vendor.iobase);
> - tpm_remove_hardware(chip->dev);
> platform_device_unregister(pdev);
> }
> }
> @@ -184,8 +184,9 @@ static int __init init_atmel(void)
> goto err_rel_reg;
> }
>
> - if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_atmel))) {
> - rc = -ENODEV;
> + chip = tpmm_chip_alloc(&pdev->dev, &tpm_atmel);
> + if (IS_ERR(chip)) {
> + rc = PTR_ERR(chip);
> goto err_unreg_dev;
> }
>
> @@ -194,6 +195,10 @@ static int __init init_atmel(void)
> chip->vendor.have_region = have_region;
> chip->vendor.region_size = region_size;
>
> + rc = tpm_chip_register(chip);
> + if (rc)
> + goto err_unreg_dev;
> +
> return 0;
>
> err_unreg_dev:
> diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
> index 7727292..8af3b4a 100644
> --- a/drivers/char/tpm/tpm_i2c_atmel.c
> +++ b/drivers/char/tpm/tpm_i2c_atmel.c
> @@ -160,11 +160,9 @@ static int i2c_atmel_probe(struct i2c_client *client,
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> return -ENODEV;
>
> - chip = tpm_register_hardware(dev, &i2c_atmel);
> - if (!chip) {
> - dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
> - return -ENODEV;
> - }
> + chip = tpmm_chip_alloc(dev, &i2c_atmel);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
> GFP_KERNEL);
> @@ -179,21 +177,16 @@ static int i2c_atmel_probe(struct i2c_client *client,
> /* There is no known way to probe for this device, and all version
> * information seems to be read via TPM commands. Thus we rely on the
> * TPM startup process in the common code to detect the device. */
> - if (tpm_get_timeouts(chip)) {
> - rc = -ENODEV;
> - goto out_err;
> - }
> + if (tpm_get_timeouts(chip))
> + return -ENODEV;
>
> - if (tpm_do_selftest(chip)) {
> - rc = -ENODEV;
> - goto out_err;
> - }
> + if (tpm_do_selftest(chip))
> + return -ENODEV;
>
> - return 0;
> + rc = tpm_chip_register(chip);
> + if (rc)
> + return rc;
>
> -out_err:
> - tpm_dev_vendor_release(chip);
> - tpm_remove_hardware(chip->dev);
> return rc;
> }
>
> @@ -201,11 +194,7 @@ static int i2c_atmel_remove(struct i2c_client *client)
> {
> struct device *dev = &(client->dev);
> struct tpm_chip *chip = dev_get_drvdata(dev);
> -
> - if (chip)
> - tpm_dev_vendor_release(chip);
> - tpm_remove_hardware(dev);
> - kfree(chip);
> + tpm_chip_unregister(chip);
> return 0;
> }
>
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 472af4b..03708e6 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -581,12 +581,9 @@ static int tpm_tis_i2c_init(struct device *dev)
> int rc = 0;
> struct tpm_chip *chip;
>
> - chip = tpm_register_hardware(dev, &tpm_tis_i2c);
> - if (!chip) {
> - dev_err(dev, "could not register hardware\n");
> - rc = -ENODEV;
> - goto out_err;
> - }
> + chip = tpmm_chip_alloc(dev, &tpm_tis_i2c);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> /* Disable interrupts */
> chip->vendor.irq = 0;
> @@ -600,7 +597,7 @@ static int tpm_tis_i2c_init(struct device *dev)
> if (request_locality(chip, 0) != 0) {
> dev_err(dev, "could not request locality\n");
> rc = -ENODEV;
> - goto out_vendor;
> + goto out_err;
> }
>
> /* read four bytes from DID_VID register */
> @@ -628,21 +625,9 @@ static int tpm_tis_i2c_init(struct device *dev)
> tpm_get_timeouts(chip);
> tpm_do_selftest(chip);
>
> - return 0;
> -
> + return tpm_chip_register(chip);
> out_release:
> release_locality(chip, chip->vendor.locality, 1);
> -
> -out_vendor:
> - /* close file handles */
> - tpm_dev_vendor_release(chip);
> -
> - /* remove hardware */
> - tpm_remove_hardware(chip->dev);
> -
> - /* reset these pointers, otherwise we oops */
> - chip->dev->release = NULL;
> - chip->release = NULL;
> tpm_dev.client = NULL;
> out_err:
> return rc;
> @@ -712,17 +697,9 @@ static int tpm_tis_i2c_probe(struct i2c_client *client,
> static int tpm_tis_i2c_remove(struct i2c_client *client)
> {
> struct tpm_chip *chip = tpm_dev.chip;
> - release_locality(chip, chip->vendor.locality, 1);
>
> - /* close file handles */
> - tpm_dev_vendor_release(chip);
> -
> - /* remove hardware */
> - tpm_remove_hardware(chip->dev);
> -
> - /* reset these pointers, otherwise we oops */
> - chip->dev->release = NULL;
> - chip->release = NULL;
> + tpm_chip_unregister(chip);
> + release_locality(chip, chip->vendor.locality, 1);
> tpm_dev.client = NULL;
>
> return 0;
> diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
> index 7b158ef..09f0c46 100644
> --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> @@ -530,11 +530,9 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
> dev_info(dev, "VID: %04X DID: %02X RID: %02X\n", (u16) vid,
> (u8) (vid >> 16), (u8) (vid >> 24));
>
> - chip = tpm_register_hardware(dev, &tpm_i2c);
> - if (!chip) {
> - dev_err(dev, "%s() error in tpm_register_hardware\n", __func__);
> - return -ENODEV;
> - }
> + chip = tpmm_chip_alloc(dev, &tpm_i2c);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> chip->vendor.priv = devm_kzalloc(dev, sizeof(struct priv_data),
> GFP_KERNEL);
> @@ -584,7 +582,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
> TPM_DATA_FIFO_W,
> 1, (u8 *) (&rc));
> if (rc < 0)
> - goto out_err;
> + return rc;
> /* TPM_STS <- 0x40 (commandReady) */
> i2c_nuvoton_ready(chip);
> } else {
> @@ -594,45 +592,33 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
> * only TPM_STS_VALID should be set
> */
> if (i2c_nuvoton_read_status(chip) !=
> - TPM_STS_VALID) {
> - rc = -EIO;
> - goto out_err;
> - }
> + TPM_STS_VALID)
> + return -EIO;
> }
> }
> }
>
> - if (tpm_get_timeouts(chip)) {
> - rc = -ENODEV;
> - goto out_err;
> - }
> + if (tpm_get_timeouts(chip))
> + return -ENODEV;
>
> - if (tpm_do_selftest(chip)) {
> - rc = -ENODEV;
> - goto out_err;
> - }
> + if (tpm_do_selftest(chip))
> + return -ENODEV;
>
> - return 0;
> + rc = tpm_chip_register(chip);
> + if (rc)
> + return rc;
>
> -out_err:
> - tpm_dev_vendor_release(chip);
> - tpm_remove_hardware(chip->dev);
> - return rc;
> + return 0;
> }
>
> static int i2c_nuvoton_remove(struct i2c_client *client)
> {
> struct device *dev = &(client->dev);
> struct tpm_chip *chip = dev_get_drvdata(dev);
> -
> - if (chip)
> - tpm_dev_vendor_release(chip);
> - tpm_remove_hardware(dev);
> - kfree(chip);
> + tpm_chip_unregister(chip);
> return 0;
> }
>
> -
> static const struct i2c_device_id i2c_nuvoton_id[] = {
> {I2C_DRIVER_NAME, 0},
> {}
> diff --git a/drivers/char/tpm/tpm_i2c_stm_st33.c b/drivers/char/tpm/tpm_i2c_stm_st33.c
> index 4669e37..b9d1a38 100644
> --- a/drivers/char/tpm/tpm_i2c_stm_st33.c
> +++ b/drivers/char/tpm/tpm_i2c_stm_st33.c
> @@ -609,37 +609,29 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
> if (client == NULL) {
> pr_info("%s: i2c client is NULL. Device not accessible.\n",
> __func__);
> - err = -ENODEV;
> - goto end;
> + return -ENODEV;
> }
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> dev_info(&client->dev, "client not i2c capable\n");
> - err = -ENODEV;
> - goto end;
> + return -ENODEV;
> }
>
> - chip = tpm_register_hardware(&client->dev, &st_i2c_tpm);
> - if (!chip) {
> - dev_info(&client->dev, "fail chip\n");
> - err = -ENODEV;
> - goto end;
> - }
> + chip = tpmm_chip_alloc(&client->dev, &st_i2c_tpm);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> platform_data = client->dev.platform_data;
>
> if (!platform_data) {
> dev_info(&client->dev, "chip not available\n");
> - err = -ENODEV;
> - goto _tpm_clean_answer;
> + return -ENODEV;
> }
>
> platform_data->tpm_i2c_buffer[0] =
> kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> - if (platform_data->tpm_i2c_buffer[0] == NULL) {
> - err = -ENOMEM;
> - goto _tpm_clean_answer;
> - }
> + if (platform_data->tpm_i2c_buffer[0] == NULL)
> + return -ENOMEM;
> platform_data->tpm_i2c_buffer[1] =
> kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
> if (platform_data->tpm_i2c_buffer[1] == NULL) {
> @@ -716,8 +708,10 @@ tpm_st33_i2c_probe(struct i2c_client *client, const struct i2c_device_id *id)
> tpm_get_timeouts(chip);
> tpm_do_selftest(chip);
>
> - dev_info(chip->dev, "TPM I2C Initialized\n");
> - return 0;
> + err = tpm_chip_register(chip);
> + if (!err)
> + return 0;
> +
> _irq_set:
> free_irq(gpio_to_irq(platform_data->io_serirq), (void *)chip);
> _gpio_init2:
> @@ -732,10 +726,6 @@ _tpm_clean_response2:
> _tpm_clean_response1:
> kzfree(platform_data->tpm_i2c_buffer[0]);
> platform_data->tpm_i2c_buffer[0] = NULL;
> -_tpm_clean_answer:
> - tpm_remove_hardware(chip->dev);
> -end:
> - pr_info("TPM I2C initialisation fail\n");
> return err;
> }
>
> @@ -752,13 +742,13 @@ static int tpm_st33_i2c_remove(struct i2c_client *client)
> ((struct i2c_client *)TPM_VPRIV(chip))->dev.platform_data;
>
> if (pin_infos != NULL) {
> + tpm_chip_unregister(chip);
> +
> free_irq(pin_infos->io_serirq, chip);
>
> gpio_free(pin_infos->io_serirq);
> gpio_free(pin_infos->io_lpcpd);
>
> - tpm_remove_hardware(chip->dev);
> -
> if (pin_infos->tpm_i2c_buffer[1] != NULL) {
> kzfree(pin_infos->tpm_i2c_buffer[1]);
> pin_infos->tpm_i2c_buffer[1] = NULL;
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index af74c57..eb95796 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -270,8 +270,11 @@ static int ibmvtpm_crq_send_init(struct ibmvtpm_dev *ibmvtpm)
> static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
> {
> struct ibmvtpm_dev *ibmvtpm = ibmvtpm_get_data(&vdev->dev);
> + struct tpm_chip *chip = dev_get_drvdata(ibmvtpm->dev);
> int rc = 0;
>
> + tpm_chip_unregister(chip);
> +
> free_irq(vdev->irq, ibmvtpm);
>
> do {
> @@ -290,8 +293,6 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev)
> kfree(ibmvtpm->rtce_buf);
> }
>
> - tpm_remove_hardware(ibmvtpm->dev);
> -
> kfree(ibmvtpm);
>
> return 0;
> @@ -555,11 +556,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> struct tpm_chip *chip;
> int rc = -ENOMEM, rc1;
>
> - chip = tpm_register_hardware(dev, &tpm_ibmvtpm);
> - if (!chip) {
> - dev_err(dev, "tpm_register_hardware failed\n");
> - return -ENODEV;
> - }
> + chip = tpmm_chip_alloc(dev, &tpm_ibmvtpm);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> ibmvtpm = kzalloc(sizeof(struct ibmvtpm_dev), GFP_KERNEL);
> if (!ibmvtpm) {
> @@ -629,7 +628,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> if (rc)
> goto init_irq_cleanup;
>
> - return rc;
> + return tpm_chip_register(chip);
> init_irq_cleanup:
> do {
> rc1 = plpar_hcall_norets(H_FREE_CRQ, vio_dev->unit_address);
> @@ -644,8 +643,6 @@ cleanup:
> kfree(ibmvtpm);
> }
>
> - tpm_remove_hardware(dev);
> -
> return rc;
> }
>
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index dc0a255..dcdb671 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -546,7 +546,14 @@ static int tpm_inf_pnp_probe(struct pnp_dev *dev,
> vendorid[0], vendorid[1],
> productid[0], productid[1], chipname);
>
> - if (!(chip = tpm_register_hardware(&dev->dev, &tpm_inf)))
> + chip = tpmm_chip_alloc(&dev->dev, &tpm_inf);
> + if (IS_ERR(chip)) {
> + rc = PTR_ERR(chip);
> + goto err_release_region;
> + }
> +
> + rc = tpm_chip_register(chip);
> + if (rc)
> goto err_release_region;
>
> return 0;
> @@ -572,17 +579,15 @@ static void tpm_inf_pnp_remove(struct pnp_dev *dev)
> {
> struct tpm_chip *chip = pnp_get_drvdata(dev);
>
> - if (chip) {
> - if (tpm_dev.iotype == TPM_INF_IO_PORT) {
> - release_region(tpm_dev.data_regs, tpm_dev.data_size);
> - release_region(tpm_dev.config_port,
> - tpm_dev.config_size);
> - } else {
> - iounmap(tpm_dev.mem_base);
> - release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
> - }
> - tpm_dev_vendor_release(chip);
> - tpm_remove_hardware(chip->dev);
> + tpm_chip_unregister(chip);
> +
> + if (tpm_dev.iotype == TPM_INF_IO_PORT) {
> + release_region(tpm_dev.data_regs, tpm_dev.data_size);
> + release_region(tpm_dev.config_port,
> + tpm_dev.config_size);
> + } else {
> + iounmap(tpm_dev.mem_base);
> + release_mem_region(tpm_dev.map_base, tpm_dev.map_size);
> }
> }
>
> diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
> index 3179ec9..00c5470 100644
> --- a/drivers/char/tpm/tpm_nsc.c
> +++ b/drivers/char/tpm/tpm_nsc.c
> @@ -247,10 +247,9 @@ static struct platform_device *pdev = NULL;
> static void tpm_nsc_remove(struct device *dev)
> {
> struct tpm_chip *chip = dev_get_drvdata(dev);
> - if ( chip ) {
> - release_region(chip->vendor.base, 2);
> - tpm_remove_hardware(chip->dev);
> - }
> +
> + tpm_chip_unregister(chip);
> + release_region(chip->vendor.base, 2);
> }
>
> static SIMPLE_DEV_PM_OPS(tpm_nsc_pm, tpm_pm_suspend, tpm_pm_resume);
> @@ -308,11 +307,16 @@ static int __init init_nsc(void)
> goto err_del_dev;
> }
>
> - if (!(chip = tpm_register_hardware(&pdev->dev, &tpm_nsc))) {
> + chip = tpmm_chip_alloc(&pdev->dev, &tpm_nsc);
> + if (IS_ERR(chip)) {
> rc = -ENODEV;
> goto err_rel_reg;
> }
>
> + rc = tpm_chip_register(chip);
> + if (rc)
> + goto err_rel_reg;
> +
> dev_dbg(&pdev->dev, "NSC TPM detected\n");
> dev_dbg(&pdev->dev,
> "NSC LDN 0x%x, SID 0x%x, SRID 0x%x\n",
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 2c46734..0066b68 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -75,9 +75,6 @@ enum tis_defaults {
> #define TPM_DID_VID(l) (0x0F00 | ((l) << 12))
> #define TPM_RID(l) (0x0F04 | ((l) << 12))
>
> -static LIST_HEAD(tis_chips);
> -static DEFINE_MUTEX(tis_lock);
> -
> #if defined(CONFIG_PNP) && defined(CONFIG_ACPI)
> static int is_itpm(struct pnp_dev *dev)
> {
> @@ -528,6 +525,17 @@ static bool interrupts = true;
> module_param(interrupts, bool, 0444);
> MODULE_PARM_DESC(interrupts, "Enable interrupts");
>
> +static void tpm_tis_remove(struct tpm_chip *chip)
> +{
> + iowrite32(~TPM_GLOBAL_INT_ENABLE &
> + ioread32(chip->vendor.iobase +
> + TPM_INT_ENABLE(chip->vendor.
> + locality)),
> + chip->vendor.iobase +
> + TPM_INT_ENABLE(chip->vendor.locality));
> + release_locality(chip, chip->vendor.locality, 1);
> +}
> +
> static int tpm_tis_init(struct device *dev, resource_size_t start,
> resource_size_t len, unsigned int irq)
> {
> @@ -535,14 +543,13 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
> int rc, i, irq_s, irq_e, probe;
> struct tpm_chip *chip;
>
> - if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
> - return -ENODEV;
> + chip = tpmm_chip_alloc(dev, &tpm_tis);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> - chip->vendor.iobase = ioremap(start, len);
> - if (!chip->vendor.iobase) {
> - rc = -EIO;
> - goto out_err;
> - }
> + chip->vendor.iobase = devm_ioremap(dev, start, len);
> + if (!chip->vendor.iobase)
> + return -EIO;
>
> /* Default timeouts */
> chip->vendor.timeout_a = msecs_to_jiffies(TIS_SHORT_TIMEOUT);
> @@ -649,8 +656,8 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
> for (i = irq_s; i <= irq_e && chip->vendor.irq == 0; i++) {
> iowrite8(i, chip->vendor.iobase +
> TPM_INT_VECTOR(chip->vendor.locality));
> - if (request_irq
> - (i, tis_int_probe, IRQF_SHARED,
> + if (devm_request_irq
> + (dev, i, tis_int_probe, IRQF_SHARED,
> chip->vendor.miscdev.name, chip) != 0) {
> dev_info(chip->dev,
> "Unable to request irq: %d for probe\n",
> @@ -690,15 +697,14 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
> iowrite32(intmask,
> chip->vendor.iobase +
> TPM_INT_ENABLE(chip->vendor.locality));
> - free_irq(i, chip);
> }
> }
> if (chip->vendor.irq) {
> iowrite8(chip->vendor.irq,
> chip->vendor.iobase +
> TPM_INT_VECTOR(chip->vendor.locality));
> - if (request_irq
> - (chip->vendor.irq, tis_int_handler, IRQF_SHARED,
> + if (devm_request_irq
> + (dev, chip->vendor.irq, tis_int_handler, IRQF_SHARED,
> chip->vendor.miscdev.name, chip) != 0) {
> dev_info(chip->dev,
> "Unable to request irq: %d for use\n",
> @@ -719,17 +725,9 @@ static int tpm_tis_init(struct device *dev, resource_size_t start,
> }
> }
>
> - INIT_LIST_HEAD(&chip->vendor.list);
> - mutex_lock(&tis_lock);
> - list_add(&chip->vendor.list, &tis_chips);
> - mutex_unlock(&tis_lock);
> -
> -
> - return 0;
> + return tpm_chip_register(chip);
> out_err:
> - if (chip->vendor.iobase)
> - iounmap(chip->vendor.iobase);
> - tpm_remove_hardware(chip->dev);
> + tpm_tis_remove(chip);
> return rc;
> }
>
> @@ -811,13 +809,10 @@ MODULE_DEVICE_TABLE(pnp, tpm_pnp_tbl);
> static void tpm_tis_pnp_remove(struct pnp_dev *dev)
> {
> struct tpm_chip *chip = pnp_get_drvdata(dev);
> -
> - tpm_dev_vendor_release(chip);
> -
> - kfree(chip);
> + tpm_chip_unregister(chip);
> + tpm_tis_remove(chip);
> }
>
> -
> static struct pnp_driver tis_pnp_driver = {
> .name = "tpm_tis",
> .id_table = tpm_pnp_tbl,
> @@ -836,7 +831,7 @@ MODULE_PARM_DESC(hid, "Set additional specific HID for this driver to probe");
>
> static struct platform_driver tis_drv = {
> .driver = {
> - .name = "tpm_tis",
> + .name = "tpm_tis",
> .owner = THIS_MODULE,
> .pm = &tpm_tis_pm,
> },
> @@ -876,31 +871,16 @@ err_dev:
>
> static void __exit cleanup_tis(void)
> {
> - struct tpm_vendor_specific *i, *j;
> struct tpm_chip *chip;
> - mutex_lock(&tis_lock);
> - list_for_each_entry_safe(i, j, &tis_chips, list) {
> - chip = to_tpm_chip(i);
> - tpm_remove_hardware(chip->dev);
> - iowrite32(~TPM_GLOBAL_INT_ENABLE &
> - ioread32(chip->vendor.iobase +
> - TPM_INT_ENABLE(chip->vendor.
> - locality)),
> - chip->vendor.iobase +
> - TPM_INT_ENABLE(chip->vendor.locality));
> - release_locality(chip, chip->vendor.locality, 1);
> - if (chip->vendor.irq)
> - free_irq(chip->vendor.irq, chip);
> - iounmap(i->iobase);
> - list_del(&i->list);
> - }
> - mutex_unlock(&tis_lock);
> #ifdef CONFIG_PNP
> if (!force) {
> pnp_unregister_driver(&tis_pnp_driver);
> return;
> }
> #endif
> + chip = dev_get_drvdata(&pdev->dev);
> + tpm_chip_unregister(chip);
> + tpm_tis_remove(chip);
> platform_device_unregister(pdev);
> platform_driver_unregister(&tis_drv);
> }
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 441b44e..c3b4f5a 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -175,9 +175,9 @@ static int setup_chip(struct device *dev, struct tpm_private *priv)
> {
> struct tpm_chip *chip;
>
> - chip = tpm_register_hardware(dev, &tpm_vtpm);
> - if (!chip)
> - return -ENODEV;
> + chip = tpmm_chip_alloc(dev, &tpm_vtpm);
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
>
> init_waitqueue_head(&chip->vendor.read_queue);
>
> @@ -286,6 +286,7 @@ static int tpmfront_probe(struct xenbus_device *dev,
> const struct xenbus_device_id *id)
> {
> struct tpm_private *priv;
> + struct tpm_chip *chip;
> int rv;
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> @@ -302,21 +303,22 @@ static int tpmfront_probe(struct xenbus_device *dev,
>
> rv = setup_ring(dev, priv);
> if (rv) {
> - tpm_remove_hardware(&dev->dev);
> + chip = dev_get_drvdata(&dev->dev);
> + tpm_chip_unregister(chip);
> ring_free(priv);
> return rv;
> }
>
> tpm_get_timeouts(priv->chip);
>
> - return rv;
> + return tpm_chip_register(priv->chip);
> }
>
> static int tpmfront_remove(struct xenbus_device *dev)
> {
> struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
> struct tpm_private *priv = TPM_VPRIV(chip);
> - tpm_remove_hardware(&dev->dev);
> + tpm_chip_unregister(chip);
> ring_free(priv);
> TPM_VPRIV(chip) = NULL;
> return 0;
Reviewed-by: Stefan Berger <stefanb@...ux.vnet.ibm.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists