Re: [PATCH] V4L/DVB: mx1-camera: compile fix

From: Guennadi Liakhovetski
Date: Tue Mar 16 2010 - 07:01:45 EST


On Tue, 16 Mar 2010, Uwe Kleine-König wrote:

> This is a regression of
>
> 7d58289 (mx1: prefix SOC specific defines with MX1_ and deprecate old names)
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
> arch/arm/plat-mxc/include/mach/dma-mx1-mx2.h | 4 +++-
> drivers/media/video/mx1_camera.c | 8 +++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/plat-mxc/include/mach/dma-mx1-mx2.h b/arch/arm/plat-mxc/include/mach/dma-mx1-mx2.h
> index 07be8ad..d25a65f 100644
> --- a/arch/arm/plat-mxc/include/mach/dma-mx1-mx2.h
> +++ b/arch/arm/plat-mxc/include/mach/dma-mx1-mx2.h
> @@ -31,7 +31,9 @@
> #define DMA_MODE_WRITE 1
> #define DMA_MODE_MASK 1
>
> -#define DMA_BASE IO_ADDRESS(DMA_BASE_ADDR)
> +#define MX1_DMA_REG(offset) MX1_IO_ADDRESS(MX1_DMA_BASE_ADDR + offset)

Here offset must be in parenthesis - plus is not the lowest-prio
operation, so, someone might well call MX1_DMA_REG(x << 1), which is also
almost what you do below, and this is a header...

> +#define MX1_DMA_CCR(x) MX1_DMA_REG(0x8c + ((x) << 6))
> +#define MX1_DMA_DIMR MX1_DMA_REG(0x08)

Why don't we also take comments to these registers - I promise, I won't
complain about > 80 characters;)

>
> #define IMX_DMA_MEMSIZE_32 (0 << 4)
> #define IMX_DMA_MEMSIZE_8 (1 << 4)
> diff --git a/drivers/media/video/mx1_camera.c b/drivers/media/video/mx1_camera.c
> index c167cc3..aa81acd 100644
> --- a/drivers/media/video/mx1_camera.c
> +++ b/drivers/media/video/mx1_camera.c
> @@ -48,8 +48,6 @@
> /*
> * CSI registers
> */
> -#define DMA_CCR(x) (0x8c + ((x) << 6)) /* Control Registers */
> -#define DMA_DIMR 0x08 /* Interrupt mask Register */
> #define CSICR1 0x00 /* CSI Control Register 1 */
> #define CSISR 0x08 /* CSI Status Register */
> #define CSIRXR 0x10 /* CSI RxFIFO Register */
> @@ -783,7 +781,7 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
> pcdev);
>
> imx_dma_config_channel(pcdev->dma_chan, IMX_DMA_TYPE_FIFO,
> - IMX_DMA_MEMSIZE_32, DMA_REQ_CSI_R, 0);
> + IMX_DMA_MEMSIZE_32, MX1_DMA_REQ_CSI_R, 0);
> /* burst length : 16 words = 64 bytes */
> imx_dma_config_burstlen(pcdev->dma_chan, 0);
>
> @@ -797,8 +795,8 @@ static int __init mx1_camera_probe(struct platform_device *pdev)
> set_fiq_handler(&mx1_camera_sof_fiq_start, &mx1_camera_sof_fiq_end -
> &mx1_camera_sof_fiq_start);
>
> - regs.ARM_r8 = DMA_BASE + DMA_DIMR;
> - regs.ARM_r9 = DMA_BASE + DMA_CCR(pcdev->dma_chan);
> + regs.ARM_r8 = (long)MX1_DMA_DIMR;
> + regs.ARM_r9 = (long)MX1_DMA_CCR(pcdev->dma_chan);
> regs.ARM_r10 = (long)pcdev->base + CSICR1;
> regs.ARM_fp = (long)pcdev->base + CSISR;
> regs.ARM_sp = 1 << pcdev->dma_chan;

Yes, I think, that looks good now. Just correct the above issues and I'll
ack it.

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/