On Wed, Nov 23, 2016 at 05:10:42PM +0200, Sakari Ailus wrote:
> Hi Javi,
Hi Sakari,
> On Wed, Nov 23, 2016 at 10:09:57AM +0000, Javi Merino wrote:
> > In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
> > a devicetree overlay, its of_node pointer will be different each time
> > the overlay is applied. We are not interested in matching the
> > pointer, what we want to match is that the path is the one we are
> > expecting. Change to use of_node_cmp() so that we continue matching
> > after the overlay has been removed and reapplied.
> >
> > Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> > Cc: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> > Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Signed-off-by: Javi Merino <javi.merino@xxxxxxxxxx>
> > ---
> > Hi,
> >
> > I feel it is a bit of a hack, but I couldn't think of anything better.
> > I'm ccing devicetree@ and Pantelis because there may be a simpler
> > solution.
>
> First I have to admit that I'm not an expert when it comes to DT overlays.
>
> That said, my understanding is that the sub-device and the async sub-device
> are supposed to point to the exactly same DT node. I wonder if there's
> actually anything wrong in the current code.
>
> If the overlay has changed between probing the driver for the async notifier
> and the async sub-device, there should be no match here, should there? The
> two nodes actually point to a node in a different overlay in that case.
Overlays are parts of the devicetree that can be added and removed.
When the overlay is applied, the camera driver is probed and does
v4l2_async_register_subdev(). However, v4l2_async_belongs() fails.
The problem is with comparing pointers. I haven't looked at the
implementation of overlays in detail, but what I see is that the
of_node pointer changes when you remove and reapply an overlay (I
guess it's dynamically allocated and when you remove the overlay, it's
freed).
Cheers,
Javi
> >
> > drivers/media/v4l2-core/v4l2-async.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> > index 5bada20..d33a17c 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
> >
> > static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> > {
> > - return sd->of_node == asd->match.of.node;
> > + return !of_node_cmp(of_node_full_name(sd->of_node),
> > + of_node_full_name(asd->match.of.node));
> > }
> >
> > static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
>
> --
> Kind regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
