sh_mobile_ceu_camera: Add physical address alignment checks

  From: Magnus Damm <damm@opensource.se>

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 <damm@opensource.se>
---

 drivers/media/video/sh_mobile_ceu_camera.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

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

Hi

On Wed, 9 Dec 2009, Magnus Damm wrote:

> From: Magnus Damm <damm@opensource.se>
> 
> 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 <damm@opensource.se>
> ---
> 
>  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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html