Re: [PATCH 4/5] TPM: addition of pnp_remove()

[Rajiv Andrade <srajiv@linux.vnet.ibm.com>]

Unfortunately we don't have an old 1.1 infineon tpm chip in order to
test this legacy driver, but probably we'd need to add a call to
tpm_dev_vendor_release() in tpm_infineon.c

Marcel, do you have any clue?

On Fri, 2008-10-03 at 15:05 -0500, Serge E. Hallyn wrote:
> Quoting Rajiv Andrade (srajiv@linux.vnet.ibm.com):
> > The tpm_dev_release function is only called for platform devices, not pnp devices, so we
> > implemented the .remove function for pnp ones.
> > Since it's code is very similar to the one inside tpm_dev_release, we've created a helper
> > function tpm_dev_vendor_release, which is called by both.
> 
> Should tpm_infineon also be switched over to this?
> 
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Signed-off-by: Rajiv Andrade <srajiv@linux.vnet.ibm.com>
> > ---
> >  drivers/char/tpm/tpm.c     |   22 ++++++++++++++++------
> >  drivers/char/tpm/tpm.h     |    1 +
> >  drivers/char/tpm/tpm_tis.c |   14 +++++++++++++-
> >  3 files changed, 30 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> > index 24fb7ab..ab03b4d 100644
> > --- a/drivers/char/tpm/tpm.c
> > +++ b/drivers/char/tpm/tpm.c
> > @@ -1133,23 +1133,33 @@ int tpm_pm_resume(struct device *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(tpm_pm_resume);
> > 
> > +/* In case vendor provided release function, call it too.*/
> > +
> > +void tpm_dev_vendor_release(struct tpm_chip *chip)
> > +{
> > +	if (chip->vendor.release)
> > +	 	chip->vendor.release(chip->dev);
> > +	
> > +	clear_bit(chip->dev_num, dev_mask);
> > +	kfree(chip->vendor.miscdev.name);
> > +}
> > +EXPORT_SYMBOL_GPL(tpm_dev_vendor_release);
> > +
> > +
> >  /*
> >   * 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);
> > 
> > -	if (chip->vendor.release)
> > -		chip->vendor.release(dev);
> > -	chip->release(dev);
> > +	tpm_dev_vendor_release(chip);
> > 
> > -	clear_bit(chip->dev_num, dev_mask);
> > -	kfree(chip->vendor.miscdev.name);
> > +	chip->release(dev);
> >  	kfree(chip);
> >  }
> > +EXPORT_SYMBOL_GPL(tpm_dev_release);
> > 
> >  /*
> >   * Called from tpm_<specific>.c probe function only for devices 
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 2756cab..8e30df4 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -132,6 +132,7 @@ extern struct tpm_chip* tpm_register_hardware(struct device *,
> >  				 const struct tpm_vendor_specific *);
> >  extern int tpm_open(struct inode *, struct file *);
> >  extern int tpm_release(struct inode *, struct file *);
> > +extern void tpm_dev_vendor_release(struct tpm_chip *);
> >  extern ssize_t tpm_write(struct file *, const char __user *, size_t,
> >  			 loff_t *);
> >  extern ssize_t tpm_read(struct file *, char __user *, size_t, loff_t *);
> > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> > index ed1879c..3491d70 100644
> > --- a/drivers/char/tpm/tpm_tis.c
> > +++ b/drivers/char/tpm/tpm_tis.c
> > @@ -630,12 +630,23 @@ static struct pnp_device_id tpm_pnp_tbl[] __devinitdata = {
> >  	{"", 0}			/* Terminator */
> >  };
> > 
> > +static __devexit void tpm_tis_pnp_remove(struct pnp_dev *dev)
> > +{
> > +	struct tpm_chip *chip = pnp_get_drvdata(dev);
> > +	
> > +	tpm_dev_vendor_release(chip);
> > +
> > +	kfree(chip);
> > +}
> > +
> > +
> >  static struct pnp_driver tis_pnp_driver = {
> >  	.name = "tpm_tis",
> >  	.id_table = tpm_pnp_tbl,
> >  	.probe = tpm_tis_pnp_init,
> >  	.suspend = tpm_tis_pnp_suspend,
> >  	.resume = tpm_tis_pnp_resume,
> > +	.remove = tpm_tis_pnp_remove,
> >  };
> > 
> >  #define TIS_HID_USR_IDX sizeof(tpm_pnp_tbl)/sizeof(struct pnp_device_id) -2
> > @@ -683,6 +694,7 @@ static void __exit cleanup_tis(void)
> >  	spin_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.
> > @@ -694,9 +706,9 @@ static void __exit cleanup_tis(void)
> >  			free_irq(chip->vendor.irq, chip);
> >  		iounmap(i->iobase);
> >  		list_del(&i->list);
> > -		tpm_remove_hardware(chip->dev);
> >  	}
> >  	spin_unlock(&tis_lock);
> > +	
> >  	if (force) {
> >  		platform_device_unregister(pdev);
> >  		driver_unregister(&tis_drv);
> > -- 
> > 1.5.6.3