diff mbox

[tpmdd-devel] tpm: Fix initialization of the cdev

Message ID 20150630191531.GA29764@obsidianresearch.com
State Accepted
Headers show

Commit Message

Jason Gunthorpe June 30, 2015, 7:15 p.m. UTC
When a cdev is contained in a dynamic structure the cdev parent kobj
should be set to the kobj that controls the lifetime of the enclosing
structure. In TPM's case this is the embedded struct device.

Also, cdev_init 0's the whole structure, so all sets must be after,
not before. This fixes module ref counting and cdev.

Fixes: 313d21eeab92 ("tpm: device class for tpm")
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/char/tpm/tpm-chip.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Peter, I've only compile tests this. Thanks

Comments

Dmitry Torokhov July 1, 2015, 10:06 p.m. UTC | #1
On Tue, Jun 30, 2015 at 01:15:31PM -0600, Jason Gunthorpe wrote:
> When a cdev is contained in a dynamic structure the cdev parent kobj
> should be set to the kobj that controls the lifetime of the enclosing
> structure. In TPM's case this is the embedded struct device.
> 
> Also, cdev_init 0's the whole structure, so all sets must be after,
> not before. This fixes module ref counting and cdev.
> 
> Fixes: 313d21eeab92 ("tpm: device class for tpm")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Right, when embedding cdev its lifetime should be tied to the lifetime
of the enclosing structure.

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/char/tpm/tpm-chip.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Peter, I've only compile tests this. Thanks
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 283f00a7f036..1082d4bb016a 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -129,8 +129,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>  
>  	device_initialize(&chip->dev);
>  
> -	chip->cdev.owner = chip->pdev->driver->owner;
>  	cdev_init(&chip->cdev, &tpm_fops);
> +	chip->cdev.owner = chip->pdev->driver->owner;
> +	chip->cdev.kobj.parent = &chip->dev.kobj;
>  
>  	return chip;
>  }
> -- 
> 2.1.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Jarkko Sakkinen July 3, 2015, 10:57 a.m. UTC | #2
On 06/30/2015 10:15 PM, Jason Gunthorpe wrote:
> When a cdev is contained in a dynamic structure the cdev parent kobj
> should be set to the kobj that controls the lifetime of the enclosing
> structure. In TPM's case this is the embedded struct device.
>
> Also, cdev_init 0's the whole structure, so all sets must be after,
> not before. This fixes module ref counting and cdev.
>
> Fixes: 313d21eeab92 ("tpm: device class for tpm")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com

Right, thank you for fixing this! Considering how big
changes we engineered things still have been more smooth
than I expected.

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

> ---
>   drivers/char/tpm/tpm-chip.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> Peter, I've only compile tests this. Thanks
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 283f00a7f036..1082d4bb016a 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -129,8 +129,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>   
>   	device_initialize(&chip->dev);
>   
> -	chip->cdev.owner = chip->pdev->driver->owner;
>   	cdev_init(&chip->cdev, &tpm_fops);
> +	chip->cdev.owner = chip->pdev->driver->owner;
> +	chip->cdev.kobj.parent = &chip->dev.kobj;
>   
>   	return chip;
>   }

PS. There is also another fix that hasn't drawn much attention:

https://lkml.org/lkml/2015/6/24/386

/Jarkko

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
Peter Hüwe July 13, 2015, 9:43 p.m. UTC | #3
Am Dienstag, 30. Juni 2015, 21:15:31 schrieb Jason Gunthorpe:
> When a cdev is contained in a dynamic structure the cdev parent kobj
> should be set to the kobj that controls the lifetime of the enclosing
> structure. In TPM's case this is the embedded struct device.
> 
> Also, cdev_init 0's the whole structure, so all sets must be after,
> not before. This fixes module ref counting and cdev.
> 
> Fixes: 313d21eeab92 ("tpm: device class for tpm")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/char/tpm/tpm-chip.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Peter, I've only compile tests this. Thanks
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 283f00a7f036..1082d4bb016a 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -129,8 +129,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> 
>  	device_initialize(&chip->dev);
> 
> -	chip->cdev.owner = chip->pdev->driver->owner;
>  	cdev_init(&chip->cdev, &tpm_fops);
> +	chip->cdev.owner = chip->pdev->driver->owner;
> +	chip->cdev.kobj.parent = &chip->dev.kobj;
> 
>  	return chip;
>  }

Applied and pushed.
Thanks

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 283f00a7f036..1082d4bb016a 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -129,8 +129,9 @@  struct tpm_chip *tpmm_chip_alloc(struct device *dev,
 
 	device_initialize(&chip->dev);
 
-	chip->cdev.owner = chip->pdev->driver->owner;
 	cdev_init(&chip->cdev, &tpm_fops);
+	chip->cdev.owner = chip->pdev->driver->owner;
+	chip->cdev.kobj.parent = &chip->dev.kobj;
 
 	return chip;
 }