Hi Hans,
Thank you for the patch.
On Tue, Mar 05, 2019 at 10:58:47AM +0100, hverkuil-ci...@xs4all.nl wrote:
> From: Hans Verkuil <hverkuil-ci...@xs4all.nl>
>
> Use the new v4l2_subdev_internal_ops release op to free the
> subdev memory only once the last user closed the file handle.
>
> Signed-off-by: Hans Verkuil <hverkuil-ci...@xs4all.nl>
> ---
> drivers/media/platform/vimc/vimc-common.c | 2 ++
> drivers/media/platform/vimc/vimc-common.h | 2 ++
> drivers/media/platform/vimc/vimc-debayer.c | 15 +++++++++++++--
> drivers/media/platform/vimc/vimc-scaler.c | 15 +++++++++++++--
> drivers/media/platform/vimc/vimc-sensor.c | 19 +++++++++++++++----
> 5 files changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-common.c
> b/drivers/media/platform/vimc/vimc-common.c
> index c1a74bb2df58..0adbfd8fd26d 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -380,6 +380,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
> u32 function,
> u16 num_pads,
> const unsigned long *pads_flag,
> + const struct v4l2_subdev_internal_ops *sd_int_ops,
> const struct v4l2_subdev_ops *sd_ops)
> {
> int ret;
> @@ -394,6 +395,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
>
> /* Initialize the subdev */
> v4l2_subdev_init(sd, sd_ops);
> + sd->internal_ops = sd_int_ops;
> sd->entity.function = function;
> sd->entity.ops = &vimc_ent_sd_mops;
> sd->owner = THIS_MODULE;
> diff --git a/drivers/media/platform/vimc/vimc-common.h
> b/drivers/media/platform/vimc/vimc-common.h
> index 84539430b5e7..07987eab988f 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -187,6 +187,7 @@ const struct vimc_pix_map
> *vimc_pix_map_by_pixelformat(u32 pixelformat);
> * @function: media entity function defined by MEDIA_ENT_F_* macros
> * @num_pads: number of pads to initialize
> * @pads_flag: flags to use in each pad
> + * @sd_int_ops: pointer to &struct v4l2_subdev_internal_ops.
Nitpicking, most of the lines here don't have a period at the end.
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> * @sd_ops: pointer to &struct v4l2_subdev_ops.
> *
> * Helper function initialize and register the struct vimc_ent_device and
> struct
> @@ -199,6 +200,7 @@ int vimc_ent_sd_register(struct vimc_ent_device *ved,
> u32 function,
> u16 num_pads,
> const unsigned long *pads_flag,
> + const struct v4l2_subdev_internal_ops *sd_int_ops,
> const struct v4l2_subdev_ops *sd_ops);
>
> /**
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c
> b/drivers/media/platform/vimc/vimc-debayer.c
> index 7d77c63b99d2..eaed4233ad1b 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -489,6 +489,18 @@ static void *vimc_deb_process_frame(struct
> vimc_ent_device *ved,
>
> }
>
> +static void vimc_deb_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_deb_device *vdeb =
> + container_of(sd, struct vimc_deb_device, sd);
> +
> + kfree(vdeb);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_deb_int_ops = {
> + .release = vimc_deb_release,
> +};
> +
> static void vimc_deb_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -497,7 +509,6 @@ static void vimc_deb_comp_unbind(struct device *comp,
> struct device *master,
> ved);
>
> vimc_ent_sd_unregister(ved, &vdeb->sd);
> - kfree(vdeb);
> }
>
> static int vimc_deb_comp_bind(struct device *comp, struct device *master,
> @@ -519,7 +530,7 @@ static int vimc_deb_comp_bind(struct device *comp, struct
> device *master,
> MEDIA_ENT_F_PROC_VIDEO_PIXEL_ENC_CONV, 2,
> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
> MEDIA_PAD_FL_SOURCE},
> - &vimc_deb_ops);
> + &vimc_deb_int_ops, &vimc_deb_ops);
> if (ret) {
> kfree(vdeb);
> return ret;
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c
> b/drivers/media/platform/vimc/vimc-scaler.c
> index 39b2a73dfcc1..2028afa4ef7a 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -348,6 +348,18 @@ static void *vimc_sca_process_frame(struct
> vimc_ent_device *ved,
> return vsca->src_frame;
> };
>
> +static void vimc_sca_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_sca_device *vsca =
> + container_of(sd, struct vimc_sca_device, sd);
> +
> + kfree(vsca);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_sca_int_ops = {
> + .release = vimc_sca_release,
> +};
> +
> static void vimc_sca_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -356,7 +368,6 @@ static void vimc_sca_comp_unbind(struct device *comp,
> struct device *master,
> ved);
>
> vimc_ent_sd_unregister(ved, &vsca->sd);
> - kfree(vsca);
> }
>
>
> @@ -379,7 +390,7 @@ static int vimc_sca_comp_bind(struct device *comp, struct
> device *master,
> MEDIA_ENT_F_PROC_VIDEO_SCALER, 2,
> (const unsigned long[2]) {MEDIA_PAD_FL_SINK,
> MEDIA_PAD_FL_SOURCE},
> - &vimc_sca_ops);
> + &vimc_sca_int_ops, &vimc_sca_ops);
> if (ret) {
> kfree(vsca);
> return ret;
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c
> b/drivers/media/platform/vimc/vimc-sensor.c
> index 59195f262623..d7891d3bbeaa 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -301,6 +301,20 @@ static const struct v4l2_ctrl_ops vimc_sen_ctrl_ops = {
> .s_ctrl = vimc_sen_s_ctrl,
> };
>
> +static void vimc_sen_release(struct v4l2_subdev *sd)
> +{
> + struct vimc_sen_device *vsen =
> + container_of(sd, struct vimc_sen_device, sd);
> +
> + v4l2_ctrl_handler_free(&vsen->hdl);
> + tpg_free(&vsen->tpg);
> + kfree(vsen);
> +}
> +
> +static const struct v4l2_subdev_internal_ops vimc_sen_int_ops = {
> + .release = vimc_sen_release,
> +};
> +
> static void vimc_sen_comp_unbind(struct device *comp, struct device *master,
> void *master_data)
> {
> @@ -309,9 +323,6 @@ static void vimc_sen_comp_unbind(struct device *comp,
> struct device *master,
> container_of(ved, struct vimc_sen_device, ved);
>
> vimc_ent_sd_unregister(ved, &vsen->sd);
> - v4l2_ctrl_handler_free(&vsen->hdl);
> - tpg_free(&vsen->tpg);
> - kfree(vsen);
> }
>
> /* Image Processing Controls */
> @@ -371,7 +382,7 @@ static int vimc_sen_comp_bind(struct device *comp, struct
> device *master,
> pdata->entity_name,
> MEDIA_ENT_F_CAM_SENSOR, 1,
> (const unsigned long[1])
> {MEDIA_PAD_FL_SOURCE},
> - &vimc_sen_ops);
> + &vimc_sen_int_ops, &vimc_sen_ops);
> if (ret)
> goto err_free_hdl;
>
--
Regards,
Laurent Pinchart
