diff mbox

[2/4,media] davinci: vpif_capture: don't lock over s_stream

Message ID 20161119003208.10550-2-khilman@baylibre.com (mailing list archive)
State Superseded, archived
Delegated to: Hans Verkuil
Headers

Commit Message

Kevin Hilman Nov. 19, 2016, 12:32 a.m. UTC 
  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(+)

Comments

Hans Verkuil Nov. 21, 2016, 2:37 p.m. UTC | #1 
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
Kevin Hilman Nov. 22, 2016, 12:10 a.m. UTC | #2 
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 mbox

Patch

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) {