linux-kernel - Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161123161511.GB1753@ct-lt-587>
Date:   Wed, 23 Nov 2016 16:15:11 +0000
From:   Javi Merino <javi.merino@...nel.org>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org,
        Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>
Subject: Re: [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in
 a dt overlay

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@...nel.org>
> > Cc: Javier Martinez Canillas <javier@....samsung.com>
> > Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > Signed-off-by: Javi Merino <javi.merino@...nel.org>
> > ---
> > 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@....fi	XMPP: sailus@...iisi.org.uk