* [PATCH 0/4] mx2_camera: mx25 fixes and enhancements
@ 2010-07-27 12:06 Baruch Siach
2010-07-27 12:06 ` [PATCH 1/4] mx2_camera: fix a race causing NULL dereference Baruch Siach
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Baruch Siach @ 2010-07-27 12:06 UTC (permalink / raw)
To: linux-arm-kernel
The first 3 pathces in this series are fixes for the mx2_camera driver which is
going upstream via the imx git tree. The last patch implements forced active
buffer termination on mx25.
Baruch Siach (4):
mx2_camera: fix a race causing NULL dereference
mx2_camera: return IRQ_NONE when doing nothing
mx2_camera: fix comment typo
mx2_camera: implement forced termination of active buffer for mx25
drivers/media/video/mx2_camera.c | 34 ++++++++++++++++++++++++++--------
1 files changed, 26 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] mx2_camera: fix a race causing NULL dereference
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
@ 2010-07-27 12:06 ` Baruch Siach
2010-07-27 12:06 ` [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing Baruch Siach
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2010-07-27 12:06 UTC (permalink / raw)
To: linux-arm-kernel
The mx25_camera_irq irq handler may get called after the camera has been
deactivated (from mx2_camera_deactivate). Detect this situation, and bail out.
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/media/video/mx2_camera.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 881d5d8..1536bd4 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -384,6 +384,9 @@ static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
spin_lock_irqsave(&pcdev->lock, flags);
+ if (*fb_active == NULL)
+ goto out;
+
vb = &(*fb_active)->vb;
dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
vb, vb->baddr, vb->bsize);
@@ -408,6 +411,7 @@ static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
*fb_active = buf;
+out:
spin_unlock_irqrestore(&pcdev->lock, flags);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
2010-07-27 12:06 ` [PATCH 1/4] mx2_camera: fix a race causing NULL dereference Baruch Siach
@ 2010-07-27 12:06 ` Baruch Siach
2010-07-28 6:53 ` Sascha Hauer
2010-07-28 11:25 ` Guennadi Liakhovetski
2010-07-27 12:06 ` [PATCH 3/4] mx2_camera: fix comment typo Baruch Siach
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Baruch Siach @ 2010-07-27 12:06 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/media/video/mx2_camera.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 1536bd4..b42ad8d 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -420,15 +420,17 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
struct mx2_camera_dev *pcdev = data;
u32 status = readl(pcdev->base_csi + CSISR);
- if (status & CSISR_DMA_TSF_FB1_INT)
+ writel(status, pcdev->base_csi + CSISR);
+
+ if (!(status & (CSISR_DMA_TSF_FB1_INT | CSISR_DMA_TSF_FB2_INT)))
+ return IRQ_NONE;
+ else if (status & CSISR_DMA_TSF_FB1_INT)
mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
else if (status & CSISR_DMA_TSF_FB2_INT)
mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
/* FIXME: handle CSISR_RFF_OR_INT */
- writel(status, pcdev->base_csi + CSISR);
-
return IRQ_HANDLED;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] mx2_camera: fix comment typo
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
2010-07-27 12:06 ` [PATCH 1/4] mx2_camera: fix a race causing NULL dereference Baruch Siach
2010-07-27 12:06 ` [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing Baruch Siach
@ 2010-07-27 12:06 ` Baruch Siach
2010-07-27 12:06 ` [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25 Baruch Siach
2010-08-23 4:11 ` [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
4 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2010-07-27 12:06 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/media/video/mx2_camera.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index b42ad8d..d327d11 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -469,7 +469,7 @@ static void free_buffer(struct videobuf_queue *vq, struct mx2_buffer *buf)
/*
* This waits until this buffer is out of danger, i.e., until it is no
- * longer in STATE_QUEUED or STATE_ACTIVE
+ * longer in state VIDEOBUF_QUEUED or VIDEOBUF_ACTIVE
*/
videobuf_waiton(vb, 0, 0);
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
` (2 preceding siblings ...)
2010-07-27 12:06 ` [PATCH 3/4] mx2_camera: fix comment typo Baruch Siach
@ 2010-07-27 12:06 ` Baruch Siach
2010-08-27 9:07 ` Guennadi Liakhovetski
2010-08-23 4:11 ` [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
4 siblings, 1 reply; 13+ messages in thread
From: Baruch Siach @ 2010-07-27 12:06 UTC (permalink / raw)
To: linux-arm-kernel
This allows userspace to terminate a capture without waiting for the current
frame to complete.
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/media/video/mx2_camera.c | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index d327d11..396542b 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -648,15 +648,27 @@ static void mx2_videobuf_release(struct videobuf_queue *vq,
* Terminate only queued but inactive buffers. Active buffers are
* released when they become inactive after videobuf_waiton().
*
- * FIXME: implement forced termination of active buffers, so that the
- * user won't get stuck in an uninterruptible state. This requires a
- * specific handling for each of the three DMA types that this driver
- * supports.
+ * FIXME: implement forced termination of active buffers for mx27 and
+ * mx27 eMMA, so that the user won't get stuck in an uninterruptible
+ * state. This requires a specific handling for each of the these DMA
+ * types.
*/
spin_lock_irqsave(&pcdev->lock, flags);
if (vb->state == VIDEOBUF_QUEUED) {
list_del(&vb->queue);
vb->state = VIDEOBUF_ERROR;
+ } else if (cpu_is_mx25() && vb->state == VIDEOBUF_ACTIVE) {
+ if (pcdev->fb1_active == buf) {
+ pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
+ writel(0, pcdev->base_csi + CSIDMASA_FB1);
+ pcdev->fb1_active = NULL;
+ } else if (pcdev->fb2_active == buf) {
+ pcdev->csicr1 &= ~CSICR1_FB2_DMA_INTEN;
+ writel(0, pcdev->base_csi + CSIDMASA_FB2);
+ pcdev->fb2_active = NULL;
+ }
+ writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
+ vb->state = VIDEOBUF_ERROR;
}
spin_unlock_irqrestore(&pcdev->lock, flags);
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing
2010-07-27 12:06 ` [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing Baruch Siach
@ 2010-07-28 6:53 ` Sascha Hauer
2010-07-28 7:27 ` Russell King - ARM Linux
2010-07-28 11:25 ` Guennadi Liakhovetski
1 sibling, 1 reply; 13+ messages in thread
From: Sascha Hauer @ 2010-07-28 6:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 27, 2010 at 03:06:08PM +0300, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> drivers/media/video/mx2_camera.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> index 1536bd4..b42ad8d 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -420,15 +420,17 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
> struct mx2_camera_dev *pcdev = data;
> u32 status = readl(pcdev->base_csi + CSISR);
>
> - if (status & CSISR_DMA_TSF_FB1_INT)
> + writel(status, pcdev->base_csi + CSISR);
> +
> + if (!(status & (CSISR_DMA_TSF_FB1_INT | CSISR_DMA_TSF_FB2_INT)))
> + return IRQ_NONE;
I'm not sure this is correct. When we get here, the interrupt definitely
is from the camera, it's not a shared interrupt. So this only provokes a
'nobody cared' message from the kernel (if it's still present, I don't
know).
Sascha
> + else if (status & CSISR_DMA_TSF_FB1_INT)
> mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
> else if (status & CSISR_DMA_TSF_FB2_INT)
> mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
>
> /* FIXME: handle CSISR_RFF_OR_INT */
>
> - writel(status, pcdev->base_csi + CSISR);
> -
> return IRQ_HANDLED;
> }
>
> --
> 1.7.1
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing
2010-07-28 6:53 ` Sascha Hauer
@ 2010-07-28 7:27 ` Russell King - ARM Linux
0 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux @ 2010-07-28 7:27 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jul 28, 2010 at 08:53:37AM +0200, Sascha Hauer wrote:
> On Tue, Jul 27, 2010 at 03:06:08PM +0300, Baruch Siach wrote:
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > drivers/media/video/mx2_camera.c | 8 +++++---
> > 1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> > index 1536bd4..b42ad8d 100644
> > --- a/drivers/media/video/mx2_camera.c
> > +++ b/drivers/media/video/mx2_camera.c
> > @@ -420,15 +420,17 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
> > struct mx2_camera_dev *pcdev = data;
> > u32 status = readl(pcdev->base_csi + CSISR);
> >
> > - if (status & CSISR_DMA_TSF_FB1_INT)
> > + writel(status, pcdev->base_csi + CSISR);
> > +
> > + if (!(status & (CSISR_DMA_TSF_FB1_INT | CSISR_DMA_TSF_FB2_INT)))
> > + return IRQ_NONE;
>
> I'm not sure this is correct. When we get here, the interrupt definitely
> is from the camera, it's not a shared interrupt. So this only provokes a
> 'nobody cared' message from the kernel (if it's still present, I don't
> know).
You'll only get the 'nobody cared' message if it's happened many times
in a short space of time. The odd spurious IRQ_NONE has little effect.
It is good practice to return IRQ_NONE if there's nothing pending - it
allows stuck IRQs to be detected and disabled without taking the system
down. In other words, it should make the system more robust.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing
2010-07-27 12:06 ` [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing Baruch Siach
2010-07-28 6:53 ` Sascha Hauer
@ 2010-07-28 11:25 ` Guennadi Liakhovetski
2010-08-09 11:46 ` Baruch Siach
1 sibling, 1 reply; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-07-28 11:25 UTC (permalink / raw)
To: linux-arm-kernel
A general comment to your patches: the actual driver is going to be merged
via the ARM tree, all other your incremental patches should rather go via
the v4l tree. So, we'll have to synchronise with ARM, let's hope ARM
patches go in early enough.
On Tue, 27 Jul 2010, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> drivers/media/video/mx2_camera.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> index 1536bd4..b42ad8d 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -420,15 +420,17 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
> struct mx2_camera_dev *pcdev = data;
> u32 status = readl(pcdev->base_csi + CSISR);
>
> - if (status & CSISR_DMA_TSF_FB1_INT)
> + writel(status, pcdev->base_csi + CSISR);
> +
> + if (!(status & (CSISR_DMA_TSF_FB1_INT | CSISR_DMA_TSF_FB2_INT)))
> + return IRQ_NONE;
> + else if (status & CSISR_DMA_TSF_FB1_INT)
> mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
> else if (status & CSISR_DMA_TSF_FB2_INT)
> mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
>
> /* FIXME: handle CSISR_RFF_OR_INT */
>
> - writel(status, pcdev->base_csi + CSISR);
> -
> return IRQ_HANDLED;
> }
I don't think this is correct. You should return IRQ_NONE if this is not
an interrupt from your device at all. In this case you don't have to ack
your interrupts, which, I presume, is what the write to CSISR is doing.
OTOH, if this is an interrupt from your device, but you're just not
interested in it, you should ack it and return IRQ_HANDLED. So, the
original behaviour was more correct, than what this your patch is doing.
The only improvement I can think of is, that you can return IRQ_NONE if
status is 0, but then you don't have to ack it.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing
2010-07-28 11:25 ` Guennadi Liakhovetski
@ 2010-08-09 11:46 ` Baruch Siach
0 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2010-08-09 11:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Guennadi,
On Wed, Jul 28, 2010 at 01:25:27PM +0200, Guennadi Liakhovetski wrote:
> A general comment to your patches: the actual driver is going to be merged
> via the ARM tree, all other your incremental patches should rather go via
> the v4l tree. So, we'll have to synchronise with ARM, let's hope ARM
> patches go in early enough.
Since the driver is now merged upstream this series can go via the v4l tree.
> On Tue, 27 Jul 2010, Baruch Siach wrote:
>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > drivers/media/video/mx2_camera.c | 8 +++++---
> > 1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> > index 1536bd4..b42ad8d 100644
> > --- a/drivers/media/video/mx2_camera.c
> > +++ b/drivers/media/video/mx2_camera.c
> > @@ -420,15 +420,17 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
> > struct mx2_camera_dev *pcdev = data;
> > u32 status = readl(pcdev->base_csi + CSISR);
> >
> > - if (status & CSISR_DMA_TSF_FB1_INT)
> > + writel(status, pcdev->base_csi + CSISR);
> > +
> > + if (!(status & (CSISR_DMA_TSF_FB1_INT | CSISR_DMA_TSF_FB2_INT)))
> > + return IRQ_NONE;
> > + else if (status & CSISR_DMA_TSF_FB1_INT)
> > mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
> > else if (status & CSISR_DMA_TSF_FB2_INT)
> > mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
> >
> > /* FIXME: handle CSISR_RFF_OR_INT */
> >
> > - writel(status, pcdev->base_csi + CSISR);
> > -
> > return IRQ_HANDLED;
> > }
>
> I don't think this is correct. You should return IRQ_NONE if this is not
> an interrupt from your device at all. In this case you don't have to ack
> your interrupts, which, I presume, is what the write to CSISR is doing.
> OTOH, if this is an interrupt from your device, but you're just not
> interested in it, you should ack it and return IRQ_HANDLED. So, the
> original behaviour was more correct, than what this your patch is doing.
> The only improvement I can think of is, that you can return IRQ_NONE if
> status is 0, but then you don't have to ack it.
OK. Drop this one, then. Patches in this series are independent from each
other, so the others can go in.
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/4] mx2_camera: mx25 fixes and enhancements
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
` (3 preceding siblings ...)
2010-07-27 12:06 ` [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25 Baruch Siach
@ 2010-08-23 4:11 ` Baruch Siach
2010-08-23 22:05 ` Guennadi Liakhovetski
4 siblings, 1 reply; 13+ messages in thread
From: Baruch Siach @ 2010-08-23 4:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Guennadi,
On Tue, Jul 27, 2010 at 03:06:06PM +0300, Baruch Siach wrote:
> The first 3 pathces in this series are fixes for the mx2_camera driver which is
> going upstream via the imx git tree. The last patch implements forced active
> buffer termination on mx25.
Ping?
> Baruch Siach (4):
> mx2_camera: fix a race causing NULL dereference
> mx2_camera: return IRQ_NONE when doing nothing
> mx2_camera: fix comment typo
> mx2_camera: implement forced termination of active buffer for mx25
>
> drivers/media/video/mx2_camera.c | 34 ++++++++++++++++++++++++++--------
> 1 files changed, 26 insertions(+), 8 deletions(-)
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/4] mx2_camera: mx25 fixes and enhancements
2010-08-23 4:11 ` [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
@ 2010-08-23 22:05 ` Guennadi Liakhovetski
0 siblings, 0 replies; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-08-23 22:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Baruch
On Mon, 23 Aug 2010, Baruch Siach wrote:
> Hi Guennadi,
>
> On Tue, Jul 27, 2010 at 03:06:06PM +0300, Baruch Siach wrote:
> > The first 3 pathces in this series are fixes for the mx2_camera driver which is
> > going upstream via the imx git tree. The last patch implements forced active
> > buffer termination on mx25.
>
> Ping?
Sorry for taking a bit long to push your patches, but, I think, we still
have a bit of time. Fixes are ok to go in after -rc2 (or even -rc3, or
-rc4...), with features we're anyway late for 2.6.36, so, they will have
to wait until 2.6.37, for which we have some time too. So, I think we're
doing fine so far. Of the four patches below patch two was unclear,
whether we want it or not, patches one and three were ok, AFAIR, will have
to double-check, patch 4 I'll have to decide whether that's a fix or a
feature;)
> > Baruch Siach (4):
> > mx2_camera: fix a race causing NULL dereference
> > mx2_camera: return IRQ_NONE when doing nothing
> > mx2_camera: fix comment typo
> > mx2_camera: implement forced termination of active buffer for mx25
> >
> > drivers/media/video/mx2_camera.c | 34 ++++++++++++++++++++++++++--------
> > 1 files changed, 26 insertions(+), 8 deletions(-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25
2010-07-27 12:06 ` [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25 Baruch Siach
@ 2010-08-27 9:07 ` Guennadi Liakhovetski
2010-08-29 7:56 ` Baruch Siach
0 siblings, 1 reply; 13+ messages in thread
From: Guennadi Liakhovetski @ 2010-08-27 9:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Baruch
On Tue, 27 Jul 2010, Baruch Siach wrote:
> This allows userspace to terminate a capture without waiting for the current
> frame to complete.
This is an improvement, not a fix, right? Without this patch the
termination just have to wait a couple of ms longer? so, it is ok to
schedule it for 2.6.37?
Thanks
Guennadi
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> drivers/media/video/mx2_camera.c | 20 ++++++++++++++++----
> 1 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> index d327d11..396542b 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -648,15 +648,27 @@ static void mx2_videobuf_release(struct videobuf_queue *vq,
> * Terminate only queued but inactive buffers. Active buffers are
> * released when they become inactive after videobuf_waiton().
> *
> - * FIXME: implement forced termination of active buffers, so that the
> - * user won't get stuck in an uninterruptible state. This requires a
> - * specific handling for each of the three DMA types that this driver
> - * supports.
> + * FIXME: implement forced termination of active buffers for mx27 and
> + * mx27 eMMA, so that the user won't get stuck in an uninterruptible
> + * state. This requires a specific handling for each of the these DMA
> + * types.
> */
> spin_lock_irqsave(&pcdev->lock, flags);
> if (vb->state == VIDEOBUF_QUEUED) {
> list_del(&vb->queue);
> vb->state = VIDEOBUF_ERROR;
> + } else if (cpu_is_mx25() && vb->state == VIDEOBUF_ACTIVE) {
> + if (pcdev->fb1_active == buf) {
> + pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
> + writel(0, pcdev->base_csi + CSIDMASA_FB1);
> + pcdev->fb1_active = NULL;
> + } else if (pcdev->fb2_active == buf) {
> + pcdev->csicr1 &= ~CSICR1_FB2_DMA_INTEN;
> + writel(0, pcdev->base_csi + CSIDMASA_FB2);
> + pcdev->fb2_active = NULL;
> + }
> + writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
> + vb->state = VIDEOBUF_ERROR;
> }
> spin_unlock_irqrestore(&pcdev->lock, flags);
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25
2010-08-27 9:07 ` Guennadi Liakhovetski
@ 2010-08-29 7:56 ` Baruch Siach
0 siblings, 0 replies; 13+ messages in thread
From: Baruch Siach @ 2010-08-29 7:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Guennadi,
On Fri, Aug 27, 2010 at 11:07:31AM +0200, Guennadi Liakhovetski wrote:
> On Tue, 27 Jul 2010, Baruch Siach wrote:
> > This allows userspace to terminate a capture without waiting for the
> > current
> > frame to complete.
>
> This is an improvement, not a fix, right? Without this patch the
> termination just have to wait a couple of ms longer? so, it is ok to
> schedule it for 2.6.37?
In my situation the image data source may stop sending data in the middle of a
frame. In this case userspace is stuck forever. So, this is a real fix for
me. This is not the usual use case, I guess, so I leave it for you to decide.
baruch
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > drivers/media/video/mx2_camera.c | 20 ++++++++++++++++----
> > 1 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> > index d327d11..396542b 100644
> > --- a/drivers/media/video/mx2_camera.c
> > +++ b/drivers/media/video/mx2_camera.c
> > @@ -648,15 +648,27 @@ static void mx2_videobuf_release(struct videobuf_queue *vq,
> > * Terminate only queued but inactive buffers. Active buffers are
> > * released when they become inactive after videobuf_waiton().
> > *
> > - * FIXME: implement forced termination of active buffers, so that the
> > - * user won't get stuck in an uninterruptible state. This requires a
> > - * specific handling for each of the three DMA types that this driver
> > - * supports.
> > + * FIXME: implement forced termination of active buffers for mx27 and
> > + * mx27 eMMA, so that the user won't get stuck in an uninterruptible
> > + * state. This requires a specific handling for each of the these DMA
> > + * types.
> > */
> > spin_lock_irqsave(&pcdev->lock, flags);
> > if (vb->state == VIDEOBUF_QUEUED) {
> > list_del(&vb->queue);
> > vb->state = VIDEOBUF_ERROR;
> > + } else if (cpu_is_mx25() && vb->state == VIDEOBUF_ACTIVE) {
> > + if (pcdev->fb1_active == buf) {
> > + pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
> > + writel(0, pcdev->base_csi + CSIDMASA_FB1);
> > + pcdev->fb1_active = NULL;
> > + } else if (pcdev->fb2_active == buf) {
> > + pcdev->csicr1 &= ~CSICR1_FB2_DMA_INTEN;
> > + writel(0, pcdev->base_csi + CSIDMASA_FB2);
> > + pcdev->fb2_active = NULL;
> > + }
> > + writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
> > + vb->state = VIDEOBUF_ERROR;
> > }
> > spin_unlock_irqrestore(&pcdev->lock, flags);
> >
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-08-29 7:56 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-27 12:06 [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
2010-07-27 12:06 ` [PATCH 1/4] mx2_camera: fix a race causing NULL dereference Baruch Siach
2010-07-27 12:06 ` [PATCH 2/4] mx2_camera: return IRQ_NONE when doing nothing Baruch Siach
2010-07-28 6:53 ` Sascha Hauer
2010-07-28 7:27 ` Russell King - ARM Linux
2010-07-28 11:25 ` Guennadi Liakhovetski
2010-08-09 11:46 ` Baruch Siach
2010-07-27 12:06 ` [PATCH 3/4] mx2_camera: fix comment typo Baruch Siach
2010-07-27 12:06 ` [PATCH 4/4] mx2_camera: implement forced termination of active buffer for mx25 Baruch Siach
2010-08-27 9:07 ` Guennadi Liakhovetski
2010-08-29 7:56 ` Baruch Siach
2010-08-23 4:11 ` [PATCH 0/4] mx2_camera: mx25 fixes and enhancements Baruch Siach
2010-08-23 22:05 ` Guennadi Liakhovetski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).