[PATCH 0/3] Various struct device cleanups

[PATCH 0/3] Various struct device cleanups

[Jason Gunthorpe]

These little clean ups were missed during the struct device conversion of
tpm_chip. There were noticed when looking at Stefan's vtpm patch sets.

Nothing very significant
 - Add some missing krefs
 - Replace chip->devname with dev_name
 - Replace chip->pdev with chip->dev.parent

Jason Gunthorpe (3):
  tpm: Hold the kref during tpm_chip_find_get
  tpm: Get rid of chip->pdev
  tpm: Get rid of devname

 drivers/char/tpm/tpm-chip.c         | 34 +++++++++++++++++++---------------
 drivers/char/tpm/tpm-dev.c          |  4 +---
 drivers/char/tpm/tpm-interface.c    | 30 ++++++++++++++++--------------
 drivers/char/tpm/tpm-sysfs.c        |  6 +++---
 drivers/char/tpm/tpm.h              |  5 ++---
 drivers/char/tpm/tpm2-cmd.c         |  8 ++++----
 drivers/char/tpm/tpm_atmel.c        | 14 +++++++-------
 drivers/char/tpm/tpm_eventlog.c     |  2 +-
 drivers/char/tpm/tpm_eventlog.h     |  2 +-
 drivers/char/tpm/tpm_i2c_atmel.c    | 16 ++++++++--------
 drivers/char/tpm/tpm_i2c_infineon.c |  6 +++---
 drivers/char/tpm/tpm_i2c_nuvoton.c  | 28 ++++++++++++++--------------
 drivers/char/tpm/tpm_infineon.c     | 22 +++++++++++-----------
 drivers/char/tpm/tpm_nsc.c          | 20 ++++++++++----------
 drivers/char/tpm/tpm_tis.c          | 18 +++++++++---------
 15 files changed, 109 insertions(+), 106 deletions(-)

-- 
2.1.4

[PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jason Gunthorpe]

This was missed during the struct device conversion, we
need to hold a kref on the chip to make sure it isn't freed.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c | 2 ++
 drivers/char/tpm/tpm.h      | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 45cc39aabeee..ae2fed8a162b 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
 			chip = pos;
 			break;
 		}
+
+		get_device(&chip->dev);
 	}
 	rcu_read_unlock();
 	return chip;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 542a80cbfd9c..f6ba79d91857 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -207,6 +207,7 @@ struct tpm_chip {
 static inline void tpm_chip_put(struct tpm_chip *chip)
 {
 	module_put(chip->pdev->driver->owner);
+	put_device(&chip->dev);
 }
 
 static inline int tpm_read_index(int base, int index)
-- 
2.1.4

Re: [PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jarkko Sakkinen]

On Fri, Feb 12, 2016 at 05:04:29PM -0700, Jason Gunthorpe wrote:
> This was missed during the struct device conversion, we
> need to hold a kref on the chip to make sure it isn't freed.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.cm>

For rest of the patches I'll look at them post-v4.5. I'm still waiting
for my first pull request containing fixes for that release to be
taken.

James, would it be possible to take the pull request containing fixes
soon that I origianally sent already in Jan and bit updated version
few days ago [1]? I still have one more pull request coming after that. It will
contain only few line changes:

* Missing put_device() in two places.
* There is a small change to ABI behavior (I'll post it
  soon) of policy sealing so I really need it for this release because
  4.5 is the first release containing this feature and I cannot change
  it when the release is tagged.

Thank you.

[1] http://www.gossamer-threads.com/lists/linux/kernel/2366902

/Jarkko


> ---
>  drivers/char/tpm/tpm-chip.c | 2 ++
>  drivers/char/tpm/tpm.h      | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 45cc39aabeee..ae2fed8a162b 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
>  			chip = pos;
>  			break;
>  		}
> +
> +		get_device(&chip->dev);
>  	}
>  	rcu_read_unlock();
>  	return chip;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 542a80cbfd9c..f6ba79d91857 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -207,6 +207,7 @@ struct tpm_chip {
>  static inline void tpm_chip_put(struct tpm_chip *chip)
>  {
>  	module_put(chip->pdev->driver->owner);
> +	put_device(&chip->dev);
>  }
>  
>  static inline int tpm_read_index(int base, int index)
> -- 
> 2.1.4
> 

Re: [PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jason Gunthorpe]

Hi Jarkko,

I found a bug in this last night, please hold off.

Jason


On Feb 13, 2016 3:08 AM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>
> On Fri, Feb 12, 2016 at 05:04:29PM -0700, Jason Gunthorpe wrote: 
> > This was missed during the struct device conversion, we 
> > need to hold a kref on the chip to make sure it isn't freed. 
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> 
>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.cm> 
>
> For rest of the patches I'll look at them post-v4.5. I'm still waiting 
> for my first pull request containing fixes for that release to be 
> taken. 
>
> James, would it be possible to take the pull request containing fixes 
> soon that I origianally sent already in Jan and bit updated version 
> few days ago [1]? I still have one more pull request coming after that. It will 
> contain only few line changes: 
>
> * Missing put_device() in two places. 
> * There is a small change to ABI behavior (I'll post it 
>   soon) of policy sealing so I really need it for this release because 
>   4.5 is the first release containing this feature and I cannot change 
>   it when the release is tagged. 
>
> Thank you. 
>
> [1] http://www.gossamer-threads.com/lists/linux/kernel/2366902 
>
> /Jarkko 
>
>
> > --- 
> >  drivers/char/tpm/tpm-chip.c | 2 ++ 
> >  drivers/char/tpm/tpm.h      | 1 + 
> >  2 files changed, 3 insertions(+) 
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c 
> > index 45cc39aabeee..ae2fed8a162b 100644 
> > --- a/drivers/char/tpm/tpm-chip.c 
> > +++ b/drivers/char/tpm/tpm-chip.c 
> > @@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num) 
> >  chip = pos; 
> >  break; 
> >  } 
> > + 
> > + get_device(&chip->dev); 
> >  } 
> >  rcu_read_unlock(); 
> >  return chip; 
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h 
> > index 542a80cbfd9c..f6ba79d91857 100644 
> > --- a/drivers/char/tpm/tpm.h 
> > +++ b/drivers/char/tpm/tpm.h 
> > @@ -207,6 +207,7 @@ struct tpm_chip { 
> >  static inline void tpm_chip_put(struct tpm_chip *chip) 
> >  { 
> >  module_put(chip->pdev->driver->owner); 
> > + put_device(&chip->dev); 
> >  } 
> >  
> >  static inline int tpm_read_index(int base, int index) 
> > -- 
> > 2.1.4 
> > 

Re: [PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jarkko Sakkinen]

On Fri, Feb 12, 2016 at 05:04:29PM -0700, Jason Gunthorpe wrote:
> This was missed during the struct device conversion, we
> need to hold a kref on the chip to make sure it isn't freed.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

I'm bit confused about this patch. What is the regression if this needs
to be dropped from my last pull request for 4.5 (that is the only one
I'm planning to include from this patch set, rest are definitely
post-4.5)?

If there is clear regression in this patch, I can do it. I didn't
reproduce any.

/Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c | 2 ++
>  drivers/char/tpm/tpm.h      | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 45cc39aabeee..ae2fed8a162b 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
>  			chip = pos;
>  			break;
>  		}
> +
> +		get_device(&chip->dev);
>  	}
>  	rcu_read_unlock();
>  	return chip;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 542a80cbfd9c..f6ba79d91857 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -207,6 +207,7 @@ struct tpm_chip {
>  static inline void tpm_chip_put(struct tpm_chip *chip)
>  {
>  	module_put(chip->pdev->driver->owner);
> +	put_device(&chip->dev);
>  }
>  
>  static inline int tpm_read_index(int base, int index)
> -- 
> 2.1.4
> 

Re: [PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jason Gunthorpe]

On Sun, Feb 14, 2016 at 06:55:12AM +0200, Jarkko Sakkinen wrote:
> On Fri, Feb 12, 2016 at 05:04:29PM -0700, Jason Gunthorpe wrote:
> > This was missed during the struct device conversion, we
> > need to hold a kref on the chip to make sure it isn't freed.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> 
> I'm bit confused about this patch. What is the regression if this
> needs

The patch is simply totally broken, the placement of the get_device is
wrong:

> > @@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
> >  			chip = pos;
> >  			break;
> >  		}
> > +
> > +		get_device(&chip->dev);

It needs to be moved up two lines before the break, into the if
statement.

As for the urgency - today the tpm core relies on module locking to
try and prevent tpm_chip_unregister from racing with stuff like the
above. That is totally broken in modern kernels, but it is what the
core tries to do. Within that framework the get/put are not needed
because of the module locking.

The only time these additional get/put do anything is when we are
racing with tpm_unregister, but if we are racing with unregister then
there are much bigger problems and things will crash anyhow.

So, this patch is just a tiny step.

The revised version of this patch with the rw_sem attempts to address
the complete race.

Jason

Re: [PATCH 1/3] tpm: Hold the kref during tpm_chip_find_get

[Jarkko Sakkinen]

On Sat, Feb 13, 2016 at 11:50:08PM -0700, Jason Gunthorpe wrote:
> On Sun, Feb 14, 2016 at 06:55:12AM +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 12, 2016 at 05:04:29PM -0700, Jason Gunthorpe wrote:
> > > This was missed during the struct device conversion, we
> > > need to hold a kref on the chip to make sure it isn't freed.
> > > 
> > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > 
> > I'm bit confused about this patch. What is the regression if this
> > needs
> 
> The patch is simply totally broken, the placement of the get_device is
> wrong:
> 
> > > @@ -53,6 +53,8 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
> > >  			chip = pos;
> > >  			break;
> > >  		}
> > > +
> > > +		get_device(&chip->dev);
> 
> It needs to be moved up two lines before the break, into the if
> statement.

Right.

> As for the urgency - today the tpm core relies on module locking to
> try and prevent tpm_chip_unregister from racing with stuff like the
> above. That is totally broken in modern kernels, but it is what the
> core tries to do. Within that framework the get/put are not needed
> because of the module locking.

Right, because that gives the guarantee that device has refcount of
at least one.

> The only time these additional get/put do anything is when we are
> racing with tpm_unregister, but if we are racing with unregister then
> there are much bigger problems and things will crash anyhow.
> 
> So, this patch is just a tiny step.
> 
> The revised version of this patch with the rw_sem attempts to address
> the complete race.

Got it. Yeah, I'll drop this from my next pull request. Thanks for
the explanation.

> Jason

/Jarkko

[PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

This is a hold over from before the struct device conversion.

- All prints should be using &chip->dev, which is the Linux
  standard. This changes prints to use tpm0 as the device name,
  not the PnP/etc ID.
- The few places involving sysfs/modules that really do need the
  parent just use chip->dev.parent instead
- We no longer need to get_device(pdev) in any places since it is no
  longer used by any of the code. The kref on the parent is held
  by the device core during device_add and dropped in device_del

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c         | 15 ++++++---------
 drivers/char/tpm/tpm-dev.c          |  4 +---
 drivers/char/tpm/tpm-interface.c    | 30 ++++++++++++++++--------------
 drivers/char/tpm/tpm-sysfs.c        |  6 +++---
 drivers/char/tpm/tpm.h              |  3 +--
 drivers/char/tpm/tpm2-cmd.c         |  8 ++++----
 drivers/char/tpm/tpm_atmel.c        | 14 +++++++-------
 drivers/char/tpm/tpm_i2c_atmel.c    | 16 ++++++++--------
 drivers/char/tpm/tpm_i2c_infineon.c |  6 +++---
 drivers/char/tpm/tpm_i2c_nuvoton.c  | 26 +++++++++++++-------------
 drivers/char/tpm/tpm_infineon.c     | 22 +++++++++++-----------
 drivers/char/tpm/tpm_nsc.c          | 20 ++++++++++----------
 drivers/char/tpm/tpm_tis.c          | 16 ++++++++--------
 13 files changed, 91 insertions(+), 95 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index ae2fed8a162b..b1364bf62492 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -49,7 +49,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
 		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
 			continue;
 
-		if (try_module_get(pos->pdev->driver->owner)) {
+		if (try_module_get(pos->dev.parent->driver->owner)) {
 			chip = pos;
 			break;
 		}
@@ -114,13 +114,11 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 
 	scnprintf(chip->devname, sizeof(chip->devname), "tpm%d", chip->dev_num);
 
-	chip->pdev = dev;
-
 	dev_set_drvdata(dev, chip);
 
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
-	chip->dev.parent = chip->pdev;
+	chip->dev.parent = dev;
 #ifdef CONFIG_ACPI
 	chip->dev.groups = chip->groups;
 #endif
@@ -135,7 +133,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 	device_initialize(&chip->dev);
 
 	cdev_init(&chip->cdev, &tpm_fops);
-	chip->cdev.owner = chip->pdev->driver->owner;
+	chip->cdev.owner = dev->driver->owner;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 
 	return chip;
@@ -236,9 +234,8 @@ int tpm_chip_register(struct tpm_chip *chip)
 	chip->flags |= TPM_CHIP_FLAG_REGISTERED;
 
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
-		rc = __compat_only_sysfs_link_entry_to_kobj(&chip->pdev->kobj,
-							    &chip->dev.kobj,
-							    "ppi");
+		rc = __compat_only_sysfs_link_entry_to_kobj(
+		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
 		if (rc && rc != -ENOENT) {
 			tpm_chip_unregister(chip);
 			return rc;
@@ -273,7 +270,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 	synchronize_rcu();
 
 	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
-		sysfs_remove_link(&chip->pdev->kobj, "ppi");
+		sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
 
 	tpm1_chip_unregister(chip);
 	tpm_dev_del_device(chip);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index de0337ebd658..4009765c14fd 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -61,7 +61,7 @@ static int tpm_open(struct inode *inode, struct file *file)
 	 * by the check of is_open variable, which is protected
 	 * by driver_lock. */
 	if (test_and_set_bit(0, &chip->is_open)) {
-		dev_dbg(chip->pdev, "Another process owns this TPM\n");
+		dev_dbg(&chip->dev, "Another process owns this TPM\n");
 		return -EBUSY;
 	}
 
@@ -79,7 +79,6 @@ static int tpm_open(struct inode *inode, struct file *file)
 	INIT_WORK(&priv->work, timeout_work);
 
 	file->private_data = priv;
-	get_device(chip->pdev);
 	return 0;
 }
 
@@ -166,7 +165,6 @@ static int tpm_release(struct inode *inode, struct file *file)
 	file->private_data = NULL;
 	atomic_set(&priv->data_pending, 0);
 	clear_bit(0, &priv->chip->is_open);
-	put_device(priv->chip->pdev);
 	kfree(priv);
 	return 0;
 }
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index e2fa89c88304..483f86ff6a0a 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -345,7 +345,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 	if (count == 0)
 		return -ENODATA;
 	if (count > bufsiz) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"invalid count value %x %zx\n", count, bufsiz);
 		return -E2BIG;
 	}
@@ -354,7 +354,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"tpm_transmit: tpm_send: error %zd\n", rc);
 		goto out;
 	}
@@ -373,7 +373,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 			goto out_recv;
 
 		if (chip->ops->req_canceled(chip, status)) {
-			dev_err(chip->pdev, "Operation Canceled\n");
+			dev_err(&chip->dev, "Operation Canceled\n");
 			rc = -ECANCELED;
 			goto out;
 		}
@@ -383,14 +383,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
 	} while (time_before(jiffies, stop));
 
 	chip->ops->cancel(chip);
-	dev_err(chip->pdev, "Operation Timed out\n");
+	dev_err(&chip->dev, "Operation Timed out\n");
 	rc = -ETIME;
 	goto out;
 
 out_recv:
 	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
 	if (rc < 0)
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %zd\n", rc);
 out:
 	mutex_unlock(&chip->tpm_mutex);
@@ -416,7 +416,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
 
 	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
-		dev_err(chip->pdev, "A TPM error (%d) occurred %s\n", err,
+		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
 			desc);
 
 	return err;
@@ -527,7 +527,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	if (rc == TPM_ERR_INVALID_POSTINIT) {
 		/* The TPM is not started, we are the first to talk to it.
 		   Execute a startup command. */
-		dev_info(chip->pdev, "Issuing TPM_STARTUP");
+		dev_info(&chip->dev, "Issuing TPM_STARTUP");
 		if (tpm_startup(chip, TPM_ST_CLEAR))
 			return rc;
 
@@ -539,7 +539,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 				  NULL);
 	}
 	if (rc) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
 			rc);
 		goto duration;
@@ -578,7 +578,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 
 	/* Report adjusted timeouts */
 	if (chip->vendor.timeout_adjusted) {
-		dev_info(chip->pdev,
+		dev_info(&chip->dev,
 			 HW_ERR "Adjusting reported timeouts: A %lu->%luus B %lu->%luus C %lu->%luus D %lu->%luus\n",
 			 old_timeout[0], new_timeout[0],
 			 old_timeout[1], new_timeout[1],
@@ -625,7 +625,7 @@ duration:
 		chip->vendor.duration[TPM_MEDIUM] *= 1000;
 		chip->vendor.duration[TPM_LONG] *= 1000;
 		chip->vendor.duration_adjusted = true;
-		dev_info(chip->pdev, "Adjusting TPM timeout parameters.");
+		dev_info(&chip->dev, "Adjusting TPM timeout parameters.");
 	}
 	return 0;
 }
@@ -815,7 +815,9 @@ int tpm_do_selftest(struct tpm_chip *chip)
 		 * around 300ms while the self test is ongoing, keep trying
 		 * until the self test duration expires. */
 		if (rc == -ETIME) {
-			dev_info(chip->pdev, HW_ERR "TPM command timed out during continue self test");
+			dev_info(
+			    &chip->dev, HW_ERR
+			    "TPM command timed out during continue self test");
 			msleep(delay_msec);
 			continue;
 		}
@@ -825,7 +827,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
 
 		rc = be32_to_cpu(cmd.header.out.return_code);
 		if (rc == TPM_ERR_DISABLED || rc == TPM_ERR_DEACTIVATED) {
-			dev_info(chip->pdev,
+			dev_info(&chip->dev,
 				 "TPM is disabled/deactivated (0x%X)\n", rc);
 			/* TPM is disabled and/or deactivated; driver can
 			 * proceed and TPM does handle commands for
@@ -978,10 +980,10 @@ int tpm_pm_suspend(struct device *dev)
 	}
 
 	if (rc)
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"Error (%d) sending savestate before suspend\n", rc);
 	else if (try > 0)
-		dev_warn(chip->pdev, "TPM savestate took %dms\n",
+		dev_warn(&chip->dev, "TPM savestate took %dms\n",
 			 try * TPM_TIMEOUT_RETRY);
 
 	return rc;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index ee66fd4673f3..d93736aa2703 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -284,16 +284,16 @@ static const struct attribute_group tpm_dev_group = {
 int tpm_sysfs_add_device(struct tpm_chip *chip)
 {
 	int err;
-	err = sysfs_create_group(&chip->pdev->kobj,
+	err = sysfs_create_group(&chip->dev.parent->kobj,
 				 &tpm_dev_group);
 
 	if (err)
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"failed to create sysfs attributes, %d\n", err);
 	return err;
 }
 
 void tpm_sysfs_del_device(struct tpm_chip *chip)
 {
-	sysfs_remove_group(&chip->pdev->kobj, &tpm_dev_group);
+	sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f6ba79d91857..371f75f4d2a7 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -174,7 +174,6 @@ enum tpm_chip_flags {
 };
 
 struct tpm_chip {
-	struct device *pdev;	/* Device stuff */
 	struct device dev;
 	struct cdev cdev;
 
@@ -206,7 +205,7 @@ struct tpm_chip {
 
 static inline void tpm_chip_put(struct tpm_chip *chip)
 {
-	module_put(chip->pdev->driver->owner);
+	module_put(chip->dev.parent->driver->owner);
 	put_device(&chip->dev);
 }
 
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 45a634016f95..6eb8b74b2d38 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -597,7 +597,7 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 
 	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
 	if (rc) {
-		dev_warn(chip->pdev, "0x%08x was not flushed, out of memory\n",
+		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
 			 handle);
 		return;
 	}
@@ -606,7 +606,7 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 
 	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
 	if (rc)
-		dev_warn(chip->pdev, "0x%08x was not flushed, rc=%d\n", handle,
+		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
 			 rc);
 
 	tpm_buf_destroy(&buf);
@@ -770,7 +770,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 	 * except print the error code on a system failure.
 	 */
 	if (rc < 0)
-		dev_warn(chip->pdev, "transmit returned %d while stopping the TPM",
+		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
 			 rc);
 }
 EXPORT_SYMBOL_GPL(tpm2_shutdown);
@@ -837,7 +837,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
 	 * immediately. This is a workaround for that.
 	 */
 	if (rc == TPM2_RC_TESTING) {
-		dev_warn(chip->pdev, "Got RC_TESTING, ignoring\n");
+		dev_warn(&chip->dev, "Got RC_TESTING, ignoring\n");
 		rc = 0;
 	}
 
diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index dfadad0916a1..a48a878f791d 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -49,7 +49,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	for (i = 0; i < 6; i++) {
 		status = ioread8(chip->vendor.iobase + 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-			dev_err(chip->pdev, "error reading header\n");
+			dev_err(&chip->dev, "error reading header\n");
 			return -EIO;
 		}
 		*buf++ = ioread8(chip->vendor.iobase);
@@ -60,12 +60,12 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	size = be32_to_cpu(*native_size);
 
 	if (count < size) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"Recv size(%d) less than available space\n", size);
 		for (; i < size; i++) {	/* clear the waiting data anyway */
 			status = ioread8(chip->vendor.iobase + 1);
 			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-				dev_err(chip->pdev, "error reading data\n");
+				dev_err(&chip->dev, "error reading data\n");
 				return -EIO;
 			}
 		}
@@ -76,7 +76,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	for (; i < size; i++) {
 		status = ioread8(chip->vendor.iobase + 1);
 		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
-			dev_err(chip->pdev, "error reading data\n");
+			dev_err(&chip->dev, "error reading data\n");
 			return -EIO;
 		}
 		*buf++ = ioread8(chip->vendor.iobase);
@@ -86,7 +86,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	status = ioread8(chip->vendor.iobase + 1);
 
 	if (status & ATML_STATUS_DATA_AVAIL) {
-		dev_err(chip->pdev, "data available is stuck\n");
+		dev_err(&chip->dev, "data available is stuck\n");
 		return -EIO;
 	}
 
@@ -97,9 +97,9 @@ static int tpm_atml_send(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	int i;
 
-	dev_dbg(chip->pdev, "tpm_atml_send:\n");
+	dev_dbg(&chip->dev, "tpm_atml_send:\n");
 	for (i = 0; i < count; i++) {
-		dev_dbg(chip->pdev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
+		dev_dbg(&chip->dev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
  		iowrite8(buf[i], chip->vendor.iobase);
 	}
 
diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
index 8dfb88b9739c..dd8f0eb3170a 100644
--- a/drivers/char/tpm/tpm_i2c_atmel.c
+++ b/drivers/char/tpm/tpm_i2c_atmel.c
@@ -52,7 +52,7 @@ struct priv_data {
 static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	struct priv_data *priv = chip->vendor.priv;
-	struct i2c_client *client = to_i2c_client(chip->pdev);
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	s32 status;
 
 	priv->len = 0;
@@ -62,7 +62,7 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
 
 	status = i2c_master_send(client, buf, len);
 
-	dev_dbg(chip->pdev,
+	dev_dbg(&chip->dev,
 		"%s(buf=%*ph len=%0zx) -> sts=%d\n", __func__,
 		(int)min_t(size_t, 64, len), buf, len, status);
 	return status;
@@ -71,7 +71,7 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
 static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
 	struct priv_data *priv = chip->vendor.priv;
-	struct i2c_client *client = to_i2c_client(chip->pdev);
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	struct tpm_output_header *hdr =
 		(struct tpm_output_header *)priv->buffer;
 	u32 expected_len;
@@ -88,7 +88,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		return -ENOMEM;
 
 	if (priv->len >= expected_len) {
-		dev_dbg(chip->pdev,
+		dev_dbg(&chip->dev,
 			"%s early(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
 			(int)min_t(size_t, 64, expected_len), buf, count,
 			expected_len);
@@ -97,7 +97,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	}
 
 	rc = i2c_master_recv(client, buf, expected_len);
-	dev_dbg(chip->pdev,
+	dev_dbg(&chip->dev,
 		"%s reread(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
 		(int)min_t(size_t, 64, expected_len), buf, count,
 		expected_len);
@@ -106,13 +106,13 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 
 static void i2c_atmel_cancel(struct tpm_chip *chip)
 {
-	dev_err(chip->pdev, "TPM operation cancellation was requested, but is not supported");
+	dev_err(&chip->dev, "TPM operation cancellation was requested, but is not supported");
 }
 
 static u8 i2c_atmel_read_status(struct tpm_chip *chip)
 {
 	struct priv_data *priv = chip->vendor.priv;
-	struct i2c_client *client = to_i2c_client(chip->pdev);
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	int rc;
 
 	/* The TPM fails the I2C read until it is ready, so we do the entire
@@ -125,7 +125,7 @@ static u8 i2c_atmel_read_status(struct tpm_chip *chip)
 	/* Once the TPM has completed the command the command remains readable
 	 * until another command is issued. */
 	rc = i2c_master_recv(client, priv->buffer, sizeof(priv->buffer));
-	dev_dbg(chip->pdev,
+	dev_dbg(&chip->dev,
 		"%s: sts=%d", __func__, rc);
 	if (rc <= 0)
 		return 0;
diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
index 63d5d22e9e60..f2aa99e34b4b 100644
--- a/drivers/char/tpm/tpm_i2c_infineon.c
+++ b/drivers/char/tpm/tpm_i2c_infineon.c
@@ -446,7 +446,7 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	/* read first 10 bytes, including tag, paramsize, and result */
 	size = recv_data(chip, buf, TPM_HEADER_SIZE);
 	if (size < TPM_HEADER_SIZE) {
-		dev_err(chip->pdev, "Unable to read header\n");
+		dev_err(&chip->dev, "Unable to read header\n");
 		goto out;
 	}
 
@@ -459,14 +459,14 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	size += recv_data(chip, &buf[TPM_HEADER_SIZE],
 			  expected - TPM_HEADER_SIZE);
 	if (size < expected) {
-		dev_err(chip->pdev, "Unable to read remainder of result\n");
+		dev_err(&chip->dev, "Unable to read remainder of result\n");
 		size = -ETIME;
 		goto out;
 	}
 
 	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c, &status);
 	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
-		dev_err(chip->pdev, "Error left over data\n");
+		dev_err(&chip->dev, "Error left over data\n");
 		size = -EIO;
 		goto out;
 	}
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 847f1597fe9b..8fb378f502e4 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -96,13 +96,13 @@ static s32 i2c_nuvoton_write_buf(struct i2c_client *client, u8 offset, u8 size,
 /* read TPM_STS register */
 static u8 i2c_nuvoton_read_status(struct tpm_chip *chip)
 {
-	struct i2c_client *client = to_i2c_client(chip->pdev);
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	s32 status;
 	u8 data;
 
 	status = i2c_nuvoton_read_buf(client, TPM_STS, 1, &data);
 	if (status <= 0) {
-		dev_err(chip->pdev, "%s() error return %d\n", __func__,
+		dev_err(&chip->dev, "%s() error return %d\n", __func__,
 			status);
 		data = TPM_STS_ERR_VAL;
 	}
@@ -127,13 +127,13 @@ static s32 i2c_nuvoton_write_status(struct i2c_client *client, u8 data)
 /* write commandReady to TPM_STS register */
 static void i2c_nuvoton_ready(struct tpm_chip *chip)
 {
-	struct i2c_client *client = to_i2c_client(chip->pdev);
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	s32 status;
 
 	/* this causes the current command to be aborted */
 	status = i2c_nuvoton_write_status(client, TPM_STS_COMMAND_READY);
 	if (status < 0)
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"%s() fail to write TPM_STS.commandReady\n", __func__);
 }
 
@@ -212,7 +212,7 @@ static int i2c_nuvoton_wait_for_stat(struct tpm_chip *chip, u8 mask, u8 value,
 				return 0;
 		} while (time_before(jiffies, stop));
 	}
-	dev_err(chip->pdev, "%s(%02x, %02x) -> timeout\n", __func__, mask,
+	dev_err(&chip->dev, "%s(%02x, %02x) -> timeout\n", __func__, mask,
 		value);
 	return -ETIMEDOUT;
 }
@@ -240,7 +240,7 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
 					       &chip->vendor.read_queue) == 0) {
 		burst_count = i2c_nuvoton_get_burstcount(client, chip);
 		if (burst_count < 0) {
-			dev_err(chip->pdev,
+			dev_err(&chip->dev,
 				"%s() fail to read burstCount=%d\n", __func__,
 				burst_count);
 			return -EIO;
@@ -249,12 +249,12 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
 		rc = i2c_nuvoton_read_buf(client, TPM_DATA_FIFO_R,
 					  bytes2read, &buf[size]);
 		if (rc < 0) {
-			dev_err(chip->pdev,
+			dev_err(&chip->dev,
 				"%s() fail on i2c_nuvoton_read_buf()=%d\n",
 				__func__, rc);
 			return -EIO;
 		}
-		dev_dbg(chip->pdev, "%s(%d):", __func__, bytes2read);
+		dev_dbg(&chip->dev, "%s(%d):", __func__, bytes2read);
 		size += bytes2read;
 	}
 
@@ -264,8 +264,8 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
 /* Read TPM command results */
 static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 {
-	struct device *dev = chip->pdev;
-	struct i2c_client *client = to_i2c_client(dev);
+	struct device *dev = &chip->dev;
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	s32 rc;
 	int expected, status, burst_count, retries, size = 0;
 
@@ -334,7 +334,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 		break;
 	}
 	i2c_nuvoton_ready(chip);
-	dev_dbg(chip->pdev, "%s() -> %d\n", __func__, size);
+	dev_dbg(&chip->dev, "%s() -> %d\n", __func__, size);
 	return size;
 }
 
@@ -347,8 +347,8 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
  */
 static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
 {
-	struct device *dev = chip->pdev;
-	struct i2c_client *client = to_i2c_client(dev);
+	struct device *dev = &chip->dev;
+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
 	u32 ordinal;
 	size_t count = 0;
 	int burst_count, bytes2write, retries, rc = -EIO;
diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
index 6c488e635fdd..e3cf9f3545c5 100644
--- a/drivers/char/tpm/tpm_infineon.c
+++ b/drivers/char/tpm/tpm_infineon.c
@@ -195,9 +195,9 @@ static int wait(struct tpm_chip *chip, int wait_for_bit)
 	}
 	if (i == TPM_MAX_TRIES) {	/* timeout occurs */
 		if (wait_for_bit == STAT_XFE)
-			dev_err(chip->pdev, "Timeout in wait(STAT_XFE)\n");
+			dev_err(&chip->dev, "Timeout in wait(STAT_XFE)\n");
 		if (wait_for_bit == STAT_RDA)
-			dev_err(chip->pdev, "Timeout in wait(STAT_RDA)\n");
+			dev_err(&chip->dev, "Timeout in wait(STAT_RDA)\n");
 		return -EIO;
 	}
 	return 0;
@@ -220,7 +220,7 @@ static void wait_and_send(struct tpm_chip *chip, u8 sendbyte)
 static void tpm_wtx(struct tpm_chip *chip)
 {
 	number_of_wtx++;
-	dev_info(chip->pdev, "Granting WTX (%02d / %02d)\n",
+	dev_info(&chip->dev, "Granting WTX (%02d / %02d)\n",
 		 number_of_wtx, TPM_MAX_WTX_PACKAGES);
 	wait_and_send(chip, TPM_VL_VER);
 	wait_and_send(chip, TPM_CTRL_WTX);
@@ -231,7 +231,7 @@ static void tpm_wtx(struct tpm_chip *chip)
 
 static void tpm_wtx_abort(struct tpm_chip *chip)
 {
-	dev_info(chip->pdev, "Aborting WTX\n");
+	dev_info(&chip->dev, "Aborting WTX\n");
 	wait_and_send(chip, TPM_VL_VER);
 	wait_and_send(chip, TPM_CTRL_WTX_ABORT);
 	wait_and_send(chip, 0x00);
@@ -257,7 +257,7 @@ recv_begin:
 	}
 
 	if (buf[0] != TPM_VL_VER) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"Wrong transport protocol implementation!\n");
 		return -EIO;
 	}
@@ -272,7 +272,7 @@ recv_begin:
 		}
 
 		if ((size == 0x6D00) && (buf[1] == 0x80)) {
-			dev_err(chip->pdev, "Error handling on vendor layer!\n");
+			dev_err(&chip->dev, "Error handling on vendor layer!\n");
 			return -EIO;
 		}
 
@@ -284,7 +284,7 @@ recv_begin:
 	}
 
 	if (buf[1] == TPM_CTRL_WTX) {
-		dev_info(chip->pdev, "WTX-package received\n");
+		dev_info(&chip->dev, "WTX-package received\n");
 		if (number_of_wtx < TPM_MAX_WTX_PACKAGES) {
 			tpm_wtx(chip);
 			goto recv_begin;
@@ -295,14 +295,14 @@ recv_begin:
 	}
 
 	if (buf[1] == TPM_CTRL_WTX_ABORT_ACK) {
-		dev_info(chip->pdev, "WTX-abort acknowledged\n");
+		dev_info(&chip->dev, "WTX-abort acknowledged\n");
 		return size;
 	}
 
 	if (buf[1] == TPM_CTRL_ERROR) {
-		dev_err(chip->pdev, "ERROR-package received:\n");
+		dev_err(&chip->dev, "ERROR-package received:\n");
 		if (buf[4] == TPM_INF_NAK)
-			dev_err(chip->pdev,
+			dev_err(&chip->dev,
 				"-> Negative acknowledgement"
 				" - retransmit command!\n");
 		return -EIO;
@@ -321,7 +321,7 @@ static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count)
 
 	ret = empty_fifo(chip, 1);
 	if (ret) {
-		dev_err(chip->pdev, "Timeout while clearing FIFO\n");
+		dev_err(&chip->dev, "Timeout while clearing FIFO\n");
 		return -EIO;
 	}
 
diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
index 289389ecef84..766370bed60c 100644
--- a/drivers/char/tpm/tpm_nsc.c
+++ b/drivers/char/tpm/tpm_nsc.c
@@ -113,7 +113,7 @@ static int nsc_wait_for_ready(struct tpm_chip *chip)
 	}
 	while (time_before(jiffies, stop));
 
-	dev_info(chip->pdev, "wait for ready failed\n");
+	dev_info(&chip->dev, "wait for ready failed\n");
 	return -EBUSY;
 }
 
@@ -129,12 +129,12 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 		return -EIO;
 
 	if (wait_for_stat(chip, NSC_STATUS_F0, NSC_STATUS_F0, &data) < 0) {
-		dev_err(chip->pdev, "F0 timeout\n");
+		dev_err(&chip->dev, "F0 timeout\n");
 		return -EIO;
 	}
 	if ((data =
 	     inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_NORMAL) {
-		dev_err(chip->pdev, "not in normal mode (0x%x)\n",
+		dev_err(&chip->dev, "not in normal mode (0x%x)\n",
 			data);
 		return -EIO;
 	}
@@ -143,7 +143,7 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 	for (p = buffer; p < &buffer[count]; p++) {
 		if (wait_for_stat
 		    (chip, NSC_STATUS_OBF, NSC_STATUS_OBF, &data) < 0) {
-			dev_err(chip->pdev,
+			dev_err(&chip->dev,
 				"OBF timeout (while reading data)\n");
 			return -EIO;
 		}
@@ -154,11 +154,11 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
 
 	if ((data & NSC_STATUS_F0) == 0 &&
 	(wait_for_stat(chip, NSC_STATUS_F0, NSC_STATUS_F0, &data) < 0)) {
-		dev_err(chip->pdev, "F0 not set\n");
+		dev_err(&chip->dev, "F0 not set\n");
 		return -EIO;
 	}
 	if ((data = inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_EOC) {
-		dev_err(chip->pdev,
+		dev_err(&chip->dev,
 			"expected end of command(0x%x)\n", data);
 		return -EIO;
 	}
@@ -189,19 +189,19 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
 		return -EIO;
 
 	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
-		dev_err(chip->pdev, "IBF timeout\n");
+		dev_err(&chip->dev, "IBF timeout\n");
 		return -EIO;
 	}
 
 	outb(NSC_COMMAND_NORMAL, chip->vendor.base + NSC_COMMAND);
 	if (wait_for_stat(chip, NSC_STATUS_IBR, NSC_STATUS_IBR, &data) < 0) {
-		dev_err(chip->pdev, "IBR timeout\n");
+		dev_err(&chip->dev, "IBR timeout\n");
 		return -EIO;
 	}
 
 	for (i = 0; i < count; i++) {
 		if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
-			dev_err(chip->pdev,
+			dev_err(&chip->dev,
 				"IBF timeout (while writing data)\n");
 			return -EIO;
 		}
@@ -209,7 +209,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
 	}
 
 	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
-		dev_err(chip->pdev, "IBF timeout\n");
+		dev_err(&chip->dev, "IBF timeout\n");
 		return -EIO;
 	}
 	outb(NSC_COMMAND_EOC, chip->vendor.base + NSC_COMMAND);
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 8a3509cb10da..f15466b7fb78 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -293,7 +293,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	/* read first 10 bytes, including tag, paramsize, and result */
 	if ((size =
 	     recv_data(chip, buf, TPM_HEADER_SIZE)) < TPM_HEADER_SIZE) {
-		dev_err(chip->pdev, "Unable to read header\n");
+		dev_err(&chip->dev, "Unable to read header\n");
 		goto out;
 	}
 
@@ -306,7 +306,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 	if ((size +=
 	     recv_data(chip, &buf[TPM_HEADER_SIZE],
 		       expected - TPM_HEADER_SIZE)) < expected) {
-		dev_err(chip->pdev, "Unable to read remainder of result\n");
+		dev_err(&chip->dev, "Unable to read remainder of result\n");
 		size = -ETIME;
 		goto out;
 	}
@@ -315,7 +315,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 			  &chip->vendor.int_queue, false);
 	status = tpm_tis_status(chip);
 	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
-		dev_err(chip->pdev, "Error left over data\n");
+		dev_err(&chip->dev, "Error left over data\n");
 		size = -EIO;
 		goto out;
 	}
@@ -401,7 +401,7 @@ static void disable_interrupts(struct tpm_chip *chip)
 	iowrite32(intmask,
 		  chip->vendor.iobase +
 		  TPM_INT_ENABLE(chip->vendor.locality));
-	devm_free_irq(chip->pdev, chip->vendor.irq, chip);
+	devm_free_irq(&chip->dev, chip->vendor.irq, chip);
 	chip->vendor.irq = 0;
 }
 
@@ -530,7 +530,7 @@ static int probe_itpm(struct tpm_chip *chip)
 
 	rc = tpm_tis_send_data(chip, cmd_getticks, len);
 	if (rc == 0) {
-		dev_info(chip->pdev, "Detected an iTPM.\n");
+		dev_info(&chip->dev, "Detected an iTPM.\n");
 		rc = 1;
 	} else
 		rc = -EFAULT;
@@ -609,9 +609,9 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
 	struct priv_data *priv = chip->vendor.priv;
 	u8 original_int_vec;
 
-	if (devm_request_irq(chip->pdev, irq, tis_int_handler, flags,
+	if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
 			     chip->devname, chip) != 0) {
-		dev_info(chip->pdev, "Unable to request irq: %d for probe\n",
+		dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
 			 irq);
 		return -1;
 	}
@@ -812,7 +812,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
 			tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
 						 tpm_info->irq);
 			if (!chip->vendor.irq)
-				dev_err(chip->pdev, FW_BUG
+				dev_err(&chip->dev, FW_BUG
 					"TPM interrupt not working, polling instead\n");
 		} else
 			tpm_tis_probe_irq(chip, intmask);
-- 
2.1.4

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

On Fri, Feb 12, 2016 at 07:37:10PM -0500, Stefan Berger wrote:
>    Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/12/2016
>    07:04:30 PM:
>    >
>    > This is a hold over from before the struct device conversion.
>    >
>    > - All prints should be using &chip->dev, which is the Linux
>    >   standard. This changes prints to use tpm0 as the device name,
>    >   not the PnP/etc ID.
>    > - The few places involving sysfs/modules that really do need the
>    >   parent just use chip->dev.parent instead
>    > - We no longer need to get_device(pdev) in any places since it is no
>    >   longer used by any of the code. The kref on the parent is held
>    >   by the device core during device_add and dropped in device_del
>    That is exactly what was needed for the vtpm driver and now you're
>    removing it. Is that still going to work after this change? Or do we
>    need to re-add it as get/put_device(chip->dev.parent) ?

That code was not correct, the get_device side has racy
lack-of-locking problems and it serves no purpose for the tpm core or
any existing driver.

I already fixed this once in commit ba0ef85479c46a 'tpm: Fix
initialization of the cdev' - that solves the racing of get_device,
and grabs the correct device kref, but I forgot to delete the broken
residual get_device. Sigh.

It is unfortunate that bogus code sent you down this rabbit hole. My
bad :(

I'll send you something else that might work for vtpm...

Jason

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

On Fri, Feb 12, 2016 at 08:31:21PM -0500, Stefan Berger wrote:

> The vtpm driver will introduce chip->priv, which will point to
> vtpm_dev. For

Why not just use chip->vendor.priv? Aka TPM_VPRIV

> this reason we need to hold a reference to the vtpm_dev->dev in the
> front end.

Yes, but all drivers are like this. Most will just kfree their priv immediately

All sane Linux core subsystems guarentee that after their unregister
returns the driver callbacks will be done and uncallable, it is a bug
that tpm does not do this.

> So we could optimize it:
> 
> if (chip->priv)
>         get_device(chip->dev.parent);

That doesn't address the race

Jason

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

On Fri, Feb 12, 2016 at 08:31:21PM -0500, Stefan Berger wrote:
> > I'll send you something else that might work for vtpm...'
> 
> The vtpm driver will introduce chip->priv, which will point to vtpm_dev. For
> this reason we need to hold a reference to the vtpm_dev->dev in the
> front end.

This should take care of it for all drivers including vtpm.

https://github.com/jgunthorpe/linux/commits/for-jarkko

At the very least this turns silent use after free into a null pointer
oops.

We should also discuss if we want to continue to have the driver
module locked while /dev/tpmX is open, that is no longer needed for
corectness.

>From 52c5710ff585e936687e57ca5e267e82e334ebc5 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Fri, 12 Feb 2016 20:29:53 -0700
Subject: [PATCH 3/3] tpm: Provide strong locking for device removal

Add a read/write semaphore around the ops function pointers so
ops can be set to null when the driver un-registers.

Previously the tpm core expected module locking to be enough to
ensure that tpm_unregister could not be called during certain times,
however that hasn't been sufficient for a long time.

Introduce a read/write semaphore around 'ops' so the core can set
it to null when unregistering. This provides a strong fence around
the driver callbacks, guaranteeing to the driver that no callbacks
are running or will run again.

For now the ops_lock is placed very high in the call stack, it could
be pushed down and made more granular in future if necessary.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c      | 72 ++++++++++++++++++++++++++++++++++++----
 drivers/char/tpm/tpm-dev.c       | 10 +++++-
 drivers/char/tpm/tpm-interface.c | 18 +++++-----
 drivers/char/tpm/tpm-sysfs.c     |  5 +++
 drivers/char/tpm/tpm.h           | 14 +++++---
 5 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index df4132e8b982..647fdf327537 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -36,9 +36,59 @@ 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
+/**
+ * tpm_try_get_ops() - Get a ref to the tpm_chip
+ * @chip: Chip to ref
+ *
+ * The caller must already have some kind of locking to ensure that chip is
+ * valid. This function will lock the chip so that the ops member can be
+ * accessed safely. The locking prevents tpm_chip_unregister from
+ * completing, so it should not be held for long periods.
+ *
+ * Returns -ERRNO if the chip could not be got.
+ */
+int tpm_try_get_ops(struct tpm_chip *chip)
+{
+	int rc = -EIO;
+
+	get_device(&chip->dev);
+
+	down_read(&chip->ops_sem);
+	if (!chip->ops)
+		goto out_lock;
+
+	if (!try_module_get(chip->dev.parent->driver->owner))
+		goto out_lock;
+
+	return 0;
+out_lock:
+	up_read(&chip->ops_sem);
+	put_device(&chip->dev);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_try_get_ops);
+
+/**
+ * tpm_put_ops() - Release a ref to the tpm_chip
+ * @chip: Chip to put
+ *
+ * This is the opposite pair to tpm_try_get_ops(). After this returns chip may
+ * be kfree'd.
+ */
+void tpm_put_ops(struct tpm_chip *chip)
+{
+	module_put(chip->dev.parent->driver->owner);
+	up_read(&chip->ops_sem);
+	put_device(&chip->dev);
+}
+EXPORT_SYMBOL_GPL(tpm_put_ops);
+
+/**
+ * tpm_chip_find_get() - return tpm_chip for a given chip number
+ * @chip_num: id to find
+ *
+ * The return'd chip has been tpm_try_get_ops'd and must be released via
+ * tpm_put_ops
  */
 struct tpm_chip *tpm_chip_find_get(int chip_num)
 {
@@ -49,10 +99,10 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
 		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
 			continue;
 
-		if (try_module_get(pos->dev.parent->driver->owner)) {
+		/* rcu prevents chip from being free'd */
+		if (!tpm_try_get_ops(pos))
 			chip = pos;
-			break;
-		}
+		break;
 	}
 	rcu_read_unlock();
 	return chip;
@@ -95,6 +145,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 		return ERR_PTR(-ENOMEM);
 
 	mutex_init(&chip->tpm_mutex);
+	init_rwsem(&chip->ops_sem);
 	INIT_LIST_HEAD(&chip->list);
 
 	chip->ops = ops;
@@ -174,6 +225,12 @@ static int tpm_dev_add_device(struct tpm_chip *chip)
 static void tpm_dev_del_device(struct tpm_chip *chip)
 {
 	cdev_del(&chip->cdev);
+
+	/* Make the driver uncallable. */
+	down_write(&chip->ops_sem);
+	chip->ops = NULL;
+	up_write(&chip->ops_sem);
+
 	device_unregister(&chip->dev);
 }
 
@@ -259,6 +316,9 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);
  * Takes the chip first away from the list of available TPM chips and then
  * cleans up all the resources reserved by tpm_chip_register().
  *
+ * Once this function returns the driver call backs in 'op's will not be
+ * running and will no longer start.
+ *
  * NOTE: This function should be only called before deinitializing chip
  * resources.
  */
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 4009765c14fd..1474f8b8297b 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -136,9 +136,17 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 		return -EFAULT;
 	}
 
-	/* atomic tpm command send and result receive */
+	/* atomic tpm command send and result receive. We only hold the ops
+	 * lock during this period so that the tpm can be unregistered even if
+	 * the char dev is held open.
+	 */
+	if (tpm_try_get_ops(priv->chip)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EPIPE;
+	}
 	out_size = tpm_transmit(priv->chip, priv->data_buffer,
 				sizeof(priv->data_buffer));
+	tpm_put_ops(priv->chip);
 	if (out_size < 0) {
 		mutex_unlock(&priv->buffer_mutex);
 		return out_size;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 483f86ff6a0a..49bdab5509b2 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -700,7 +700,7 @@ int tpm_is_tpm2(u32 chip_num)
 
 	rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 
 	return rc;
 }
@@ -729,7 +729,7 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
 		rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
 	else
 		rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_read);
@@ -764,7 +764,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		rc = tpm2_pcr_extend(chip, pcr_idx, hash);
-		tpm_chip_put(chip);
+		tpm_put_ops(chip);
 		return rc;
 	}
 
@@ -774,7 +774,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
 			      "attempting extend a PCR value");
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_pcr_extend);
@@ -855,7 +855,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 
 	rc = tpm_transmit_cmd(chip, cmd, buflen, "attempting tpm_cmd");
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_send);
@@ -1037,7 +1037,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		err = tpm2_get_random(chip, out, max);
-		tpm_chip_put(chip);
+		tpm_put_ops(chip);
 		return err;
 	}
 
@@ -1059,7 +1059,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 		num_bytes -= recd;
 	} while (retries-- && total < max);
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return total ? total : -EIO;
 }
 EXPORT_SYMBOL_GPL(tpm_get_random);
@@ -1085,7 +1085,7 @@ int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
 
 	rc = tpm2_seal_trusted(chip, payload, options);
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_seal_trusted);
@@ -1111,7 +1111,7 @@ int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
 
 	rc = tpm2_unseal_trusted(chip, payload, options);
 
-	tpm_chip_put(chip);
+	tpm_put_ops(chip);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(tpm_unseal_trusted);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index d93736aa2703..34e7fc7e590c 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -295,5 +295,10 @@ int tpm_sysfs_add_device(struct tpm_chip *chip)
 
 void tpm_sysfs_del_device(struct tpm_chip *chip)
 {
+	/* The sysfs routines rely on an implicit tpm_try_get_ops, this
+	 * function is called before ops is null'd and the sysfs core
+	 * synchronizes this removal so that no callbacks are running or can
+	 * run again
+	 */
 	sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1e097350ce79..026695ae44ca 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -177,7 +177,13 @@ struct tpm_chip {
 	struct device dev;
 	struct cdev cdev;
 
+	/* A driver callback under ops cannot be run unless ops_sem is held
+	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
+	 * when the driver is unregistered, see tpm_try_get_ops.
+	 */
+	struct rw_semaphore ops_sem;
 	const struct tpm_class_ops *ops;
+
 	unsigned int flags;
 
 	int dev_num;		/* /dev/tpm# */
@@ -202,11 +208,6 @@ struct tpm_chip {
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
 
-static inline void tpm_chip_put(struct tpm_chip *chip)
-{
-	module_put(chip->dev.parent->driver->owner);
-}
-
 static inline int tpm_read_index(int base, int index)
 {
 	outb(index, base);
@@ -514,6 +515,9 @@ 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);
+__must_check int tpm_try_get_ops(struct tpm_chip *chip);
+void tpm_put_ops(struct tpm_chip *chip);
+
 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);
-- 
1.9.1

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jarkko Sakkinen]

On Fri, Feb 12, 2016 at 08:33:20PM -0700, Jason Gunthorpe wrote:
> On Fri, Feb 12, 2016 at 08:31:21PM -0500, Stefan Berger wrote:
> > > I'll send you something else that might work for vtpm...'
> > 
> > The vtpm driver will introduce chip->priv, which will point to vtpm_dev. For
> > this reason we need to hold a reference to the vtpm_dev->dev in the
> > front end.
> 
> This should take care of it for all drivers including vtpm.
> 
> https://github.com/jgunthorpe/linux/commits/for-jarkko
> 
> At the very least this turns silent use after free into a null pointer
> oops.
> 
> We should also discuss if we want to continue to have the driver
> module locked while /dev/tpmX is open, that is no longer needed for
> corectness.

I'm happy the patch that was sent before although I didn't give it
Reviewed-by because it had couple of style errors. If those two
style errors are the *only* issues I can fix up them.

Unless the differences are trivial (like a missing return value or
couple of minor style errors something very obvious) for me to fixup I'd
hope to re-review the code.

I'm not trying to be difficult here. For small errors I can amend
the commits but it seems that there was something more non-trival
done

Other than the first patch (for which I'm still waiting a clear
explanation why it is wrong), these are not going to 4.5 anyway so
there's lots time to things the long way.

/Jarkko

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

On Sun, Feb 14, 2016 at 07:24:14AM +0200, Jarkko Sakkinen wrote:
> > This should take care of it for all drivers including vtpm.
> > 
> > https://github.com/jgunthorpe/linux/commits/for-jarkko
> > 
> > At the very least this turns silent use after free into a null pointer
> > oops.
> > 
> > We should also discuss if we want to continue to have the driver
> > module locked while /dev/tpmX is open, that is no longer needed for
> > corectness.
> 
> I'm happy the patch that was sent before although I didn't give it
> Reviewed-by because it had couple of style errors. If those two
> style errors are the *only* issues I can fix up them.

This patch replaces the get/put_device patch entirely, if Stefan is
happy with it we can just go ahead in this direction for 4.6

There was also a 0day build error on the devname patch, so the whole
series will be reposted.

Jason

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jarkko Sakkinen]

On Sat, Feb 13, 2016 at 11:57:24PM -0700, Jason Gunthorpe wrote:
> On Sun, Feb 14, 2016 at 07:24:14AM +0200, Jarkko Sakkinen wrote:
> > > This should take care of it for all drivers including vtpm.
> > > 
> > > https://github.com/jgunthorpe/linux/commits/for-jarkko
> > > 
> > > At the very least this turns silent use after free into a null pointer
> > > oops.
> > > 
> > > We should also discuss if we want to continue to have the driver
> > > module locked while /dev/tpmX is open, that is no longer needed for
> > > corectness.
> > 
> > I'm happy the patch that was sent before although I didn't give it
> > Reviewed-by because it had couple of style errors. If those two
> > style errors are the *only* issues I can fix up them.
> 
> This patch replaces the get/put_device patch entirely, if Stefan is
> happy with it we can just go ahead in this direction for 4.6
> 
> There was also a 0day build error on the devname patch, so the whole
> series will be reposted.

Perfect, thank you.

> Jason

/Jarkko

Re: [tpmdd-devel] [PATCH 2/3] tpm: Get rid of chip->pdev

[Stefan Berger]

On 02/12/2016 07:04 PM, Jason Gunthorpe wrote:
> This is a hold over from before the struct device conversion.
>
> - All prints should be using &chip->dev, which is the Linux
>    standard. This changes prints to use tpm0 as the device name,
>    not the PnP/etc ID.
> - The few places involving sysfs/modules that really do need the
>    parent just use chip->dev.parent instead
> - We no longer need to get_device(pdev) in any places since it is no
>    longer used by any of the code. The kref on the parent is held
>    by the device core during device_add and dropped in device_del
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---

> @@ -347,8 +347,8 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>    */
>   static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
>   {
> -	struct device *dev = chip->pdev;
> -	struct i2c_client *client = to_i2c_client(dev);
> +	struct device *dev = &chip->dev;
This looks wrong or are you fixing a bug here? chip->dev.parent ?
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);

Does this need to be replaced ?

     Stefan

Re: [tpmdd-devel] [PATCH 2/3] tpm: Get rid of chip->pdev

[Jason Gunthorpe]

On Sat, Feb 13, 2016 at 10:39:11AM -0500, Stefan Berger wrote:

> >@@ -347,8 +347,8 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> >   */
> >  static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
> >  {
> >-	struct device *dev = chip->pdev;
> >-	struct i2c_client *client = to_i2c_client(dev);
> >+	struct device *dev = &chip->dev;
> This looks wrong or are you fixing a bug here? chip->dev.parent ?

No, in this function dev is only used for dev_err/etc so it should be
changed, like every other dev_err/etc call in this patch.

Expect for this use:

> >+	struct i2c_client *client = to_i2c_client(chip->dev.parent);
> 
> Does this need to be replaced ?

Which must stay as the parent.

Jason

Re: [PATCH 2/3] tpm: Get rid of chip->pdev

[Jarkko Sakkinen]

On Fri, Feb 12, 2016 at 05:04:30PM -0700, Jason Gunthorpe wrote:
> This is a hold over from before the struct device conversion.
> 
> - All prints should be using &chip->dev, which is the Linux
>   standard. This changes prints to use tpm0 as the device name,
>   not the PnP/etc ID.
> - The few places involving sysfs/modules that really do need the
>   parent just use chip->dev.parent instead
> - We no longer need to get_device(pdev) in any places since it is no
>   longer used by any of the code. The kref on the parent is held
>   by the device core during device_add and dropped in device_del

The change makes obviously sense but is too big infrastructure change
for 4.5.

I've now managed to find time to quickly try out quickly Stefans code
and got an impression how it works and understand his concerns.

I would still merge this and would not mix addressing those concerns
to this patch review since vTPM is still in RFC phase.

> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm-chip.c         | 15 ++++++---------
>  drivers/char/tpm/tpm-dev.c          |  4 +---
>  drivers/char/tpm/tpm-interface.c    | 30 ++++++++++++++++--------------
>  drivers/char/tpm/tpm-sysfs.c        |  6 +++---
>  drivers/char/tpm/tpm.h              |  3 +--
>  drivers/char/tpm/tpm2-cmd.c         |  8 ++++----
>  drivers/char/tpm/tpm_atmel.c        | 14 +++++++-------
>  drivers/char/tpm/tpm_i2c_atmel.c    | 16 ++++++++--------
>  drivers/char/tpm/tpm_i2c_infineon.c |  6 +++---
>  drivers/char/tpm/tpm_i2c_nuvoton.c  | 26 +++++++++++++-------------
>  drivers/char/tpm/tpm_infineon.c     | 22 +++++++++++-----------
>  drivers/char/tpm/tpm_nsc.c          | 20 ++++++++++----------
>  drivers/char/tpm/tpm_tis.c          | 16 ++++++++--------
>  13 files changed, 91 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index ae2fed8a162b..b1364bf62492 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -49,7 +49,7 @@ struct tpm_chip *tpm_chip_find_get(int chip_num)
>  		if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
>  			continue;
>  
> -		if (try_module_get(pos->pdev->driver->owner)) {
> +		if (try_module_get(pos->dev.parent->driver->owner)) {
>  			chip = pos;
>  			break;
>  		}
> @@ -114,13 +114,11 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  
>  	scnprintf(chip->devname, sizeof(chip->devname), "tpm%d", chip->dev_num);
>  
> -	chip->pdev = dev;
> -
>  	dev_set_drvdata(dev, chip);
>  
>  	chip->dev.class = tpm_class;
>  	chip->dev.release = tpm_dev_release;
> -	chip->dev.parent = chip->pdev;
> +	chip->dev.parent = dev;
>  #ifdef CONFIG_ACPI
>  	chip->dev.groups = chip->groups;
>  #endif
> @@ -135,7 +133,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  	device_initialize(&chip->dev);
>  
>  	cdev_init(&chip->cdev, &tpm_fops);
> -	chip->cdev.owner = chip->pdev->driver->owner;
> +	chip->cdev.owner = dev->driver->owner;
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
>  
>  	return chip;
> @@ -236,9 +234,8 @@ int tpm_chip_register(struct tpm_chip *chip)
>  	chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>  
>  	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> -		rc = __compat_only_sysfs_link_entry_to_kobj(&chip->pdev->kobj,
> -							    &chip->dev.kobj,
> -							    "ppi");
> +		rc = __compat_only_sysfs_link_entry_to_kobj(
> +		    &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");

Did you run this through checkpatch.pl?

>  		if (rc && rc != -ENOENT) {
>  			tpm_chip_unregister(chip);
>  			return rc;
> @@ -273,7 +270,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>  	synchronize_rcu();
>  
>  	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> -		sysfs_remove_link(&chip->pdev->kobj, "ppi");
> +		sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
>  
>  	tpm1_chip_unregister(chip);
>  	tpm_dev_del_device(chip);
> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> index de0337ebd658..4009765c14fd 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -61,7 +61,7 @@ static int tpm_open(struct inode *inode, struct file *file)
>  	 * by the check of is_open variable, which is protected
>  	 * by driver_lock. */
>  	if (test_and_set_bit(0, &chip->is_open)) {
> -		dev_dbg(chip->pdev, "Another process owns this TPM\n");
> +		dev_dbg(&chip->dev, "Another process owns this TPM\n");
>  		return -EBUSY;
>  	}
>  
> @@ -79,7 +79,6 @@ static int tpm_open(struct inode *inode, struct file *file)
>  	INIT_WORK(&priv->work, timeout_work);
>  
>  	file->private_data = priv;
> -	get_device(chip->pdev);
>  	return 0;
>  }
>  
> @@ -166,7 +165,6 @@ static int tpm_release(struct inode *inode, struct file *file)
>  	file->private_data = NULL;
>  	atomic_set(&priv->data_pending, 0);
>  	clear_bit(0, &priv->chip->is_open);
> -	put_device(priv->chip->pdev);
>  	kfree(priv);
>  	return 0;
>  }
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index e2fa89c88304..483f86ff6a0a 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -345,7 +345,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>  	if (count == 0)
>  		return -ENODATA;
>  	if (count > bufsiz) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"invalid count value %x %zx\n", count, bufsiz);
>  		return -E2BIG;
>  	}
> @@ -354,7 +354,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>  
>  	rc = chip->ops->send(chip, (u8 *) buf, count);
>  	if (rc < 0) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"tpm_transmit: tpm_send: error %zd\n", rc);
>  		goto out;
>  	}
> @@ -373,7 +373,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>  			goto out_recv;
>  
>  		if (chip->ops->req_canceled(chip, status)) {
> -			dev_err(chip->pdev, "Operation Canceled\n");
> +			dev_err(&chip->dev, "Operation Canceled\n");
>  			rc = -ECANCELED;
>  			goto out;
>  		}
> @@ -383,14 +383,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
>  	} while (time_before(jiffies, stop));
>  
>  	chip->ops->cancel(chip);
> -	dev_err(chip->pdev, "Operation Timed out\n");
> +	dev_err(&chip->dev, "Operation Timed out\n");
>  	rc = -ETIME;
>  	goto out;
>  
>  out_recv:
>  	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
>  	if (rc < 0)
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"tpm_transmit: tpm_recv: error %zd\n", rc);
>  out:
>  	mutex_unlock(&chip->tpm_mutex);
> @@ -416,7 +416,7 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, void *cmd,
>  
>  	err = be32_to_cpu(header->return_code);
>  	if (err != 0 && desc)
> -		dev_err(chip->pdev, "A TPM error (%d) occurred %s\n", err,
> +		dev_err(&chip->dev, "A TPM error (%d) occurred %s\n", err,
>  			desc);
>  
>  	return err;
> @@ -527,7 +527,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  	if (rc == TPM_ERR_INVALID_POSTINIT) {
>  		/* The TPM is not started, we are the first to talk to it.
>  		   Execute a startup command. */
> -		dev_info(chip->pdev, "Issuing TPM_STARTUP");
> +		dev_info(&chip->dev, "Issuing TPM_STARTUP");
>  		if (tpm_startup(chip, TPM_ST_CLEAR))
>  			return rc;
>  
> @@ -539,7 +539,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  				  NULL);
>  	}
>  	if (rc) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"A TPM error (%zd) occurred attempting to determine the timeouts\n",
>  			rc);
>  		goto duration;
> @@ -578,7 +578,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  
>  	/* Report adjusted timeouts */
>  	if (chip->vendor.timeout_adjusted) {
> -		dev_info(chip->pdev,
> +		dev_info(&chip->dev,
>  			 HW_ERR "Adjusting reported timeouts: A %lu->%luus B %lu->%luus C %lu->%luus D %lu->%luus\n",
>  			 old_timeout[0], new_timeout[0],
>  			 old_timeout[1], new_timeout[1],
> @@ -625,7 +625,7 @@ duration:
>  		chip->vendor.duration[TPM_MEDIUM] *= 1000;
>  		chip->vendor.duration[TPM_LONG] *= 1000;
>  		chip->vendor.duration_adjusted = true;
> -		dev_info(chip->pdev, "Adjusting TPM timeout parameters.");
> +		dev_info(&chip->dev, "Adjusting TPM timeout parameters.");
>  	}
>  	return 0;
>  }
> @@ -815,7 +815,9 @@ int tpm_do_selftest(struct tpm_chip *chip)
>  		 * around 300ms while the self test is ongoing, keep trying
>  		 * until the self test duration expires. */
>  		if (rc == -ETIME) {
> -			dev_info(chip->pdev, HW_ERR "TPM command timed out during continue self test");
> +			dev_info(
> +			    &chip->dev, HW_ERR
> +			    "TPM command timed out during continue self test");

Did you run this through checkpatch.pl?

>  			msleep(delay_msec);
>  			continue;
>  		}
> @@ -825,7 +827,7 @@ int tpm_do_selftest(struct tpm_chip *chip)
>  
>  		rc = be32_to_cpu(cmd.header.out.return_code);
>  		if (rc == TPM_ERR_DISABLED || rc == TPM_ERR_DEACTIVATED) {
> -			dev_info(chip->pdev,
> +			dev_info(&chip->dev,
>  				 "TPM is disabled/deactivated (0x%X)\n", rc);
>  			/* TPM is disabled and/or deactivated; driver can
>  			 * proceed and TPM does handle commands for
> @@ -978,10 +980,10 @@ int tpm_pm_suspend(struct device *dev)
>  	}
>  
>  	if (rc)
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"Error (%d) sending savestate before suspend\n", rc);
>  	else if (try > 0)
> -		dev_warn(chip->pdev, "TPM savestate took %dms\n",
> +		dev_warn(&chip->dev, "TPM savestate took %dms\n",
>  			 try * TPM_TIMEOUT_RETRY);
>  
>  	return rc;
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index ee66fd4673f3..d93736aa2703 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -284,16 +284,16 @@ static const struct attribute_group tpm_dev_group = {
>  int tpm_sysfs_add_device(struct tpm_chip *chip)
>  {
>  	int err;
> -	err = sysfs_create_group(&chip->pdev->kobj,
> +	err = sysfs_create_group(&chip->dev.parent->kobj,
>  				 &tpm_dev_group);
>  
>  	if (err)
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"failed to create sysfs attributes, %d\n", err);
>  	return err;
>  }
>  
>  void tpm_sysfs_del_device(struct tpm_chip *chip)
>  {
> -	sysfs_remove_group(&chip->pdev->kobj, &tpm_dev_group);
> +	sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group);
>  }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f6ba79d91857..371f75f4d2a7 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -174,7 +174,6 @@ enum tpm_chip_flags {
>  };
>  
>  struct tpm_chip {
> -	struct device *pdev;	/* Device stuff */
>  	struct device dev;
>  	struct cdev cdev;
>  
> @@ -206,7 +205,7 @@ struct tpm_chip {
>  
>  static inline void tpm_chip_put(struct tpm_chip *chip)
>  {
> -	module_put(chip->pdev->driver->owner);
> +	module_put(chip->dev.parent->driver->owner);
>  	put_device(&chip->dev);
>  }
>  
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 45a634016f95..6eb8b74b2d38 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -597,7 +597,7 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
>  
>  	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
>  	if (rc) {
> -		dev_warn(chip->pdev, "0x%08x was not flushed, out of memory\n",
> +		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
>  			 handle);
>  		return;
>  	}
> @@ -606,7 +606,7 @@ static void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
>  
>  	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, "flushing context");
>  	if (rc)
> -		dev_warn(chip->pdev, "0x%08x was not flushed, rc=%d\n", handle,
> +		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
>  			 rc);
>  
>  	tpm_buf_destroy(&buf);
> @@ -770,7 +770,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>  	 * except print the error code on a system failure.
>  	 */
>  	if (rc < 0)
> -		dev_warn(chip->pdev, "transmit returned %d while stopping the TPM",
> +		dev_warn(&chip->dev, "transmit returned %d while stopping the TPM",
>  			 rc);
>  }
>  EXPORT_SYMBOL_GPL(tpm2_shutdown);
> @@ -837,7 +837,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
>  	 * immediately. This is a workaround for that.
>  	 */
>  	if (rc == TPM2_RC_TESTING) {
> -		dev_warn(chip->pdev, "Got RC_TESTING, ignoring\n");
> +		dev_warn(&chip->dev, "Got RC_TESTING, ignoring\n");
>  		rc = 0;
>  	}
>  
> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> index dfadad0916a1..a48a878f791d 100644
> --- a/drivers/char/tpm/tpm_atmel.c
> +++ b/drivers/char/tpm/tpm_atmel.c
> @@ -49,7 +49,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	for (i = 0; i < 6; i++) {
>  		status = ioread8(chip->vendor.iobase + 1);
>  		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> -			dev_err(chip->pdev, "error reading header\n");
> +			dev_err(&chip->dev, "error reading header\n");
>  			return -EIO;
>  		}
>  		*buf++ = ioread8(chip->vendor.iobase);
> @@ -60,12 +60,12 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	size = be32_to_cpu(*native_size);
>  
>  	if (count < size) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"Recv size(%d) less than available space\n", size);
>  		for (; i < size; i++) {	/* clear the waiting data anyway */
>  			status = ioread8(chip->vendor.iobase + 1);
>  			if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> -				dev_err(chip->pdev, "error reading data\n");
> +				dev_err(&chip->dev, "error reading data\n");
>  				return -EIO;
>  			}
>  		}
> @@ -76,7 +76,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	for (; i < size; i++) {
>  		status = ioread8(chip->vendor.iobase + 1);
>  		if ((status & ATML_STATUS_DATA_AVAIL) == 0) {
> -			dev_err(chip->pdev, "error reading data\n");
> +			dev_err(&chip->dev, "error reading data\n");
>  			return -EIO;
>  		}
>  		*buf++ = ioread8(chip->vendor.iobase);
> @@ -86,7 +86,7 @@ static int tpm_atml_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	status = ioread8(chip->vendor.iobase + 1);
>  
>  	if (status & ATML_STATUS_DATA_AVAIL) {
> -		dev_err(chip->pdev, "data available is stuck\n");
> +		dev_err(&chip->dev, "data available is stuck\n");
>  		return -EIO;
>  	}
>  
> @@ -97,9 +97,9 @@ static int tpm_atml_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  {
>  	int i;
>  
> -	dev_dbg(chip->pdev, "tpm_atml_send:\n");
> +	dev_dbg(&chip->dev, "tpm_atml_send:\n");
>  	for (i = 0; i < count; i++) {
> -		dev_dbg(chip->pdev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
> +		dev_dbg(&chip->dev, "%d 0x%x(%d)\n",  i, buf[i], buf[i]);
>   		iowrite8(buf[i], chip->vendor.iobase);
>  	}
>  
> diff --git a/drivers/char/tpm/tpm_i2c_atmel.c b/drivers/char/tpm/tpm_i2c_atmel.c
> index 8dfb88b9739c..dd8f0eb3170a 100644
> --- a/drivers/char/tpm/tpm_i2c_atmel.c
> +++ b/drivers/char/tpm/tpm_i2c_atmel.c
> @@ -52,7 +52,7 @@ struct priv_data {
>  static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  {
>  	struct priv_data *priv = chip->vendor.priv;
> -	struct i2c_client *client = to_i2c_client(chip->pdev);
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);

I saw Stefans remark. If pdev is gone, isn't this right? Please correct,
if I'm too tired to notice something.

>  	s32 status;
>  
>  	priv->len = 0;
> @@ -62,7 +62,7 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  
>  	status = i2c_master_send(client, buf, len);
>  
> -	dev_dbg(chip->pdev,
> +	dev_dbg(&chip->dev,
>  		"%s(buf=%*ph len=%0zx) -> sts=%d\n", __func__,
>  		(int)min_t(size_t, 64, len), buf, len, status);
>  	return status;
> @@ -71,7 +71,7 @@ static int i2c_atmel_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  {
>  	struct priv_data *priv = chip->vendor.priv;
> -	struct i2c_client *client = to_i2c_client(chip->pdev);
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	struct tpm_output_header *hdr =
>  		(struct tpm_output_header *)priv->buffer;
>  	u32 expected_len;
> @@ -88,7 +88,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		return -ENOMEM;
>  
>  	if (priv->len >= expected_len) {
> -		dev_dbg(chip->pdev,
> +		dev_dbg(&chip->dev,
>  			"%s early(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
>  			(int)min_t(size_t, 64, expected_len), buf, count,
>  			expected_len);
> @@ -97,7 +97,7 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	}
>  
>  	rc = i2c_master_recv(client, buf, expected_len);
> -	dev_dbg(chip->pdev,
> +	dev_dbg(&chip->dev,
>  		"%s reread(buf=%*ph count=%0zx) -> ret=%d\n", __func__,
>  		(int)min_t(size_t, 64, expected_len), buf, count,
>  		expected_len);
> @@ -106,13 +106,13 @@ static int i2c_atmel_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  
>  static void i2c_atmel_cancel(struct tpm_chip *chip)
>  {
> -	dev_err(chip->pdev, "TPM operation cancellation was requested, but is not supported");
> +	dev_err(&chip->dev, "TPM operation cancellation was requested, but is not supported");
>  }
>  
>  static u8 i2c_atmel_read_status(struct tpm_chip *chip)
>  {
>  	struct priv_data *priv = chip->vendor.priv;
> -	struct i2c_client *client = to_i2c_client(chip->pdev);
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	int rc;
>  
>  	/* The TPM fails the I2C read until it is ready, so we do the entire
> @@ -125,7 +125,7 @@ static u8 i2c_atmel_read_status(struct tpm_chip *chip)
>  	/* Once the TPM has completed the command the command remains readable
>  	 * until another command is issued. */
>  	rc = i2c_master_recv(client, priv->buffer, sizeof(priv->buffer));
> -	dev_dbg(chip->pdev,
> +	dev_dbg(&chip->dev,
>  		"%s: sts=%d", __func__, rc);
>  	if (rc <= 0)
>  		return 0;
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c b/drivers/char/tpm/tpm_i2c_infineon.c
> index 63d5d22e9e60..f2aa99e34b4b 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -446,7 +446,7 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	/* read first 10 bytes, including tag, paramsize, and result */
>  	size = recv_data(chip, buf, TPM_HEADER_SIZE);
>  	if (size < TPM_HEADER_SIZE) {
> -		dev_err(chip->pdev, "Unable to read header\n");
> +		dev_err(&chip->dev, "Unable to read header\n");
>  		goto out;
>  	}
>  
> @@ -459,14 +459,14 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	size += recv_data(chip, &buf[TPM_HEADER_SIZE],
>  			  expected - TPM_HEADER_SIZE);
>  	if (size < expected) {
> -		dev_err(chip->pdev, "Unable to read remainder of result\n");
> +		dev_err(&chip->dev, "Unable to read remainder of result\n");
>  		size = -ETIME;
>  		goto out;
>  	}
>  
>  	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c, &status);
>  	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
> -		dev_err(chip->pdev, "Error left over data\n");
> +		dev_err(&chip->dev, "Error left over data\n");
>  		size = -EIO;
>  		goto out;
>  	}
> diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
> index 847f1597fe9b..8fb378f502e4 100644
> --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> @@ -96,13 +96,13 @@ static s32 i2c_nuvoton_write_buf(struct i2c_client *client, u8 offset, u8 size,
>  /* read TPM_STS register */
>  static u8 i2c_nuvoton_read_status(struct tpm_chip *chip)
>  {
> -	struct i2c_client *client = to_i2c_client(chip->pdev);
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	s32 status;
>  	u8 data;
>  
>  	status = i2c_nuvoton_read_buf(client, TPM_STS, 1, &data);
>  	if (status <= 0) {
> -		dev_err(chip->pdev, "%s() error return %d\n", __func__,
> +		dev_err(&chip->dev, "%s() error return %d\n", __func__,
>  			status);
>  		data = TPM_STS_ERR_VAL;
>  	}
> @@ -127,13 +127,13 @@ static s32 i2c_nuvoton_write_status(struct i2c_client *client, u8 data)
>  /* write commandReady to TPM_STS register */
>  static void i2c_nuvoton_ready(struct tpm_chip *chip)
>  {
> -	struct i2c_client *client = to_i2c_client(chip->pdev);
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	s32 status;
>  
>  	/* this causes the current command to be aborted */
>  	status = i2c_nuvoton_write_status(client, TPM_STS_COMMAND_READY);
>  	if (status < 0)
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"%s() fail to write TPM_STS.commandReady\n", __func__);
>  }
>  
> @@ -212,7 +212,7 @@ static int i2c_nuvoton_wait_for_stat(struct tpm_chip *chip, u8 mask, u8 value,
>  				return 0;
>  		} while (time_before(jiffies, stop));
>  	}
> -	dev_err(chip->pdev, "%s(%02x, %02x) -> timeout\n", __func__, mask,
> +	dev_err(&chip->dev, "%s(%02x, %02x) -> timeout\n", __func__, mask,
>  		value);
>  	return -ETIMEDOUT;
>  }
> @@ -240,7 +240,7 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
>  					       &chip->vendor.read_queue) == 0) {
>  		burst_count = i2c_nuvoton_get_burstcount(client, chip);
>  		if (burst_count < 0) {
> -			dev_err(chip->pdev,
> +			dev_err(&chip->dev,
>  				"%s() fail to read burstCount=%d\n", __func__,
>  				burst_count);
>  			return -EIO;
> @@ -249,12 +249,12 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
>  		rc = i2c_nuvoton_read_buf(client, TPM_DATA_FIFO_R,
>  					  bytes2read, &buf[size]);
>  		if (rc < 0) {
> -			dev_err(chip->pdev,
> +			dev_err(&chip->dev,
>  				"%s() fail on i2c_nuvoton_read_buf()=%d\n",
>  				__func__, rc);
>  			return -EIO;
>  		}
> -		dev_dbg(chip->pdev, "%s(%d):", __func__, bytes2read);
> +		dev_dbg(&chip->dev, "%s(%d):", __func__, bytes2read);
>  		size += bytes2read;
>  	}
>  
> @@ -264,8 +264,8 @@ static int i2c_nuvoton_recv_data(struct i2c_client *client,
>  /* Read TPM command results */
>  static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  {
> -	struct device *dev = chip->pdev;
> -	struct i2c_client *client = to_i2c_client(dev);
> +	struct device *dev = &chip->dev;
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	s32 rc;
>  	int expected, status, burst_count, retries, size = 0;
>  
> @@ -334,7 +334,7 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  		break;
>  	}
>  	i2c_nuvoton_ready(chip);
> -	dev_dbg(chip->pdev, "%s() -> %d\n", __func__, size);
> +	dev_dbg(&chip->dev, "%s() -> %d\n", __func__, size);
>  	return size;
>  }
>  
> @@ -347,8 +347,8 @@ static int i2c_nuvoton_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>   */
>  static int i2c_nuvoton_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  {
> -	struct device *dev = chip->pdev;
> -	struct i2c_client *client = to_i2c_client(dev);
> +	struct device *dev = &chip->dev;
> +	struct i2c_client *client = to_i2c_client(chip->dev.parent);
>  	u32 ordinal;
>  	size_t count = 0;
>  	int burst_count, bytes2write, retries, rc = -EIO;
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 6c488e635fdd..e3cf9f3545c5 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -195,9 +195,9 @@ static int wait(struct tpm_chip *chip, int wait_for_bit)
>  	}
>  	if (i == TPM_MAX_TRIES) {	/* timeout occurs */
>  		if (wait_for_bit == STAT_XFE)
> -			dev_err(chip->pdev, "Timeout in wait(STAT_XFE)\n");
> +			dev_err(&chip->dev, "Timeout in wait(STAT_XFE)\n");
>  		if (wait_for_bit == STAT_RDA)
> -			dev_err(chip->pdev, "Timeout in wait(STAT_RDA)\n");
> +			dev_err(&chip->dev, "Timeout in wait(STAT_RDA)\n");
>  		return -EIO;
>  	}
>  	return 0;
> @@ -220,7 +220,7 @@ static void wait_and_send(struct tpm_chip *chip, u8 sendbyte)
>  static void tpm_wtx(struct tpm_chip *chip)
>  {
>  	number_of_wtx++;
> -	dev_info(chip->pdev, "Granting WTX (%02d / %02d)\n",
> +	dev_info(&chip->dev, "Granting WTX (%02d / %02d)\n",
>  		 number_of_wtx, TPM_MAX_WTX_PACKAGES);
>  	wait_and_send(chip, TPM_VL_VER);
>  	wait_and_send(chip, TPM_CTRL_WTX);
> @@ -231,7 +231,7 @@ static void tpm_wtx(struct tpm_chip *chip)
>  
>  static void tpm_wtx_abort(struct tpm_chip *chip)
>  {
> -	dev_info(chip->pdev, "Aborting WTX\n");
> +	dev_info(&chip->dev, "Aborting WTX\n");
>  	wait_and_send(chip, TPM_VL_VER);
>  	wait_and_send(chip, TPM_CTRL_WTX_ABORT);
>  	wait_and_send(chip, 0x00);
> @@ -257,7 +257,7 @@ recv_begin:
>  	}
>  
>  	if (buf[0] != TPM_VL_VER) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"Wrong transport protocol implementation!\n");
>  		return -EIO;
>  	}
> @@ -272,7 +272,7 @@ recv_begin:
>  		}
>  
>  		if ((size == 0x6D00) && (buf[1] == 0x80)) {
> -			dev_err(chip->pdev, "Error handling on vendor layer!\n");
> +			dev_err(&chip->dev, "Error handling on vendor layer!\n");
>  			return -EIO;
>  		}
>  
> @@ -284,7 +284,7 @@ recv_begin:
>  	}
>  
>  	if (buf[1] == TPM_CTRL_WTX) {
> -		dev_info(chip->pdev, "WTX-package received\n");
> +		dev_info(&chip->dev, "WTX-package received\n");
>  		if (number_of_wtx < TPM_MAX_WTX_PACKAGES) {
>  			tpm_wtx(chip);
>  			goto recv_begin;
> @@ -295,14 +295,14 @@ recv_begin:
>  	}
>  
>  	if (buf[1] == TPM_CTRL_WTX_ABORT_ACK) {
> -		dev_info(chip->pdev, "WTX-abort acknowledged\n");
> +		dev_info(&chip->dev, "WTX-abort acknowledged\n");
>  		return size;
>  	}
>  
>  	if (buf[1] == TPM_CTRL_ERROR) {
> -		dev_err(chip->pdev, "ERROR-package received:\n");
> +		dev_err(&chip->dev, "ERROR-package received:\n");
>  		if (buf[4] == TPM_INF_NAK)
> -			dev_err(chip->pdev,
> +			dev_err(&chip->dev,
>  				"-> Negative acknowledgement"
>  				" - retransmit command!\n");
>  		return -EIO;
> @@ -321,7 +321,7 @@ static int tpm_inf_send(struct tpm_chip *chip, u8 * buf, size_t count)
>  
>  	ret = empty_fifo(chip, 1);
>  	if (ret) {
> -		dev_err(chip->pdev, "Timeout while clearing FIFO\n");
> +		dev_err(&chip->dev, "Timeout while clearing FIFO\n");
>  		return -EIO;
>  	}
>  
> diff --git a/drivers/char/tpm/tpm_nsc.c b/drivers/char/tpm/tpm_nsc.c
> index 289389ecef84..766370bed60c 100644
> --- a/drivers/char/tpm/tpm_nsc.c
> +++ b/drivers/char/tpm/tpm_nsc.c
> @@ -113,7 +113,7 @@ static int nsc_wait_for_ready(struct tpm_chip *chip)
>  	}
>  	while (time_before(jiffies, stop));
>  
> -	dev_info(chip->pdev, "wait for ready failed\n");
> +	dev_info(&chip->dev, "wait for ready failed\n");
>  	return -EBUSY;
>  }
>  
> @@ -129,12 +129,12 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  		return -EIO;
>  
>  	if (wait_for_stat(chip, NSC_STATUS_F0, NSC_STATUS_F0, &data) < 0) {
> -		dev_err(chip->pdev, "F0 timeout\n");
> +		dev_err(&chip->dev, "F0 timeout\n");
>  		return -EIO;
>  	}
>  	if ((data =
>  	     inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_NORMAL) {
> -		dev_err(chip->pdev, "not in normal mode (0x%x)\n",
> +		dev_err(&chip->dev, "not in normal mode (0x%x)\n",
>  			data);
>  		return -EIO;
>  	}
> @@ -143,7 +143,7 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  	for (p = buffer; p < &buffer[count]; p++) {
>  		if (wait_for_stat
>  		    (chip, NSC_STATUS_OBF, NSC_STATUS_OBF, &data) < 0) {
> -			dev_err(chip->pdev,
> +			dev_err(&chip->dev,
>  				"OBF timeout (while reading data)\n");
>  			return -EIO;
>  		}
> @@ -154,11 +154,11 @@ static int tpm_nsc_recv(struct tpm_chip *chip, u8 * buf, size_t count)
>  
>  	if ((data & NSC_STATUS_F0) == 0 &&
>  	(wait_for_stat(chip, NSC_STATUS_F0, NSC_STATUS_F0, &data) < 0)) {
> -		dev_err(chip->pdev, "F0 not set\n");
> +		dev_err(&chip->dev, "F0 not set\n");
>  		return -EIO;
>  	}
>  	if ((data = inb(chip->vendor.base + NSC_DATA)) != NSC_COMMAND_EOC) {
> -		dev_err(chip->pdev,
> +		dev_err(&chip->dev,
>  			"expected end of command(0x%x)\n", data);
>  		return -EIO;
>  	}
> @@ -189,19 +189,19 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
>  		return -EIO;
>  
>  	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
> -		dev_err(chip->pdev, "IBF timeout\n");
> +		dev_err(&chip->dev, "IBF timeout\n");
>  		return -EIO;
>  	}
>  
>  	outb(NSC_COMMAND_NORMAL, chip->vendor.base + NSC_COMMAND);
>  	if (wait_for_stat(chip, NSC_STATUS_IBR, NSC_STATUS_IBR, &data) < 0) {
> -		dev_err(chip->pdev, "IBR timeout\n");
> +		dev_err(&chip->dev, "IBR timeout\n");
>  		return -EIO;
>  	}
>  
>  	for (i = 0; i < count; i++) {
>  		if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
> -			dev_err(chip->pdev,
> +			dev_err(&chip->dev,
>  				"IBF timeout (while writing data)\n");
>  			return -EIO;
>  		}
> @@ -209,7 +209,7 @@ static int tpm_nsc_send(struct tpm_chip *chip, u8 * buf, size_t count)
>  	}
>  
>  	if (wait_for_stat(chip, NSC_STATUS_IBF, 0, &data) < 0) {
> -		dev_err(chip->pdev, "IBF timeout\n");
> +		dev_err(&chip->dev, "IBF timeout\n");
>  		return -EIO;
>  	}
>  	outb(NSC_COMMAND_EOC, chip->vendor.base + NSC_COMMAND);
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index 8a3509cb10da..f15466b7fb78 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -293,7 +293,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	/* read first 10 bytes, including tag, paramsize, and result */
>  	if ((size =
>  	     recv_data(chip, buf, TPM_HEADER_SIZE)) < TPM_HEADER_SIZE) {
> -		dev_err(chip->pdev, "Unable to read header\n");
> +		dev_err(&chip->dev, "Unable to read header\n");
>  		goto out;
>  	}
>  
> @@ -306,7 +306,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  	if ((size +=
>  	     recv_data(chip, &buf[TPM_HEADER_SIZE],
>  		       expected - TPM_HEADER_SIZE)) < expected) {
> -		dev_err(chip->pdev, "Unable to read remainder of result\n");
> +		dev_err(&chip->dev, "Unable to read remainder of result\n");
>  		size = -ETIME;
>  		goto out;
>  	}
> @@ -315,7 +315,7 @@ static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>  			  &chip->vendor.int_queue, false);
>  	status = tpm_tis_status(chip);
>  	if (status & TPM_STS_DATA_AVAIL) {	/* retry? */
> -		dev_err(chip->pdev, "Error left over data\n");
> +		dev_err(&chip->dev, "Error left over data\n");
>  		size = -EIO;
>  		goto out;
>  	}
> @@ -401,7 +401,7 @@ static void disable_interrupts(struct tpm_chip *chip)
>  	iowrite32(intmask,
>  		  chip->vendor.iobase +
>  		  TPM_INT_ENABLE(chip->vendor.locality));
> -	devm_free_irq(chip->pdev, chip->vendor.irq, chip);
> +	devm_free_irq(&chip->dev, chip->vendor.irq, chip);
>  	chip->vendor.irq = 0;
>  }
>  
> @@ -530,7 +530,7 @@ static int probe_itpm(struct tpm_chip *chip)
>  
>  	rc = tpm_tis_send_data(chip, cmd_getticks, len);
>  	if (rc == 0) {
> -		dev_info(chip->pdev, "Detected an iTPM.\n");
> +		dev_info(&chip->dev, "Detected an iTPM.\n");
>  		rc = 1;
>  	} else
>  		rc = -EFAULT;
> @@ -609,9 +609,9 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
>  	struct priv_data *priv = chip->vendor.priv;
>  	u8 original_int_vec;
>  
> -	if (devm_request_irq(chip->pdev, irq, tis_int_handler, flags,
> +	if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
>  			     chip->devname, chip) != 0) {
> -		dev_info(chip->pdev, "Unable to request irq: %d for probe\n",
> +		dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
>  			 irq);
>  		return -1;
>  	}
> @@ -812,7 +812,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
>  			tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
>  						 tpm_info->irq);
>  			if (!chip->vendor.irq)
> -				dev_err(chip->pdev, FW_BUG
> +				dev_err(&chip->dev, FW_BUG
>  					"TPM interrupt not working, polling instead\n");
>  		} else
>  			tpm_tis_probe_irq(chip, intmask);
> -- 
> 2.1.4
> 

/Jarkko

[PATCH 3/3] tpm: Get rid of devname

[Jason Gunthorpe]

Now that we have a proper struct device just use dev_name() to
access this value instead of keeping two copies.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c        | 17 +++++++++++------
 drivers/char/tpm/tpm.h             |  1 -
 drivers/char/tpm/tpm_eventlog.c    |  2 +-
 drivers/char/tpm/tpm_eventlog.h    |  2 +-
 drivers/char/tpm/tpm_i2c_nuvoton.c |  2 +-
 drivers/char/tpm/tpm_tis.c         |  2 +-
 6 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index b1364bf62492..caa52a6110ec 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -90,6 +90,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 				 const struct tpm_class_ops *ops)
 {
 	struct tpm_chip *chip;
+	int err;
 
 	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
 	if (chip == NULL)
@@ -112,8 +113,6 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 
 	set_bit(chip->dev_num, dev_mask);
 
-	scnprintf(chip->devname, sizeof(chip->devname), "tpm%d", chip->dev_num);
-
 	dev_set_drvdata(dev, chip);
 
 	chip->dev.class = tpm_class;
@@ -128,7 +127,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 	else
 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
 
-	dev_set_name(&chip->dev, "%s", chip->devname);
+	err = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);
+	if (err)
+		goto out;
 
 	device_initialize(&chip->dev);
 
@@ -137,6 +138,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 
 	return chip;
+
+out:
+	put_device(&chip->dev);
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
 
@@ -148,7 +153,7 @@ static int tpm_dev_add_device(struct tpm_chip *chip)
 	if (rc) {
 		dev_err(&chip->dev,
 			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
-			chip->devname, MAJOR(chip->dev.devt),
+			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
 		device_unregister(&chip->dev);
@@ -159,7 +164,7 @@ static int tpm_dev_add_device(struct tpm_chip *chip)
 	if (rc) {
 		dev_err(&chip->dev,
 			"unable to device_register() %s, major %d, minor %d, err=%d\n",
-			chip->devname, MAJOR(chip->dev.devt),
+			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
 		return rc;
@@ -185,7 +190,7 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 	if (rc)
 		return rc;
 
-	chip->bios_dir = tpm_bios_log_setup(chip->devname);
+	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
 
 	return 0;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 371f75f4d2a7..a53fc699027b 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -181,7 +181,6 @@ struct tpm_chip {
 	unsigned int flags;
 
 	int dev_num;		/* /dev/tpm# */
-	char devname[7];
 	unsigned long is_open;	/* only one allowed */
 	int time_expired;
 
diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index bd72fb04225e..49e50976efc8 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -397,7 +397,7 @@ static int is_bad(void *p)
 	return 0;
 }
 
-struct dentry **tpm_bios_log_setup(char *name)
+struct dentry **tpm_bios_log_setup(const char *name)
 {
 	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
 
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 267bfbd1b7bb..f072a1a1d5cc 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -77,7 +77,7 @@ int read_log(struct tpm_bios_log *log);
 
 #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
 	defined(CONFIG_ACPI)
-extern struct dentry **tpm_bios_log_setup(char *);
+extern struct dentry **tpm_bios_log_setup(const char *name);
 extern void tpm_bios_log_teardown(struct dentry **);
 #else
 static inline struct dentry **tpm_bios_log_setup(char *name)
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index 8fb378f502e4..6dd74d114fb3 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -560,7 +560,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->devname,
+				      dev_name(&chip->dev),
 				      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 f15466b7fb78..0cd57371b755 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -610,7 +610,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
 	u8 original_int_vec;
 
 	if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
-			     chip->devname, chip) != 0) {
+			     dev_name(&chip->dev), chip) != 0) {
 		dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
 			 irq);
 		return -1;
-- 
2.1.4

Re: [PATCH 3/3] tpm: Get rid of devname

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

Hi Jason,

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.5-rc3 next-20160212]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jason-Gunthorpe/tpm-Hold-the-kref-during-tpm_chip_find_get/20160213-080824
config: xtensa-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/char/tpm/tpm-chip.c: In function 'tpm1_chip_register':
>> drivers/char/tpm/tpm-chip.c:193:19: warning: passing argument 1 of 'tpm_bios_log_setup' discards 'const' qualifier from pointer target type
     chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
                      ^
   In file included from drivers/char/tpm/tpm-chip.c:30:0:
   drivers/char/tpm/tpm_eventlog.h:83:31: note: expected 'char *' but argument is of type 'const char *'
    static inline struct dentry **tpm_bios_log_setup(char *name)
                                  ^

vim +193 drivers/char/tpm/tpm-chip.c

   177	{
   178		cdev_del(&chip->cdev);
   179		device_unregister(&chip->dev);
   180	}
   181	
   182	static int tpm1_chip_register(struct tpm_chip *chip)
   183	{
   184		int rc;
   185	
   186		if (chip->flags & TPM_CHIP_FLAG_TPM2)
   187			return 0;
   188	
   189		rc = tpm_sysfs_add_device(chip);
   190		if (rc)
   191			return rc;
   192	
 > 193		chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
   194	
   195		return 0;
   196	}
   197	
   198	static void tpm1_chip_unregister(struct tpm_chip *chip)
   199	{
   200		if (chip->flags & TPM_CHIP_FLAG_TPM2)
   201			return;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44085 bytes --]

Re: [PATCH 3/3] tpm: Get rid of devname

[Jason Gunthorpe]

On Sat, Feb 13, 2016 at 09:01:06AM +0800, kbuild test robot wrote:

> url:    https://github.com/0day-ci/linux/commits/Jason-Gunthorpe/tpm-Hold-the-kref-during-tpm_chip_find_get/20160213-080824
> config: xtensa-allyesconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=xtensa 
> 
> All warnings (new ones prefixed by >>):
>
>    drivers/char/tpm/tpm-chip.c: In function 'tpm1_chip_register':
> >> drivers/char/tpm/tpm-chip.c:193:19: warning: passing argument 1 of 'tpm_bios_log_setup' discards 'const' qualifier from pointer target type
>      chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>                       ^
>    In file included from drivers/char/tpm/tpm-chip.c:30:0:
>    drivers/char/tpm/tpm_eventlog.h:83:31: note: expected 'char *' but argument is of type 'const char *'
>     static inline struct dentry **tpm_bios_log_setup(char *name)

Got it, thanks, didn't notice that kconfig variation.

Jason

Re: [PATCH 3/3] tpm: Get rid of devname

[Jarkko Sakkinen]

On Fri, Feb 12, 2016 at 05:04:31PM -0700, Jason Gunthorpe wrote:
> Now that we have a proper struct device just use dev_name() to
> access this value instead of keeping two copies.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c        | 17 +++++++++++------
>  drivers/char/tpm/tpm.h             |  1 -
>  drivers/char/tpm/tpm_eventlog.c    |  2 +-
>  drivers/char/tpm/tpm_eventlog.h    |  2 +-
>  drivers/char/tpm/tpm_i2c_nuvoton.c |  2 +-
>  drivers/char/tpm/tpm_tis.c         |  2 +-
>  6 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index b1364bf62492..caa52a6110ec 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -90,6 +90,7 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  				 const struct tpm_class_ops *ops)
>  {
>  	struct tpm_chip *chip;
> +	int err;
>  
>  	chip = kzalloc(sizeof(*chip), GFP_KERNEL);
>  	if (chip == NULL)
> @@ -112,8 +113,6 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  
>  	set_bit(chip->dev_num, dev_mask);
>  
> -	scnprintf(chip->devname, sizeof(chip->devname), "tpm%d", chip->dev_num);
> -
>  	dev_set_drvdata(dev, chip);
>  
>  	chip->dev.class = tpm_class;
> @@ -128,7 +127,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  	else
>  		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
>  
> -	dev_set_name(&chip->dev, "%s", chip->devname);
> +	err = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);
> +	if (err)
> +		goto out;
>  
>  	device_initialize(&chip->dev);
>  
> @@ -137,6 +138,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
>  
>  	return chip;
> +
> +out:
> +	put_device(&chip->dev);
> +	return ERR_PTR(err);
>  }
>  EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
>  
> @@ -148,7 +153,7 @@ static int tpm_dev_add_device(struct tpm_chip *chip)
>  	if (rc) {
>  		dev_err(&chip->dev,
>  			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> -			chip->devname, MAJOR(chip->dev.devt),
> +			dev_name(&chip->dev), MAJOR(chip->dev.devt),
>  			MINOR(chip->dev.devt), rc);
>  
>  		device_unregister(&chip->dev);
> @@ -159,7 +164,7 @@ static int tpm_dev_add_device(struct tpm_chip *chip)
>  	if (rc) {
>  		dev_err(&chip->dev,
>  			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> -			chip->devname, MAJOR(chip->dev.devt),
> +			dev_name(&chip->dev), MAJOR(chip->dev.devt),
>  			MINOR(chip->dev.devt), rc);
>  
>  		return rc;
> @@ -185,7 +190,7 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>  	if (rc)
>  		return rc;
>  
> -	chip->bios_dir = tpm_bios_log_setup(chip->devname);
> +	chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>  
>  	return 0;
>  }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 371f75f4d2a7..a53fc699027b 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -181,7 +181,6 @@ struct tpm_chip {
>  	unsigned int flags;
>  
>  	int dev_num;		/* /dev/tpm# */
> -	char devname[7];
>  	unsigned long is_open;	/* only one allowed */
>  	int time_expired;
>  
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index bd72fb04225e..49e50976efc8 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -397,7 +397,7 @@ static int is_bad(void *p)
>  	return 0;
>  }
>  
> -struct dentry **tpm_bios_log_setup(char *name)
> +struct dentry **tpm_bios_log_setup(const char *name)
>  {
>  	struct dentry **ret = NULL, *tpm_dir, *bin_file, *ascii_file;
>  
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index 267bfbd1b7bb..f072a1a1d5cc 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -77,7 +77,7 @@ int read_log(struct tpm_bios_log *log);
>  
>  #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
>  	defined(CONFIG_ACPI)
> -extern struct dentry **tpm_bios_log_setup(char *);
> +extern struct dentry **tpm_bios_log_setup(const char *name);
>  extern void tpm_bios_log_teardown(struct dentry **);
>  #else
>  static inline struct dentry **tpm_bios_log_setup(char *name)
> diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
> index 8fb378f502e4..6dd74d114fb3 100644
> --- a/drivers/char/tpm/tpm_i2c_nuvoton.c
> +++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
> @@ -560,7 +560,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->devname,
> +				      dev_name(&chip->dev),
>  				      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 f15466b7fb78..0cd57371b755 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -610,7 +610,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
>  	u8 original_int_vec;
>  
>  	if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
> -			     chip->devname, chip) != 0) {
> +			     dev_name(&chip->dev), chip) != 0) {
>  		dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
>  			 irq);
>  		return -1;
> -- 
> 2.1.4
>