diff mbox

[v3,06/16] tpm-chip: utilize new cdev_device_add helper function

Message ID 1488783873-2614-7-git-send-email-logang@deltatee.com
State Not Applicable
Headers show

Commit Message

Logan Gunthorpe March 6, 2017, 7:04 a.m. UTC
Replace the open coded registration of the cdev and dev with the
new device_add_cdev() helper. The helper replaces a common pattern by
taking the proper reference against the parent device and adding both
the cdev and the device.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

Comments

Jarkko Sakkinen March 6, 2017, 9:04 p.m. UTC | #1
On Mon, Mar 06, 2017 at 12:04:22AM -0700, Logan Gunthorpe wrote:
> Replace the open coded registration of the cdev and dev with the
> new device_add_cdev() helper. The helper replaces a common pattern by
> taking the proper reference against the parent device and adding both
> the cdev and the device.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm-chip.c | 19 +++----------------
>  1 file changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c406343..935f0e9 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -187,7 +187,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  
>  	cdev_init(&chip->cdev, &tpm_fops);
>  	chip->cdev.owner = THIS_MODULE;
> -	chip->cdev.kobj.parent = &chip->dev.kobj;
>  
>  	return chip;
>  
> @@ -230,27 +229,16 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>  {
>  	int rc;
>  
> -	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
> +	rc = cdev_device_add(&chip->cdev, &chip->dev);
>  	if (rc) {
>  		dev_err(&chip->dev,
> -			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> +			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
>  			dev_name(&chip->dev), MAJOR(chip->dev.devt),
>  			MINOR(chip->dev.devt), rc);
>  
>  		return rc;
>  	}
>  
> -	rc = device_add(&chip->dev);
> -	if (rc) {
> -		dev_err(&chip->dev,
> -			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> -			dev_name(&chip->dev), MAJOR(chip->dev.devt),
> -			MINOR(chip->dev.devt), rc);
> -
> -		cdev_del(&chip->cdev);
> -		return rc;
> -	}
> -
>  	/* Make the chip available. */
>  	mutex_lock(&idr_lock);
>  	idr_replace(&dev_nums_idr, chip, chip->dev_num);
> @@ -261,8 +249,7 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>  
>  static void tpm_del_char_device(struct tpm_chip *chip)
>  {
> -	cdev_del(&chip->cdev);
> -	device_del(&chip->dev);
> +	cdev_device_del(&chip->cdev, &chip->dev);
>  
>  	/* Make the chip unavailable. */
>  	mutex_lock(&idr_lock);
> -- 
> 2.1.4
> 

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

I cannot test this at this point as security tree does not include
the commit that is dependent on this. I'm also wondering if this
commit is even going through my tree to upstream?

/Jarkko
Greg Kroah-Hartman March 7, 2017, 5:31 a.m. UTC | #2
On Mon, Mar 06, 2017 at 11:04:26PM +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 06, 2017 at 12:04:22AM -0700, Logan Gunthorpe wrote:
> > Replace the open coded registration of the cdev and dev with the
> > new device_add_cdev() helper. The helper replaces a common pattern by
> > taking the proper reference against the parent device and adding both
> > the cdev and the device.
> > 
> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > ---
> >  drivers/char/tpm/tpm-chip.c | 19 +++----------------
> >  1 file changed, 3 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index c406343..935f0e9 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -187,7 +187,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
> >  
> >  	cdev_init(&chip->cdev, &tpm_fops);
> >  	chip->cdev.owner = THIS_MODULE;
> > -	chip->cdev.kobj.parent = &chip->dev.kobj;
> >  
> >  	return chip;
> >  
> > @@ -230,27 +229,16 @@ static int tpm_add_char_device(struct tpm_chip *chip)
> >  {
> >  	int rc;
> >  
> > -	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
> > +	rc = cdev_device_add(&chip->cdev, &chip->dev);
> >  	if (rc) {
> >  		dev_err(&chip->dev,
> > -			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> > +			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
> >  			dev_name(&chip->dev), MAJOR(chip->dev.devt),
> >  			MINOR(chip->dev.devt), rc);
> >  
> >  		return rc;
> >  	}
> >  
> > -	rc = device_add(&chip->dev);
> > -	if (rc) {
> > -		dev_err(&chip->dev,
> > -			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> > -			dev_name(&chip->dev), MAJOR(chip->dev.devt),
> > -			MINOR(chip->dev.devt), rc);
> > -
> > -		cdev_del(&chip->cdev);
> > -		return rc;
> > -	}
> > -
> >  	/* Make the chip available. */
> >  	mutex_lock(&idr_lock);
> >  	idr_replace(&dev_nums_idr, chip, chip->dev_num);
> > @@ -261,8 +249,7 @@ static int tpm_add_char_device(struct tpm_chip *chip)
> >  
> >  static void tpm_del_char_device(struct tpm_chip *chip)
> >  {
> > -	cdev_del(&chip->cdev);
> > -	device_del(&chip->dev);
> > +	cdev_device_del(&chip->cdev, &chip->dev);
> >  
> >  	/* Make the chip unavailable. */
> >  	mutex_lock(&idr_lock);
> > -- 
> > 2.1.4
> > 
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> I cannot test this at this point as security tree does not include
> the commit that is dependent on this. I'm also wondering if this
> commit is even going through my tree to upstream?

I can take it all at once through my tree.

thanks,

greg k-h
Jarkko Sakkinen March 7, 2017, 8:31 a.m. UTC | #3
On Tue, Mar 07, 2017 at 06:31:38AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 06, 2017 at 11:04:26PM +0200, Jarkko Sakkinen wrote:
> > On Mon, Mar 06, 2017 at 12:04:22AM -0700, Logan Gunthorpe wrote:
> > > Replace the open coded registration of the cdev and dev with the
> > > new device_add_cdev() helper. The helper replaces a common pattern by
> > > taking the proper reference against the parent device and adding both
> > > the cdev and the device.
> > > 
> > > Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> > > Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > ---
> > >  drivers/char/tpm/tpm-chip.c | 19 +++----------------
> > >  1 file changed, 3 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index c406343..935f0e9 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -187,7 +187,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
> > >  
> > >  	cdev_init(&chip->cdev, &tpm_fops);
> > >  	chip->cdev.owner = THIS_MODULE;
> > > -	chip->cdev.kobj.parent = &chip->dev.kobj;
> > >  
> > >  	return chip;
> > >  
> > > @@ -230,27 +229,16 @@ static int tpm_add_char_device(struct tpm_chip *chip)
> > >  {
> > >  	int rc;
> > >  
> > > -	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
> > > +	rc = cdev_device_add(&chip->cdev, &chip->dev);
> > >  	if (rc) {
> > >  		dev_err(&chip->dev,
> > > -			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> > > +			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
> > >  			dev_name(&chip->dev), MAJOR(chip->dev.devt),
> > >  			MINOR(chip->dev.devt), rc);
> > >  
> > >  		return rc;
> > >  	}
> > >  
> > > -	rc = device_add(&chip->dev);
> > > -	if (rc) {
> > > -		dev_err(&chip->dev,
> > > -			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> > > -			dev_name(&chip->dev), MAJOR(chip->dev.devt),
> > > -			MINOR(chip->dev.devt), rc);
> > > -
> > > -		cdev_del(&chip->cdev);
> > > -		return rc;
> > > -	}
> > > -
> > >  	/* Make the chip available. */
> > >  	mutex_lock(&idr_lock);
> > >  	idr_replace(&dev_nums_idr, chip, chip->dev_num);
> > > @@ -261,8 +249,7 @@ static int tpm_add_char_device(struct tpm_chip *chip)
> > >  
> > >  static void tpm_del_char_device(struct tpm_chip *chip)
> > >  {
> > > -	cdev_del(&chip->cdev);
> > > -	device_del(&chip->dev);
> > > +	cdev_device_del(&chip->cdev, &chip->dev);
> > >  
> > >  	/* Make the chip unavailable. */
> > >  	mutex_lock(&idr_lock);
> > > -- 
> > > 2.1.4
> > > 
> > 
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > 
> > I cannot test this at this point as security tree does not include
> > the commit that is dependent on this. I'm also wondering if this
> > commit is even going through my tree to upstream?
> 
> I can take it all at once through my tree.
> 
> thanks,
> 
> greg k-h

Alright, thank you!

/Jarkko
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c406343..935f0e9 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -187,7 +187,6 @@  struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 
 	cdev_init(&chip->cdev, &tpm_fops);
 	chip->cdev.owner = THIS_MODULE;
-	chip->cdev.kobj.parent = &chip->dev.kobj;
 
 	return chip;
 
@@ -230,27 +229,16 @@  static int tpm_add_char_device(struct tpm_chip *chip)
 {
 	int rc;
 
-	rc = cdev_add(&chip->cdev, chip->dev.devt, 1);
+	rc = cdev_device_add(&chip->cdev, &chip->dev);
 	if (rc) {
 		dev_err(&chip->dev,
-			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			"unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
 			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
 		return rc;
 	}
 
-	rc = device_add(&chip->dev);
-	if (rc) {
-		dev_err(&chip->dev,
-			"unable to device_register() %s, major %d, minor %d, err=%d\n",
-			dev_name(&chip->dev), MAJOR(chip->dev.devt),
-			MINOR(chip->dev.devt), rc);
-
-		cdev_del(&chip->cdev);
-		return rc;
-	}
-
 	/* Make the chip available. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, chip, chip->dev_num);
@@ -261,8 +249,7 @@  static int tpm_add_char_device(struct tpm_chip *chip)
 
 static void tpm_del_char_device(struct tpm_chip *chip)
 {
-	cdev_del(&chip->cdev);
-	device_del(&chip->dev);
+	cdev_device_del(&chip->cdev, &chip->dev);
 
 	/* Make the chip unavailable. */
 	mutex_lock(&idr_lock);