Message ID | 20161119003208.10550-2-khilman@baylibre.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 19/11/16 01:32, Kevin Hilman wrote: > Video capture subdevs may be over I2C and may sleep during xfer, so we > cannot do IRQ-disabled locking when calling the subdev. > > Signed-off-by: Kevin Hilman <khilman@baylibre.com> > --- > drivers/media/platform/davinci/vpif_capture.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c > index 79cef74e164f..becc3e63b472 100644 > --- a/drivers/media/platform/davinci/vpif_capture.c > +++ b/drivers/media/platform/davinci/vpif_capture.c > @@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) > } > } > > + spin_unlock_irqrestore(&common->irqlock, flags); > + > ret = v4l2_subdev_call(ch->sd, video, s_stream, 1); > if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) { > vpif_dbg(1, debug, "stream on failed in subdev\n"); > goto err; > } > > + spin_lock_irqsave(&common->irqlock, flags); This needs to be moved to right after the v4l2_subdev_call, otherwise the goto err above will not have the spinlock. Hans > + > /* Call vpif_set_params function to set the parameters and addresses */ > ret = vpif_set_video_params(vpif, ch->channel_id); > if (ret < 0) { > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans Verkuil <hverkuil@xs4all.nl> writes: > On 19/11/16 01:32, Kevin Hilman wrote: >> Video capture subdevs may be over I2C and may sleep during xfer, so we >> cannot do IRQ-disabled locking when calling the subdev. >> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com> >> --- >> drivers/media/platform/davinci/vpif_capture.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c >> index 79cef74e164f..becc3e63b472 100644 >> --- a/drivers/media/platform/davinci/vpif_capture.c >> +++ b/drivers/media/platform/davinci/vpif_capture.c >> @@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) >> } >> } >> >> + spin_unlock_irqrestore(&common->irqlock, flags); >> + >> ret = v4l2_subdev_call(ch->sd, video, s_stream, 1); >> if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) { >> vpif_dbg(1, debug, "stream on failed in subdev\n"); >> goto err; >> } >> >> + spin_lock_irqsave(&common->irqlock, flags); > > This needs to be moved to right after the v4l2_subdev_call, otherwise the > goto err above will not have the spinlock. Yes indeed. Will respin. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index 79cef74e164f..becc3e63b472 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -193,12 +193,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) } } + spin_unlock_irqrestore(&common->irqlock, flags); + ret = v4l2_subdev_call(ch->sd, video, s_stream, 1); if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV) { vpif_dbg(1, debug, "stream on failed in subdev\n"); goto err; } + spin_lock_irqsave(&common->irqlock, flags); + /* Call vpif_set_params function to set the parameters and addresses */ ret = vpif_set_video_params(vpif, ch->channel_id); if (ret < 0) {
Video capture subdevs may be over I2C and may sleep during xfer, so we cannot do IRQ-disabled locking when calling the subdev. Signed-off-by: Kevin Hilman <khilman@baylibre.com> --- drivers/media/platform/davinci/vpif_capture.c | 4 ++++ 1 file changed, 4 insertions(+)