Hi, On 2017-08-18 14:15:26 +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 15 Aug 2017 19:16:14 Sakari Ailus wrote: > > On Mon, Jul 31, 2017 at 12:31:55AM +0200, Niklas Söderlund wrote: > > > The call to v4l2_async_cleanup() will set sd->asd to NULL so passing it > > > to notifier->unbind() have no effect and leaves the notifier confused. > > > Call the unbind() callback prior to cleaning up the subdevice to avoid > > > this. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se> > > > > This is a bugfix and worthy without any other patches and so should be > > applied separately. > > > > I think it'd be safer to store sd->asd locally and call the notifier unbind > > with that. Now you're making changes to the order in which things work, and > > that's not necessary to achieve the objective of passing the async subdev > > pointer to the notifier. > > But on the other hand I think the unbind notification should be called before > the subdevice gets unbound, the same way the bound notification is called > after it gets bound. One of the purposes of the unbind notification is to > allow drivers to prepare for subdev about to be unbound, and they can't > prepare if the unbind happened already.
I'm not opposed to move in the direction suggested by Sakari but I agree with Laurent here. It makes more sens that the unbind callback is called before the actual unbind happens. At the same time I agree that it dose change the behavior, but I think it's for the better. > > > With that changed, > > > > Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com> > > > > > --- > > > > > > drivers/media/v4l2-core/v4l2-async.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-async.c > > > b/drivers/media/v4l2-core/v4l2-async.c index > > > 851f128eba2219ad..0acf288d7227ba97 100644 > > > --- a/drivers/media/v4l2-core/v4l2-async.c > > > +++ b/drivers/media/v4l2-core/v4l2-async.c > > > @@ -226,14 +226,14 @@ void v4l2_async_notifier_unregister(struct > > > v4l2_async_notifier *notifier)> > > > d = get_device(sd->dev); > > > > > > + if (notifier->unbind) > > > + notifier->unbind(notifier, sd, sd->asd); > > > + > > > > > > v4l2_async_cleanup(sd); > > > > > > /* If we handled USB devices, we'd have to lock the parent too > */ > > > device_release_driver(d); > > > > > > - if (notifier->unbind) > > > - notifier->unbind(notifier, sd, sd->asd); > > > - > > > > > > /* > > > > > > * Store device at the device cache, in order to call > > > * put_device() on the final step > > -- > Regards, > > Laurent Pinchart > -- Regards, Niklas Söderlund
