Re: [PATCH v4] V4L: dynamically allocate video_device nodes in subdevices

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Guennadi,

On 09/13/2011 11:18 PM, Guennadi Liakhovetski wrote:
> On Tue, 13 Sep 2011, Sylwester Nawrocki wrote:
>> On 09/13/2011 04:48 PM, Guennadi Liakhovetski wrote:
>>> Currently only very few drivers actually use video_device nodes, embedded
>>> in struct v4l2_subdev. Allocate these nodes dynamically for those drivers
>>> to save memory for the rest.
>>>
>>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>>
>> I have tested this patch with Samsung FIMC driver and with MC enabled
>> sensor driver.
>> After some hundreds of module load/unload I didn't observe anything unusual.
>> The patch seem to be safe for device node enabled subdevs. You can stick my:
>>
>> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>
>> if you feel so.
> 
> Thanks very much for testing! However, depending on your test scenario, 
> you might still not notice a problem by just loading and unloading of 
> modules. It would, however, be useful to execute just one test:
> 
> 1. add one line v4l2-device.c:
> 
> diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
> index a3b89f4..33226857 100644
> --- a/drivers/media/video/v4l2-device.c
> +++ b/drivers/media/video/v4l2-device.c
> @@ -195,6 +195,7 @@ EXPORT_SYMBOL_GPL(v4l2_device_register_subdev);
>  static void v4l2_device_release_subdev_node(struct video_device *vdev)
>  {
>  	struct v4l2_subdev *sd = video_get_drvdata(vdev);
> +	dev_info(&vdev->dev, "%s()\n", __func__);
>  	sd->devnode = NULL;
>  	kfree(vdev);
>  }
> 
> 2. with this patch start and stop capture
> 
> 3. check dmesg - v4l2_device_release_subdev_node() output should not be 
> there yet
> 
> 4. rmmod modules, then the output should be there
> 
> If you could test that - that would be great!

OK, I double checked if v4l2_device_release_subdev_node() is called at the right
time, i.e. I've also checked if the streaming works in between the module unload/load.

I'd added the printk and everything behaved as expected, other than I've tracked
down a few minor bugs in the drivers in the meantime;)

I'll keep your patch applied in my development tree.

Thanks,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center