diff mbox

[v3,03/11] tpm: Get rid of devname

Message ID 1455885728-10315-4-git-send-email-stefanb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Berger Feb. 19, 2016, 12:42 p.m. UTC 
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

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        | 16 ++++++++++------
 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         |  4 ++--
 6 files changed, 15 insertions(+), 12 deletions(-)

Comments

Jason Gunthorpe Feb. 22, 2016, 6:19 p.m. UTC | #1 
On Fri, Feb 19, 2016 at 07:42:00AM -0500, Stefan Berger wrote:
>  
> -	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);
>  
> @@ -142,6 +142,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  	}
>  
>  	return chip;
> +
> +out:
> +	put_device(&chip->dev);
> +	return ERR_PTR(err);

Oops, something went wrong here in all the rebasing - dev_set_name has
to be after devince_initialize.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jarkko Sakkinen Feb. 22, 2016, 7:42 p.m. UTC | #2 
On Mon, Feb 22, 2016 at 11:19:29AM -0700, Jason Gunthorpe wrote:
> On Fri, Feb 19, 2016 at 07:42:00AM -0500, Stefan Berger wrote:
> >  
> > -	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);
> >  
> > @@ -142,6 +142,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> >  	}
> >  
> >  	return chip;
> > +
> > +out:
> > +	put_device(&chip->dev);
> > +	return ERR_PTR(err);
> 
> Oops, something went wrong here in all the rebasing - dev_set_name has
> to be after devince_initialize.

Why dev_set_name() cannot be called before device_initialize(). I see
many drivers call it before device_register() like RTC driver just as
an example:

http://lxr.free-electrons.com/source/drivers/rtc/class.c?v4.4#L226

If the call order is wrong, shouldn't fix for the call order be a
separate patch?

> Jason

/Jarkko

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe Feb. 22, 2016, 7:58 p.m. UTC | #3 
On Mon, Feb 22, 2016 at 09:42:02PM +0200, Jarkko Sakkinen wrote:
> > Oops, something went wrong here in all the rebasing - dev_set_name has
> > to be after devince_initialize.
> 
> Why dev_set_name() cannot be called before device_initialize(). I see
> many drivers call it before device_register() like RTC driver just as
> an example:

Yep, you are right, it still looks nicer with the init earlier, but
the later patch does that anyhow.

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe Feb. 22, 2016, 8:34 p.m. UTC | #4 
On Mon, Feb 22, 2016 at 12:58:16PM -0700, Jason Gunthorpe wrote:
> On Mon, Feb 22, 2016 at 09:42:02PM +0200, Jarkko Sakkinen wrote:
> > > Oops, something went wrong here in all the rebasing - dev_set_name has
> > > to be after devince_initialize.
> > 
> > Why dev_set_name() cannot be called before device_initialize(). I see
> > many drivers call it before device_register() like RTC driver just as
> > an example:
> 
> Yep, you are right, it still looks nicer with the init earlier, but
> the later patch does that anyhow.

Sorry, to be clear, the dev_set_name is OK, but the error unwind for
it in this patch is not OK, it requires device_initialize to have been
called.

The change should still be made..

Jason

------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Stefan Berger Feb. 23, 2016, 12:22 a.m. UTC | #5 
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 
01:19:29 PM:

> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> To: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: tpmdd-devel@lists.sourceforge.net
> Date: 02/22/2016 01:20 PM
> Subject: Re: [tpmdd-devel] [PATCH v3 03/11] tpm: Get rid of devname
> 
> On Fri, Feb 19, 2016 at 07:42:00AM -0500, Stefan Berger wrote:
> > 
> > -   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);
> > 
> > @@ -142,6 +142,10 @@ struct tpm_chip *tpmm_chip_alloc(struct device 
*dev,
> >     }
> > 
> >     return chip;
> > +
> > +out:
> > +   put_device(&chip->dev);
> > +   return ERR_PTR(err);
> 
> Oops, something went wrong here in all the rebasing - dev_set_name has
> to be after devince_initialize.

dev_set_name is now after device_initialize.

   Stefan
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5ed9bed..5df0149 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -111,8 +111,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;
@@ -127,7 +125,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);
 
@@ -142,6 +142,10 @@  struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 	}
 
 	return chip;
+
+out:
+	put_device(&chip->dev);
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(tpmm_chip_alloc);
 
@@ -153,7 +157,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);
@@ -164,7 +168,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;
@@ -190,7 +194,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 1ce6547..31d9a8e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -178,7 +178,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 bd72fb0..49e5097 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 267bfbd..cc9672f 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 *);
 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 a1e1474..d61d43f 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 0b5fc41..37f3d44 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -765,7 +765,7 @@  static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
 				 TPM_INT_VECTOR(chip->vendor.locality));
 			if (devm_request_irq
 			    (dev, i, tis_int_probe, IRQF_SHARED,
-			     chip->devname, chip) != 0) {
+			     dev_name(&chip->dev), chip) != 0) {
 				dev_info(&chip->dev,
 					 "Unable to request irq: %d for probe\n",
 					 i);
@@ -817,7 +817,7 @@  static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
 			 TPM_INT_VECTOR(chip->vendor.locality));
 		if (devm_request_irq
 		    (dev, chip->vendor.irq, tis_int_handler, IRQF_SHARED,
-		     chip->devname, chip) != 0) {
+		     dev_name(&chip->dev), chip) != 0) {
 			dev_info(&chip->dev,
 				 "Unable to request irq: %d for use\n",
 				 chip->vendor.irq);