Hi Hans, On Thursday 27 October 2011 13:18:01 Hans de Goede wrote: > 1: There is no reason for this after v4l2_event_unsubscribe releases the > spinlock nothing is holding a reference to the sev anymore except for the > local reference in the v4l2_event_unsubscribe function. > > 2: Setting sev->fh to NULL causes problems for the del op added in the next > patch of this series, since this op needs a way to get to its own data > structures, and typically this will be done by using container_of on an > embedded v4l2_fh struct. > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> While reviewing the patch I noticed that v4l2_event_unsubscribe_all() calls v4l2_event_unsubscribe(), which performs control lookup again. Is there a reason for that, instead of handling event unsubscription directly in v4l2_event_unsubscribe_all() ? > --- > drivers/media/video/v4l2-event.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/video/v4l2-event.c > b/drivers/media/video/v4l2-event.c index 01cbb7f..3d27300 100644 > --- a/drivers/media/video/v4l2-event.c > +++ b/drivers/media/video/v4l2-event.c > @@ -304,7 +304,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh, > } > } > list_del(&sev->list); > - sev->fh = NULL; > } > > spin_unlock_irqrestore(&fh->vdev->fh_lock, flags); -- Regards, Laurent Pinchart -- 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
