All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Wiklander <jens.wiklander@linaro.org>
To: op-tee@lists.trustedfirmware.org
Subject: Re: [PATCH 1/2] tee: replace cdev_add + device_add with cdev_device_add
Date: Fri, 18 Sep 2020 10:41:00 +0200	[thread overview]
Message-ID: <20200918084100.GA1219771@jade> (raw)
In-Reply-To: <20200901103335.685-1-sudeep.holla@arm.com>

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

On Tue, Sep 01, 2020 at 11:33:34AM +0100, Sudeep Holla wrote:
> Commit 233ed09d7fda ("chardev: add helper function to register char devs
> with a struct device") added a helper function 'cdev_device_add'.
> 
> Make use of cdev_device_add in tee_device_register to replace cdev_add
> and device_add. Since cdev_device_add takes care of setting the
> kobj->parent, drop explicit initialisation in tee_device_alloc.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/tee/tee_core.c | 21 ++++-----------------
>  1 file changed, 4 insertions(+), 17 deletions(-)

Thanks for the nice cleanups. I'm picking up this patch and
"[PATCH 2/2] tee: avoid explicit sysfs_create/delete_group by
initialising dev->groups".

Cheers,
Jens

> 
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 64637e09a095..b4a8b362d78f 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -917,7 +917,6 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
>  
>  	cdev_init(&teedev->cdev, &tee_fops);
>  	teedev->cdev.owner = teedesc->owner;
> -	teedev->cdev.kobj.parent = &teedev->dev.kobj;
>  
>  	dev_set_drvdata(&teedev->dev, driver_data);
>  	device_initialize(&teedev->dev);
> @@ -985,24 +984,15 @@ int tee_device_register(struct tee_device *teedev)
>  		return -EINVAL;
>  	}
>  
> -	rc = cdev_add(&teedev->cdev, teedev->dev.devt, 1);
> +	rc = cdev_device_add(&teedev->cdev, &teedev->dev);
>  	if (rc) {
>  		dev_err(&teedev->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",
>  			teedev->name, MAJOR(teedev->dev.devt),
>  			MINOR(teedev->dev.devt), rc);
>  		return rc;
>  	}
>  
> -	rc = device_add(&teedev->dev);
> -	if (rc) {
> -		dev_err(&teedev->dev,
> -			"unable to device_add() %s, major %d, minor %d, err=%d\n",
> -			teedev->name, MAJOR(teedev->dev.devt),
> -			MINOR(teedev->dev.devt), rc);
> -		goto err_device_add;
> -	}
> -
>  	rc = sysfs_create_group(&teedev->dev.kobj, &tee_dev_group);
>  	if (rc) {
>  		dev_err(&teedev->dev,
> @@ -1014,9 +1004,7 @@ int tee_device_register(struct tee_device *teedev)
>  	return 0;
>  
>  err_sysfs_create_group:
> -	device_del(&teedev->dev);
> -err_device_add:
> -	cdev_del(&teedev->cdev);
> +	cdev_device_del(&teedev->cdev, &teedev->dev);
>  	return rc;
>  }
>  EXPORT_SYMBOL_GPL(tee_device_register);
> @@ -1062,8 +1050,7 @@ void tee_device_unregister(struct tee_device *teedev)
>  
>  	if (teedev->flags & TEE_DEVICE_FLAG_REGISTERED) {
>  		sysfs_remove_group(&teedev->dev.kobj, &tee_dev_group);
> -		cdev_del(&teedev->cdev);
> -		device_del(&teedev->dev);
> +		cdev_device_del(&teedev->cdev, &teedev->dev);
>  	}
>  
>  	tee_device_put(teedev);
> -- 
> 2.17.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: Jens Wiklander <jens.wiklander@linaro.org>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org
Subject: Re: [PATCH 1/2] tee: replace cdev_add + device_add with cdev_device_add
Date: Fri, 18 Sep 2020 10:41:00 +0200	[thread overview]
Message-ID: <20200918084100.GA1219771@jade> (raw)
In-Reply-To: <20200901103335.685-1-sudeep.holla@arm.com>

On Tue, Sep 01, 2020 at 11:33:34AM +0100, Sudeep Holla wrote:
> Commit 233ed09d7fda ("chardev: add helper function to register char devs
> with a struct device") added a helper function 'cdev_device_add'.
> 
> Make use of cdev_device_add in tee_device_register to replace cdev_add
> and device_add. Since cdev_device_add takes care of setting the
> kobj->parent, drop explicit initialisation in tee_device_alloc.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/tee/tee_core.c | 21 ++++-----------------
>  1 file changed, 4 insertions(+), 17 deletions(-)

Thanks for the nice cleanups. I'm picking up this patch and
"[PATCH 2/2] tee: avoid explicit sysfs_create/delete_group by
initialising dev->groups".

Cheers,
Jens

> 
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 64637e09a095..b4a8b362d78f 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -917,7 +917,6 @@ struct tee_device *tee_device_alloc(const struct tee_desc *teedesc,
>  
>  	cdev_init(&teedev->cdev, &tee_fops);
>  	teedev->cdev.owner = teedesc->owner;
> -	teedev->cdev.kobj.parent = &teedev->dev.kobj;
>  
>  	dev_set_drvdata(&teedev->dev, driver_data);
>  	device_initialize(&teedev->dev);
> @@ -985,24 +984,15 @@ int tee_device_register(struct tee_device *teedev)
>  		return -EINVAL;
>  	}
>  
> -	rc = cdev_add(&teedev->cdev, teedev->dev.devt, 1);
> +	rc = cdev_device_add(&teedev->cdev, &teedev->dev);
>  	if (rc) {
>  		dev_err(&teedev->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",
>  			teedev->name, MAJOR(teedev->dev.devt),
>  			MINOR(teedev->dev.devt), rc);
>  		return rc;
>  	}
>  
> -	rc = device_add(&teedev->dev);
> -	if (rc) {
> -		dev_err(&teedev->dev,
> -			"unable to device_add() %s, major %d, minor %d, err=%d\n",
> -			teedev->name, MAJOR(teedev->dev.devt),
> -			MINOR(teedev->dev.devt), rc);
> -		goto err_device_add;
> -	}
> -
>  	rc = sysfs_create_group(&teedev->dev.kobj, &tee_dev_group);
>  	if (rc) {
>  		dev_err(&teedev->dev,
> @@ -1014,9 +1004,7 @@ int tee_device_register(struct tee_device *teedev)
>  	return 0;
>  
>  err_sysfs_create_group:
> -	device_del(&teedev->dev);
> -err_device_add:
> -	cdev_del(&teedev->cdev);
> +	cdev_device_del(&teedev->cdev, &teedev->dev);
>  	return rc;
>  }
>  EXPORT_SYMBOL_GPL(tee_device_register);
> @@ -1062,8 +1050,7 @@ void tee_device_unregister(struct tee_device *teedev)
>  
>  	if (teedev->flags & TEE_DEVICE_FLAG_REGISTERED) {
>  		sysfs_remove_group(&teedev->dev.kobj, &tee_dev_group);
> -		cdev_del(&teedev->cdev);
> -		device_del(&teedev->dev);
> +		cdev_device_del(&teedev->cdev, &teedev->dev);
>  	}
>  
>  	tee_device_put(teedev);
> -- 
> 2.17.1
> 

  parent reply	other threads:[~2020-09-18  8:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 10:33 [PATCH 1/2] tee: replace cdev_add + device_add with cdev_device_add Sudeep Holla
2020-09-01 10:33 ` Sudeep Holla
2020-09-01 10:33 ` [PATCH 2/2] tee: avoid explicit sysfs_create/delete_group by initialising dev->groups Sudeep Holla
2020-09-01 10:33   ` Sudeep Holla
2020-09-18  8:41 ` Jens Wiklander [this message]
2020-09-18  8:41   ` [PATCH 1/2] tee: replace cdev_add + device_add with cdev_device_add Jens Wiklander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200918084100.GA1219771@jade \
    --to=jens.wiklander@linaro.org \
    --cc=op-tee@lists.trustedfirmware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.