On Fri, 20 Jan 2012, Javier Martin wrote:

> 
> Signed-off-by: Javier Martin <javier.mar...@vista-silicon.com>
> ---
>  drivers/media/video/mx2_camera.c |   23 +++++++++--------------
>  1 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/video/mx2_camera.c 
> b/drivers/media/video/mx2_camera.c
> index e0c5dd4..cdc614f 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -1274,7 +1274,10 @@ static void mx27_camera_frame_done_emma(struct 
> mx2_camera_dev *pcdev,
>               buf->state = state;
>               do_gettimeofday(&vb->v4l2_buf.timestamp);
>               vb->v4l2_buf.sequence = pcdev->frame_count;
> -             vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> +             if (state == MX2_STATE_ERROR)
> +                     vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> +             else
> +                     vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
>       }
>  
>       pcdev->frame_count++;
> @@ -1309,19 +1312,11 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma, 
> void *data)
>       struct mx2_buffer *buf;
>  
>       if (status & (1 << 7)) { /* overflow */
> -             u32 cntl;
> -             /*
> -              * We only disable channel 1 here since this is the only
> -              * enabled channel
> -              *
> -              * FIXME: the correct DMA overflow handling should be resetting
> -              * the buffer, returning an error frame, and continuing with
> -              * the next one.
> -              */
> -             cntl = readl(pcdev->base_emma + PRP_CNTL);
> -             writel(cntl & ~(PRP_CNTL_CH1EN | PRP_CNTL_CH2EN),
> -                    pcdev->base_emma + PRP_CNTL);
> -             writel(cntl, pcdev->base_emma + PRP_CNTL);
> +             buf = list_entry(pcdev->active_bufs.next,
> +                     struct mx2_buffer, queue);
> +             mx27_camera_frame_done_emma(pcdev,
> +                                     buf->bufnum, MX2_STATE_ERROR);
> +             status &= ~(1 << 7);
>       }
>       if ((((status & (3 << 5)) == (3 << 5)) ||

Does it make sense continuing processing here, if an error occurred? To me 
all the four "if" statements in this function seem mutually-exclusive and 
should be handled by a

        if () {
        } else if () {
        ...
chain.

>               ((status & (3 << 3)) == (3 << 3)))
> -- 
> 1.7.0.4
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to