[PATCH v2 0/5] media: vimc: bug fixes related to memory management

[PATCH v2 0/5] media: vimc: bug fixes related to memory management

[Dafna Hirschfeld]

Changes since v1:
fixes due to Helen Koike and Andrzej Pietrasiewicz's comments (thanks for the review!):

 - split the first patch into two patches, one that fixes the bug and one cleanup patch
 - fix allignment issue
 - add a last patch that removes vimc_ent_sd_unregister since it is not needed anymore

Dafna Hirschfeld (5):
  media: vimc: initialize vim entity pointers to NULL
  media: vimc: cleanup code that assigns entity in entities array
  media: vimc: sensor: register subdevice only after initialization
  media: vimc: move media_entity_cleanup to release callbacks
  media: vimc: remove the helper function vimc_ent_sd_unregister

 drivers/media/platform/vimc/vimc-capture.c |  2 +-
 drivers/media/platform/vimc/vimc-common.c  |  6 ------
 drivers/media/platform/vimc/vimc-common.h  | 12 -----------
 drivers/media/platform/vimc/vimc-core.c    | 11 ++++-------
 drivers/media/platform/vimc/vimc-debayer.c |  3 ++-
 drivers/media/platform/vimc/vimc-scaler.c  |  3 ++-
 drivers/media/platform/vimc/vimc-sensor.c  | 23 +++++++++++-----------
 7 files changed, 21 insertions(+), 39 deletions(-)

-- 
2.20.1

[PATCH v2 1/5] media: vimc: initialize vim entity pointers to NULL

[Dafna Hirschfeld]

since NULL value for vimc entity pointer indicates
that entity creation failed and this is tested, the
pointers should be initialized to NULL.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/vimc/vimc-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
index 6e3e5c91ae39..b8add4087652 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -199,9 +199,8 @@ static int vimc_register_devices(struct vimc_device *vimc)
 	}
 
 	/* allocate ent_devs */
-	vimc->ent_devs = kmalloc_array(vimc->pipe_cfg->num_ents,
-				       sizeof(*vimc->ent_devs),
-				       GFP_KERNEL);
+	vimc->ent_devs = kcalloc(vimc->pipe_cfg->num_ents,
+				 sizeof(*vimc->ent_devs), GFP_KERNEL);
 	if (!vimc->ent_devs)
 		goto err_v4l2_unregister;
 
-- 
2.20.1

[PATCH v2 2/5] media: vimc: cleanup code that assigns entity in entities array

[Dafna Hirschfeld]

Since the add callback returns NULL on failure and the array
is initialized to NULLs, there is no need for the intermediate
assignment to local var.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/vimc/vimc-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
index b8add4087652..325f55d0d8b3 100644
--- a/drivers/media/platform/vimc/vimc-core.c
+++ b/drivers/media/platform/vimc/vimc-core.c
@@ -160,19 +160,17 @@ static int vimc_create_links(struct vimc_device *vimc)
 static int vimc_add_subdevs(struct vimc_device *vimc)
 {
 	unsigned int i;
-	struct vimc_ent_device *ved;
 
 	for (i = 0; i < vimc->pipe_cfg->num_ents; i++) {
 		dev_dbg(&vimc->pdev.dev, "new entity for %s\n",
 			vimc->pipe_cfg->ents[i].name);
-		ved = vimc->pipe_cfg->ents[i].add(vimc,
+		vimc->ent_devs[i] = vimc->pipe_cfg->ents[i].add(vimc,
 					vimc->pipe_cfg->ents[i].name);
-		if (!ved) {
+		if (!vimc->ent_devs[i]) {
 			dev_err(&vimc->pdev.dev, "add new entity for %s\n",
 				vimc->pipe_cfg->ents[i].name);
 			return -EINVAL;
 		}
-		vimc->ent_devs[i] = ved;
 	}
 	return 0;
 }
-- 
2.20.1

[PATCH v2 3/5] media: vimc: sensor: register subdevice only after initialization

[Dafna Hirschfeld]

vimc_sen_add function first registers the subdevice and then
calls tpg_alloc. If tpg_alloc fails it unregisters the subdevice
and then frees vsen, this cause double free since the release
callback that follows subdevice unregistration also frees vsen.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/media/platform/vimc/vimc-sensor.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 46dc6a535abe..ee2306c08569 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -358,6 +358,13 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
 		goto err_free_vsen;
 	}
 
+	/* Initialize the test pattern generator */
+	tpg_init(&vsen->tpg, vsen->mbus_format.width,
+		 vsen->mbus_format.height);
+	ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
+	if (ret)
+		goto err_free_hdl;
+
 	/* Initialize ved and sd */
 	ret = vimc_ent_sd_register(&vsen->ved, &vsen->sd, v4l2_dev,
 				   vcfg_name,
@@ -365,7 +372,7 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
 				   (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
 				   &vimc_sen_int_ops, &vimc_sen_ops);
 	if (ret)
-		goto err_free_hdl;
+		goto err_free_tpg;
 
 	vsen->ved.process_frame = vimc_sen_process_frame;
 	vsen->dev = &vimc->pdev.dev;
@@ -373,17 +380,10 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
 	/* Initialize the frame format */
 	vsen->mbus_format = fmt_default;
 
-	/* Initialize the test pattern generator */
-	tpg_init(&vsen->tpg, vsen->mbus_format.width,
-		 vsen->mbus_format.height);
-	ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
-	if (ret)
-		goto err_unregister_ent_sd;
-
 	return &vsen->ved;
 
-err_unregister_ent_sd:
-	vimc_ent_sd_unregister(&vsen->ved,  &vsen->sd);
+err_free_tpg:
+	tpg_free(&vsen->tpg);
 err_free_hdl:
 	v4l2_ctrl_handler_free(&vsen->hdl);
 err_free_vsen:
-- 
2.20.1

[PATCH v2 4/5] media: vimc: move media_entity_cleanup to release callbacks

[Dafna Hirschfeld]

according to the docs, this function must be called during
the cleanup phase after unregistering the entity.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
Acked-by: Helen Koike <helen.koike@collabora.com>
---
 drivers/media/platform/vimc/vimc-capture.c | 2 +-
 drivers/media/platform/vimc/vimc-common.c  | 1 -
 drivers/media/platform/vimc/vimc-debayer.c | 1 +
 drivers/media/platform/vimc/vimc-scaler.c  | 1 +
 drivers/media/platform/vimc/vimc-sensor.c  | 1 +
 5 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-capture.c b/drivers/media/platform/vimc/vimc-capture.c
index 602f80323031..5f353c20e605 100644
--- a/drivers/media/platform/vimc/vimc-capture.c
+++ b/drivers/media/platform/vimc/vimc-capture.c
@@ -330,6 +330,7 @@ static void vimc_cap_release(struct video_device *vdev)
 	struct vimc_cap_device *vcap =
 		container_of(vdev, struct vimc_cap_device, vdev);
 
+	media_entity_cleanup(vcap->ved.ent);
 	vimc_pads_cleanup(vcap->ved.pads);
 	kfree(vcap);
 }
@@ -340,7 +341,6 @@ void vimc_cap_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
 
 	vcap = container_of(ved, struct vimc_cap_device, ved);
 	vb2_queue_release(&vcap->queue);
-	media_entity_cleanup(ved->ent);
 	video_unregister_device(&vcap->vdev);
 }
 
diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index a3120f4f7a90..999bc353fb10 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -423,7 +423,6 @@ EXPORT_SYMBOL_GPL(vimc_ent_sd_register);
 
 void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd)
 {
-	media_entity_cleanup(ved->ent);
 	v4l2_device_unregister_subdev(sd);
 }
 EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister);
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index feac47d79449..e1bad6713cde 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -477,6 +477,7 @@ static void vimc_deb_release(struct v4l2_subdev *sd)
 	struct vimc_deb_device *vdeb =
 				container_of(sd, struct vimc_deb_device, sd);
 
+	media_entity_cleanup(vdeb->ved.ent);
 	vimc_pads_cleanup(vdeb->ved.pads);
 	kfree(vdeb);
 }
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index a6a3cc5be872..1982bc089af5 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -336,6 +336,7 @@ static void vimc_sca_release(struct v4l2_subdev *sd)
 	struct vimc_sca_device *vsca =
 				container_of(sd, struct vimc_sca_device, sd);
 
+	media_entity_cleanup(vsca->ved.ent);
 	vimc_pads_cleanup(vsca->ved.pads);
 	kfree(vsca);
 }
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index ee2306c08569..63fe024ccea5 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -291,6 +291,7 @@ static void vimc_sen_release(struct v4l2_subdev *sd)
 
 	v4l2_ctrl_handler_free(&vsen->hdl);
 	tpg_free(&vsen->tpg);
+	media_entity_cleanup(vsen->ved.ent);
 	vimc_pads_cleanup(vsen->ved.pads);
 	kfree(vsen);
 }
-- 
2.20.1

[PATCH v2 5/5] media: vimc: remove the helper function vimc_ent_sd_unregister

[Dafna Hirschfeld]

since this function only calls v4l2_device_unregister_subdev,
it is pointless.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/vimc/vimc-common.c  |  5 -----
 drivers/media/platform/vimc/vimc-common.h  | 12 ------------
 drivers/media/platform/vimc/vimc-debayer.c |  2 +-
 drivers/media/platform/vimc/vimc-scaler.c  |  2 +-
 drivers/media/platform/vimc/vimc-sensor.c  |  2 +-
 5 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index 999bc353fb10..67b53dc1849d 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -421,8 +421,3 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
 }
 EXPORT_SYMBOL_GPL(vimc_ent_sd_register);
 
-void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd)
-{
-	v4l2_device_unregister_subdev(sd);
-}
-EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister);
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 698db7c07645..af5b1166dc1f 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -251,18 +251,6 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
 			 const struct v4l2_subdev_internal_ops *sd_int_ops,
 			 const struct v4l2_subdev_ops *sd_ops);
 
-/**
- * vimc_ent_sd_unregister - cleanup and unregister a subdev node
- *
- * @ved:	the vimc_ent_device struct to be cleaned up
- * @sd:		the v4l2_subdev struct to be unregistered
- *
- * Helper function cleanup and unregister the struct vimc_ent_device and struct
- * v4l2_subdev which represents a subdev node in the topology
- */
-void vimc_ent_sd_unregister(struct vimc_ent_device *ved,
-			    struct v4l2_subdev *sd);
-
 /**
  * vimc_link_validate - validates a media link
  *
diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index e1bad6713cde..4e5316c671e0 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -491,7 +491,7 @@ void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
 	struct vimc_deb_device *vdeb;
 
 	vdeb = container_of(ved, struct vimc_deb_device, ved);
-	vimc_ent_sd_unregister(ved, &vdeb->sd);
+	v4l2_device_unregister_subdev(&vdeb->sd);
 }
 
 struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index 1982bc089af5..4fe2ba578652 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -350,7 +350,7 @@ void vimc_sca_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
 	struct vimc_sca_device *vsca;
 
 	vsca = container_of(ved, struct vimc_sca_device, ved);
-	vimc_ent_sd_unregister(ved, &vsca->sd);
+	v4l2_device_unregister_subdev(&vsca->sd);
 }
 
 struct vimc_ent_device *vimc_sca_add(struct vimc_device *vimc,
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 63fe024ccea5..14838362d871 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -305,7 +305,7 @@ void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
 	struct vimc_sen_device *vsen;
 
 	vsen = container_of(ved, struct vimc_sen_device, ved);
-	vimc_ent_sd_unregister(ved, &vsen->sd);
+	v4l2_device_unregister_subdev(&vsen->sd);
 }
 
 /* Image Processing Controls */
-- 
2.20.1

Re: [PATCH v2 1/5] media: vimc: initialize vim entity pointers to NULL

[Helen Koike]

On 10/3/19 7:44 AM, Dafna Hirschfeld wrote:
> since NULL value for vimc entity pointer indicates
> that entity creation failed and this is tested, the
> pointers should be initialized to NULL.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Acked-by: Helen Koike <helen.koike@collabora.com>

> ---
>  drivers/media/platform/vimc/vimc-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
> index 6e3e5c91ae39..b8add4087652 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -199,9 +199,8 @@ static int vimc_register_devices(struct vimc_device *vimc)
>  	}
>  
>  	/* allocate ent_devs */
> -	vimc->ent_devs = kmalloc_array(vimc->pipe_cfg->num_ents,
> -				       sizeof(*vimc->ent_devs),
> -				       GFP_KERNEL);
> +	vimc->ent_devs = kcalloc(vimc->pipe_cfg->num_ents,
> +				 sizeof(*vimc->ent_devs), GFP_KERNEL);
>  	if (!vimc->ent_devs)
>  		goto err_v4l2_unregister;
>  
> 

Re: [PATCH v2 2/5] media: vimc: cleanup code that assigns entity in entities array

[Helen Koike]

On 10/3/19 7:44 AM, Dafna Hirschfeld wrote:
> Since the add callback returns NULL on failure and the array
> is initialized to NULLs, there is no need for the intermediate
> assignment to local var.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

I would just rename the title to something like:
"media: vimc: remove unnecessary var when adding entities"

It makes clear what you are doing and where, what do you think?

With this:

Acked-by: Helen Koike <helen.koike@collabora.com>

> ---
>  drivers/media/platform/vimc/vimc-core.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-core.c b/drivers/media/platform/vimc/vimc-core.c
> index b8add4087652..325f55d0d8b3 100644
> --- a/drivers/media/platform/vimc/vimc-core.c
> +++ b/drivers/media/platform/vimc/vimc-core.c
> @@ -160,19 +160,17 @@ static int vimc_create_links(struct vimc_device *vimc)
>  static int vimc_add_subdevs(struct vimc_device *vimc)
>  {
>  	unsigned int i;
> -	struct vimc_ent_device *ved;
>  
>  	for (i = 0; i < vimc->pipe_cfg->num_ents; i++) {
>  		dev_dbg(&vimc->pdev.dev, "new entity for %s\n",
>  			vimc->pipe_cfg->ents[i].name);
> -		ved = vimc->pipe_cfg->ents[i].add(vimc,
> +		vimc->ent_devs[i] = vimc->pipe_cfg->ents[i].add(vimc,
>  					vimc->pipe_cfg->ents[i].name);
> -		if (!ved) {
> +		if (!vimc->ent_devs[i]) {
>  			dev_err(&vimc->pdev.dev, "add new entity for %s\n",
>  				vimc->pipe_cfg->ents[i].name);
>  			return -EINVAL;
>  		}
> -		vimc->ent_devs[i] = ved;
>  	}
>  	return 0;
>  }
> 

Re: [PATCH v2 3/5] media: vimc: sensor: register subdevice only after initialization

[Helen Koike]

Hi Dafna,

Could you just rename the title tags to "media: vimc: sen: " to follow the
model of the other commits?

Thanks
Helen

On 10/3/19 7:44 AM, Dafna Hirschfeld wrote:
> vimc_sen_add function first registers the subdevice and then
> calls tpg_alloc. If tpg_alloc fails it unregisters the subdevice
> and then frees vsen, this cause double free since the release
> callback that follows subdevice unregistration also frees vsen.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> Acked-by: Helen Koike <helen.koike@collabora.com>
> ---
>  drivers/media/platform/vimc/vimc-sensor.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 46dc6a535abe..ee2306c08569 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -358,6 +358,13 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
>  		goto err_free_vsen;
>  	}
>  
> +	/* Initialize the test pattern generator */
> +	tpg_init(&vsen->tpg, vsen->mbus_format.width,
> +		 vsen->mbus_format.height);
> +	ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
> +	if (ret)
> +		goto err_free_hdl;
> +
>  	/* Initialize ved and sd */
>  	ret = vimc_ent_sd_register(&vsen->ved, &vsen->sd, v4l2_dev,
>  				   vcfg_name,
> @@ -365,7 +372,7 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
>  				   (const unsigned long[1]) {MEDIA_PAD_FL_SOURCE},
>  				   &vimc_sen_int_ops, &vimc_sen_ops);
>  	if (ret)
> -		goto err_free_hdl;
> +		goto err_free_tpg;
>  
>  	vsen->ved.process_frame = vimc_sen_process_frame;
>  	vsen->dev = &vimc->pdev.dev;
> @@ -373,17 +380,10 @@ struct vimc_ent_device *vimc_sen_add(struct vimc_device *vimc,
>  	/* Initialize the frame format */
>  	vsen->mbus_format = fmt_default;
>  
> -	/* Initialize the test pattern generator */
> -	tpg_init(&vsen->tpg, vsen->mbus_format.width,
> -		 vsen->mbus_format.height);
> -	ret = tpg_alloc(&vsen->tpg, VIMC_FRAME_MAX_WIDTH);
> -	if (ret)
> -		goto err_unregister_ent_sd;
> -
>  	return &vsen->ved;
>  
> -err_unregister_ent_sd:
> -	vimc_ent_sd_unregister(&vsen->ved,  &vsen->sd);
> +err_free_tpg:
> +	tpg_free(&vsen->tpg);
>  err_free_hdl:
>  	v4l2_ctrl_handler_free(&vsen->hdl);
>  err_free_vsen:
> 

Re: [PATCH v2 5/5] media: vimc: remove the helper function vimc_ent_sd_unregister

[Helen Koike]

On 10/3/19 7:44 AM, Dafna Hirschfeld wrote:
> since this function only calls v4l2_device_unregister_subdev,
> it is pointless.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Acked-by: Helen Koike <helen.koike@collabora.com>

> ---
>  drivers/media/platform/vimc/vimc-common.c  |  5 -----
>  drivers/media/platform/vimc/vimc-common.h  | 12 ------------
>  drivers/media/platform/vimc/vimc-debayer.c |  2 +-
>  drivers/media/platform/vimc/vimc-scaler.c  |  2 +-
>  drivers/media/platform/vimc/vimc-sensor.c  |  2 +-
>  5 files changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> index 999bc353fb10..67b53dc1849d 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -421,8 +421,3 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
>  }
>  EXPORT_SYMBOL_GPL(vimc_ent_sd_register);
>  
> -void vimc_ent_sd_unregister(struct vimc_ent_device *ved, struct v4l2_subdev *sd)
> -{
> -	v4l2_device_unregister_subdev(sd);
> -}
> -EXPORT_SYMBOL_GPL(vimc_ent_sd_unregister);
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 698db7c07645..af5b1166dc1f 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -251,18 +251,6 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
>  			 const struct v4l2_subdev_internal_ops *sd_int_ops,
>  			 const struct v4l2_subdev_ops *sd_ops);
>  
> -/**
> - * vimc_ent_sd_unregister - cleanup and unregister a subdev node
> - *
> - * @ved:	the vimc_ent_device struct to be cleaned up
> - * @sd:		the v4l2_subdev struct to be unregistered
> - *
> - * Helper function cleanup and unregister the struct vimc_ent_device and struct
> - * v4l2_subdev which represents a subdev node in the topology
> - */
> -void vimc_ent_sd_unregister(struct vimc_ent_device *ved,
> -			    struct v4l2_subdev *sd);
> -
>  /**
>   * vimc_link_validate - validates a media link
>   *
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index e1bad6713cde..4e5316c671e0 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -491,7 +491,7 @@ void vimc_deb_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
>  	struct vimc_deb_device *vdeb;
>  
>  	vdeb = container_of(ved, struct vimc_deb_device, ved);
> -	vimc_ent_sd_unregister(ved, &vdeb->sd);
> +	v4l2_device_unregister_subdev(&vdeb->sd);
>  }
>  
>  struct vimc_ent_device *vimc_deb_add(struct vimc_device *vimc,
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index 1982bc089af5..4fe2ba578652 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -350,7 +350,7 @@ void vimc_sca_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
>  	struct vimc_sca_device *vsca;
>  
>  	vsca = container_of(ved, struct vimc_sca_device, ved);
> -	vimc_ent_sd_unregister(ved, &vsca->sd);
> +	v4l2_device_unregister_subdev(&vsca->sd);
>  }
>  
>  struct vimc_ent_device *vimc_sca_add(struct vimc_device *vimc,
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 63fe024ccea5..14838362d871 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -305,7 +305,7 @@ void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved)
>  	struct vimc_sen_device *vsen;
>  
>  	vsen = container_of(ved, struct vimc_sen_device, ved);
> -	vimc_ent_sd_unregister(ved, &vsen->sd);
> +	v4l2_device_unregister_subdev(&vsen->sd);
>  }
>  
>  /* Image Processing Controls */
>