Re: [PATCH RFC] omap3isp: prevent releasing MC too early

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Mauro,

(CC'ing Greg)

On Wednesday 14 Dec 2016 13:14:06 Mauro Carvalho Chehab wrote:
> Avoid calling streamoff without having the media structs allocated.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

The driver has a maintainer listed in MAINTAINERS, and you know that Sakari is 
also actively involved here. You could have CC'ed us.

> ---
> 
> Javier,
> 
> Could you please test this patch?
> 
> Thanks!
> Mauro
> 
>  drivers/media/platform/omap3isp/ispvideo.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index
> 7354469670b7..f60995ed0a1f 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1488,11 +1488,17 @@ int omap3isp_video_register(struct isp_video *video,
> struct v4l2_device *vdev) "%s: could not register video device (%d)\n",
>  			__func__, ret);
> 
> +	/* Prevent destroying MC before unregistering */
> +	kobject_get(vdev->v4l2_dev->mdev->devnode->dev.parent);

This doesn't even compile. Please make sure to at least compile-test patches 
you send for review, otherwise you end up wasting time for all reviewers and 
testers. I assume you meant

	kobject_get(&vdev->mdev->devnode->dev.parent->kobj);

and similarly below.

That's a long list of pointer dereferences, going deep down the device core. 
Greg, are drivers allowed to do this by the driver model ?

> +
>  	return ret;
>  }
> 
>  void omap3isp_video_unregister(struct isp_video *video)
>  {
> -	if (video_is_registered(&video->video))
> -		video_unregister_device(&video->video);
> +	if (!video_is_registered(&video->video))
> +		return;
> +
> +	video_unregister_device(&video->video);
> +	kobject_put(vdev->v4l2_dev->mdev->devnode->dev.parent);
>  }

-- 
Regards,

Laurent Pinchart