Hello Mauro,
Patch looks almost good to me, I just have a question below:
On Wed, Mar 16, 2016 at 9:04 AM, Mauro Carvalho Chehab
<mchehab@xxxxxxxxxxxxxxx> wrote:
> Now that the media_device can be used by multiple drivers,
> via devres, we need to be sure that it will be dropped only
> when all drivers stop using it.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
> ---
> drivers/media/media-device.c | 48 +++++++++++++++++++++++++++++++-------------
> include/media/media-device.h | 3 +++
> 2 files changed, 37 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index c32fa15cc76e..38e6c319fe6e 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -721,6 +721,15 @@ int __must_check __media_device_register(struct media_device *mdev,
> {
> int ret;
>
> + /* Check if mdev was ever registered at all */
> + mutex_lock(&mdev->graph_mutex);
> + if (media_devnode_is_registered(&mdev->devnode)) {
> + kref_get(&mdev->kref);
> + mutex_unlock(&mdev->graph_mutex);
> + return 0;
> + }
> + kref_init(&mdev->kref);
> +
> /* Register the device node. */
> mdev->devnode.fops = &media_device_fops;
> mdev->devnode.parent = mdev->dev;
> @@ -730,8 +739,10 @@ int __must_check __media_device_register(struct media_device *mdev,
> mdev->topology_version = 0;
>
> ret = media_devnode_register(&mdev->devnode, owner);
> - if (ret < 0)
> + if (ret < 0) {
> + media_devnode_unregister(&mdev->devnode);
Why are you adding this? If media_devnode_register() failed then the
device node won't be registered so is not correct to call
media_devnode_unregister(). Or maybe I'm missing something.
Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
