On Fri, Nov 25, 2016 at 10:21:21AM +0200, Sakari Ailus wrote: Hi Sakari, > On Wed, Nov 23, 2016 at 04:15:11PM +0000, Javi Merino wrote: > > 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). > > The concern here which we were discussing was whether the overlay should be > relied on having compliant configuration compared to the part which was not > part of the overlay. > > As external components are involved, quite possibly also the ISP DT node > will require changes, not just the image source (TV tuner, camera sensor > etc.). This could be because of number of CSI-2 lanes or parallel bus width, > for instance. > > I'd also be interested in having an actual driver implement support for > removing and adding a DT overlay first so we'd see how this would actually > work. We need both in order to be able to actually remove and add DT > overlays _without_ unbinding the ISP driver. Otherwise it should already > work in the current codebase. Unfortunately, the driver I'm working on is not upstream and I can't submit it to mainline. This patch fixes the issue for me, so I thought it could be useful fix for the kernel. Cheers, Javi -- 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
