Hi Laurent,
On Tue, Jun 02, 2015 at 05:47:12AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch. Please see below for one small comment.
>
> On Monday 01 June 2015 01:24:39 Sakari Ailus wrote:
> > V4L2 async sub-devices are currently matched (OF case) based on the struct
> > device_node pointer in struct device. LED devices may have more than one
> > LED, and in that case the OF node to match is not directly the device's
> > node, but a LED's node.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> > ---
> > since v1:
> >
> > - Move conditional setting of struct v4l2_subdev.of_node from
> > v4l2_device_register_subdev() to v4l2_async_register_subdev.
> >
> > - Remove the check for NULL struct v4l2_subdev.of_node from match_of() as
> > it's no longer needed.
> >
> > - Unconditionally state in the struct v4l2_subdev.of_node field comment that
> > the field contains (a pointer to) the sub-device's of_node.
> >
> > drivers/media/v4l2-core/v4l2-async.c | 34 +++++++++++++++++++++------------
> > include/media/v4l2-subdev.h | 2 ++
> > 2 files changed, 24 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c index 85a6a34..b0badac 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
>
> [snip]
>
> > @@ -266,6 +273,9 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> > {
> > struct v4l2_async_notifier *notifier;
> >
> > + if (!sd->of_node && sd->dev)
> > + sd->of_node = sd->dev->of_node;
> > +
>
> I think we don't need to take a reference to of_node here, as we assume
> there's a reference to dev through the whole life of the subdev, and dev
> should have a reference to of_node, but could you double-check ?
>
> If that's indeed not a problem,
>
> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Good question.
I don't think a reference would be needed also because we are only
interested in the pointer value itself, the memory the pointer is pointing
to is never accessed.
As far as I can tell, there's no reference through the device.
I can add a comment on this if you like. Perhaps it'd be a good idea
actually, as you usually do take a reference in such cases.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
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
