Hi Andrzej,

Thanks for the patchset!

On Thu, May 16, 2013 at 10:14:33AM +0200, Andrzej Hajda wrote:
> This patch adds managed version of initialization
> function for v4l2 control handler.
> 
> Signed-off-by: Andrzej Hajda <a.ha...@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> ---
> v3:
>       - removed managed cleanup
> v2:
>       - added missing struct device forward declaration,
>       - corrected few comments
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c |   32 ++++++++++++++++++++++++++++++++
>  include/media/v4l2-ctrls.h           |   16 ++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index ebb8e48..f47ccfa 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1421,6 +1421,38 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler 
> *hdl)
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_handler_free);
>  
> +static void devm_v4l2_ctrl_handler_release(struct device *dev, void *res)
> +{
> +     struct v4l2_ctrl_handler **hdl = res;
> +
> +     v4l2_ctrl_handler_free(*hdl);

v4l2_ctrl_handler_free() acquires hdl->mutex which is independent of the
existence of hdl. By default hdl->lock is in the handler, but it may also be
elsewhere, e.g. in a driver-specific device struct such as struct
smiapp_sensor defined in drivers/media/i2c/smiapp/smiapp.h. I wonder if
anything guarantees that hdl->mutex still exists at the time the device is
removed.

I have to say I don't think it's neither meaningful to acquire that mutex in
v4l2_ctrl_handler_free(), though, since the whole going to be freed next
anyway: reference counting would be needed to prevent bad things from
happening, in case the drivers wouldn't take care of that.

> +}
> +

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi     XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to