[v2,14/29] media: mc: Refactor media devnode minor clearing

Message ID 20231220103713.113386-15-sakari.ailus@linux.intel.com (mailing list archive)
State Changes Requested
Delegated to: Sakari Ailus
Headers
Series Media device lifetime management |

Commit Message

Sakari Ailus Dec. 20, 2023, 10:36 a.m. UTC
  Refactor clearing media devnode minor bit in media devnode bitmap. Note
that number is used instead of struct media_devnode as argument since the
minor number will also be stored in a different structure soon.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/mc/mc-devnode.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)
  

Comments

Hans Verkuil Feb. 5, 2024, 2:46 p.m. UTC | #1
On 20/12/2023 11:36, Sakari Ailus wrote:
> Refactor clearing media devnode minor bit in media devnode bitmap. Note
> that number is used instead of struct media_devnode as argument since the
> minor number will also be stored in a different structure soon.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

> ---
>  drivers/media/mc/mc-devnode.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
> index 7b17419050fb..717408791a7c 100644
> --- a/drivers/media/mc/mc-devnode.c
> +++ b/drivers/media/mc/mc-devnode.c
> @@ -44,17 +44,22 @@ static dev_t media_dev_t;
>  static DEFINE_MUTEX(media_devnode_lock);
>  static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
>  
> -/* Called when the last user of the media device exits. */
> -static void media_devnode_release(struct device *cd)
> +static void media_devnode_free_minor(unsigned int minor)
>  {
> -	struct media_devnode *devnode = to_media_devnode(cd);
> -
>  	mutex_lock(&media_devnode_lock);
>  
>  	/* Mark device node number as free */
> -	clear_bit(devnode->minor, media_devnode_nums);
> +	clear_bit(minor, media_devnode_nums);
>  
>  	mutex_unlock(&media_devnode_lock);
> +}
> +
> +/* Called when the last user of the media device exits. */
> +static void media_devnode_release(struct device *cd)
> +{
> +	struct media_devnode *devnode = to_media_devnode(cd);
> +
> +	media_devnode_free_minor(devnode->minor);
>  
>  	/* Release media_devnode and perform other cleanups as needed. */
>  	if (devnode->release)
> @@ -254,9 +259,7 @@ int __must_check media_devnode_register(struct media_devnode *devnode,
>  	return 0;
>  
>  cdev_add_error:
> -	mutex_lock(&media_devnode_lock);
> -	clear_bit(devnode->minor, media_devnode_nums);
> -	mutex_unlock(&media_devnode_lock);
> +	media_devnode_free_minor(devnode->minor);
>  
>  	return ret;
>  }
  
Laurent Pinchart Feb. 7, 2024, 10:53 a.m. UTC | #2
Hi Sakari,

Thank you for the patch.

On Wed, Dec 20, 2023 at 12:36:58PM +0200, Sakari Ailus wrote:
> Refactor clearing media devnode minor bit in media devnode bitmap. Note
> that number is used instead of struct media_devnode as argument since the
> minor number will also be stored in a different structure soon.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  drivers/media/mc/mc-devnode.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
> index 7b17419050fb..717408791a7c 100644
> --- a/drivers/media/mc/mc-devnode.c
> +++ b/drivers/media/mc/mc-devnode.c
> @@ -44,17 +44,22 @@ static dev_t media_dev_t;
>  static DEFINE_MUTEX(media_devnode_lock);
>  static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
>  
> -/* Called when the last user of the media device exits. */
> -static void media_devnode_release(struct device *cd)
> +static void media_devnode_free_minor(unsigned int minor)
>  {
> -	struct media_devnode *devnode = to_media_devnode(cd);
> -
>  	mutex_lock(&media_devnode_lock);
>  
>  	/* Mark device node number as free */
> -	clear_bit(devnode->minor, media_devnode_nums);
> +	clear_bit(minor, media_devnode_nums);
>  
>  	mutex_unlock(&media_devnode_lock);
> +}
> +
> +/* Called when the last user of the media device exits. */
> +static void media_devnode_release(struct device *cd)
> +{
> +	struct media_devnode *devnode = to_media_devnode(cd);
> +
> +	media_devnode_free_minor(devnode->minor);
>  
>  	/* Release media_devnode and perform other cleanups as needed. */
>  	if (devnode->release)
> @@ -254,9 +259,7 @@ int __must_check media_devnode_register(struct media_devnode *devnode,
>  	return 0;
>  
>  cdev_add_error:
> -	mutex_lock(&media_devnode_lock);
> -	clear_bit(devnode->minor, media_devnode_nums);
> -	mutex_unlock(&media_devnode_lock);
> +	media_devnode_free_minor(devnode->minor);
>  
>  	return ret;
>  }
  

Patch

diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
index 7b17419050fb..717408791a7c 100644
--- a/drivers/media/mc/mc-devnode.c
+++ b/drivers/media/mc/mc-devnode.c
@@ -44,17 +44,22 @@  static dev_t media_dev_t;
 static DEFINE_MUTEX(media_devnode_lock);
 static DECLARE_BITMAP(media_devnode_nums, MEDIA_NUM_DEVICES);
 
-/* Called when the last user of the media device exits. */
-static void media_devnode_release(struct device *cd)
+static void media_devnode_free_minor(unsigned int minor)
 {
-	struct media_devnode *devnode = to_media_devnode(cd);
-
 	mutex_lock(&media_devnode_lock);
 
 	/* Mark device node number as free */
-	clear_bit(devnode->minor, media_devnode_nums);
+	clear_bit(minor, media_devnode_nums);
 
 	mutex_unlock(&media_devnode_lock);
+}
+
+/* Called when the last user of the media device exits. */
+static void media_devnode_release(struct device *cd)
+{
+	struct media_devnode *devnode = to_media_devnode(cd);
+
+	media_devnode_free_minor(devnode->minor);
 
 	/* Release media_devnode and perform other cleanups as needed. */
 	if (devnode->release)
@@ -254,9 +259,7 @@  int __must_check media_devnode_register(struct media_devnode *devnode,
 	return 0;
 
 cdev_add_error:
-	mutex_lock(&media_devnode_lock);
-	clear_bit(devnode->minor, media_devnode_nums);
-	mutex_unlock(&media_devnode_lock);
+	media_devnode_free_minor(devnode->minor);
 
 	return ret;
 }