* [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
@ 2013-12-18 7:29 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-12-16 15:06 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, Laurent Pinchart, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
The spin_lock_irqsave() macro is not nestable. The second call will
overwrite the first record of "flags" so the IRQs will not be enabled
correctly at the end of the function.
I haven't looked at all the callers but it could be that this function
is always called with interrupts disabled and the bug doesn't cause
problems in real life.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 766491e6a8d0..c9b71c750b15 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
}
if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
- spin_lock_irqsave(&pipe->lock, flags);
+ spin_lock(&pipe->lock);
pipe->state &= ~ISS_PIPELINE_STREAM;
- spin_unlock_irqrestore(&pipe->lock, flags);
+ spin_unlock(&pipe->lock);
}
buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
^ permalink raw reply related [flat|nested] 6+ messages in thread* [patch v2] [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
@ 2013-12-18 7:29 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-12-18 7:29 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, Laurent Pinchart, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
The spin_lock_irqsave() macro is not nestable. The second call will
overwrite the first record of "flags" so the IRQs will not be enabled
correctly at the end of the function.
In the current code, this function is always called from the IRQ handler
so everything works fine and this fix doesn't change anything.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Updated the change log
diff --git a/drivers/staging/media/omap4iss/iss_video.c b/drivers/staging/media/omap4iss/iss_video.c
index 766491e6a8d0..c9b71c750b15 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct iss_video *video)
}
if (video->type = V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
- spin_lock_irqsave(&pipe->lock, flags);
+ spin_lock(&pipe->lock);
pipe->state &= ~ISS_PIPELINE_STREAM;
- spin_unlock_irqrestore(&pipe->lock, flags);
+ spin_unlock(&pipe->lock);
}
buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
2013-12-18 7:29 ` [patch v2] " Dan Carpenter
@ 2013-12-17 17:28 ` Laurent Pinchart
-1 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-12-17 17:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
Hi Dan,
Thank you for the patch.
On Monday 16 December 2013 18:06:12 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> I haven't looked at all the callers but it could be that this function
> is always called with interrupts disabled and the bug doesn't cause
> problems in real life.
The function is called from interrupt handlers only. Would you like to
rephrase the commit message accordingly ?
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type = V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
@ 2013-12-17 17:28 ` Laurent Pinchart
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-12-17 17:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, Hans Verkuil,
Sergio Aguirre, linux-media, devel, kernel-janitors
Hi Dan,
Thank you for the patch.
On Monday 16 December 2013 18:06:12 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> I haven't looked at all the callers but it could be that this function
> is always called with interrupts disabled and the bug doesn't cause
> problems in real life.
The function is called from interrupt handlers only. Would you like to
rephrase the commit message accordingly ?
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch v2] [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
2013-12-18 7:29 ` [patch v2] " Dan Carpenter
@ 2013-12-19 1:03 ` Laurent Pinchart
-1 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-12-19 1:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, devel, Greg Kroah-Hartman, kernel-janitors,
Sergio Aguirre, Hans Verkuil, linux-media
Hi Dan,
Thank you for the patch.
On Wednesday 18 December 2013 10:29:24 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> In the current code, this function is always called from the IRQ handler
> so everything works fine and this fix doesn't change anything.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
and applied to my tree. I'll send a pull request shortly.
> ---
> v2: Updated the change log
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type = V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch v2] [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next()
@ 2013-12-19 1:03 ` Laurent Pinchart
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-12-19 1:03 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, devel, Greg Kroah-Hartman, kernel-janitors,
Sergio Aguirre, Hans Verkuil, linux-media
Hi Dan,
Thank you for the patch.
On Wednesday 18 December 2013 10:29:24 Dan Carpenter wrote:
> The spin_lock_irqsave() macro is not nestable. The second call will
> overwrite the first record of "flags" so the IRQs will not be enabled
> correctly at the end of the function.
>
> In the current code, this function is always called from the IRQ handler
> so everything works fine and this fix doesn't change anything.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
and applied to my tree. I'll send a pull request shortly.
> ---
> v2: Updated the change log
>
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> 766491e6a8d0..c9b71c750b15 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -451,9 +451,9 @@ struct iss_buffer *omap4iss_video_buffer_next(struct
> iss_video *video) }
>
> if (video->type == V4L2_BUF_TYPE_VIDEO_CAPTURE && pipe->input != NULL) {
> - spin_lock_irqsave(&pipe->lock, flags);
> + spin_lock(&pipe->lock);
> pipe->state &= ~ISS_PIPELINE_STREAM;
> - spin_unlock_irqrestore(&pipe->lock, flags);
> + spin_unlock(&pipe->lock);
> }
>
> buf = list_first_entry(&video->dmaqueue, struct iss_buffer,
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-19 1:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-16 15:06 [media] v4l: omap4iss: Restore irq flags correctly in omap4iss_video_buffer_next() Dan Carpenter
2013-12-18 7:29 ` [patch v2] " Dan Carpenter
2013-12-17 17:28 ` Laurent Pinchart
2013-12-17 17:28 ` Laurent Pinchart
2013-12-19 1:03 ` [patch v2] " Laurent Pinchart
2013-12-19 1:03 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.