Re: [PATCH] s5p-fimc: Fix platform entities registration

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

Hi Laurent,

On 10/25/2012 01:35 PM, Laurent Pinchart wrote:
> On Thursday 25 October 2012 11:06:56 Sylwester Nawrocki wrote:
>> Make sure there is no v4l2_device_unregister_subdev() call
>> on a subdev which wasn't registered.
> 
> I'm not implying that this fix is bad, but doesn't the V4L2 core already 
> handle this ? v4l2_device_unregister_subdev() returns immediately without 
> doing anything if the subdev hasn't been registered.

Indeed, the patch summary might be a bit misleading and incomplete.
I of course wanted to make sure the platform subdevs are not treated
as registered when any part of v4l2_device_register_subdev() fails.


Looking at function v4l2_device_register_subdev(), I'm wondering whether
line
 159         sd->v4l2_dev = v4l2_dev;

shouldn't be moved right before

 190         spin_lock(&v4l2_dev->lock);

so sd->v4l2_dev is set only if we return 0 in this function ?

Since in function v4l2_device_unregister_subdev() there is a check like

 259         /* return if it isn't registered */
 260         if (sd == NULL || sd->v4l2_dev == NULL)
 261                 return;

i.e. if subdev is not really registered, e.g. internal .registered
op fails, it should be NULL.

In my case sd wasn't null since this structure was embedded in
other one.

--

Thanks,
Sylwester