Hi

On Wed, 9 Dec 2009, Magnus Damm wrote:

> From: Magnus Damm <[email protected]>
> 
> Make sure physical addresses are 32-bit aligned in the
> SuperH Mobile CEU driver. The lowest two bits of the
> address registers are fixed to zero so frame buffers
> have to bit 32-bit aligned. The V4L2 mmap() case is
> using dma_alloc_coherent() for this driver which will
> return aligned addresses, but in the USERPTR case we
> must make sure that the user space pointer is valid.
> 
> Signed-off-by: Magnus Damm <[email protected]>
> ---
> 
>  drivers/media/video/sh_mobile_ceu_camera.c |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> --- 0001/drivers/media/video/sh_mobile_ceu_camera.c
> +++ work/drivers/media/video/sh_mobile_ceu_camera.c   2009-12-09 
> 17:16:47.000000000 +0900
> @@ -278,9 +278,14 @@ static int sh_mobile_ceu_capture(struct 
>  
>       phys_addr_top = videobuf_to_dma_contig(pcdev->active);
>       ceu_write(pcdev, CDAYR, phys_addr_top);
> +     if (phys_addr_top & 3)
> +             return -EINVAL;
> +

I'm afraid, no. This is too late to check buffer alignment in 
sh_mobile_ceu_capture(), which is called from qbuf and from the ISR to 
queue the next buffer. Besides, as comment for this function explains, its 
return code doesn't reflect success or failure to queue the new buffer, 
but the status of the previous one. These tests have to be done in 
sh_mobile_ceu_videobuf_prepare() and in .set_fmt(), where the geometry is 
set.

Thanks
Guennadi

>       if (pcdev->is_interlaced) {
>               phys_addr_bottom = phys_addr_top + icd->user_width;
>               ceu_write(pcdev, CDBYR, phys_addr_bottom);
> +             if (phys_addr_bottom & 3)
> +                     return -EINVAL;
>       }
>  
>       switch (icd->current_fmt->fourcc) {
> @@ -288,13 +293,16 @@ static int sh_mobile_ceu_capture(struct 
>       case V4L2_PIX_FMT_NV21:
>       case V4L2_PIX_FMT_NV16:
>       case V4L2_PIX_FMT_NV61:
> -             phys_addr_top += icd->user_width *
> -                     icd->user_height;
> +             phys_addr_top += icd->user_width * icd->user_height;
>               ceu_write(pcdev, CDACR, phys_addr_top);
> +             if (phys_addr_top & 3)
> +                     return -EINVAL;
> +
>               if (pcdev->is_interlaced) {
> -                     phys_addr_bottom = phys_addr_top +
> -                             icd->user_width;
> +                     phys_addr_bottom = phys_addr_top + icd->user_width;
>                       ceu_write(pcdev, CDBCR, phys_addr_bottom);
> +                     if (phys_addr_bottom & 3)
> +                             return -EINVAL;
>               }
>       }
>  
> 

---
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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to