On 05/12/2011 07:42 PM, Josh Wu wrote:
> This patch is to enable Atmel Image Sensor Interface (ISI) driver support. 
> - Using soc-camera framework with videobuf2 dma-contig allocator
> - Supporting video streaming of YUV packed format
> - Tested on AT91SAM9M10G45-EK with OV2640

Hi Josh,

Thansk for doing this. Overall the patch looks really good. A few
comments below.

~Ryan

> 
> Signed-off-by: Josh Wu <josh...@atmel.com>
> ---
> base on branch staging/for_v2.6.40
> 
>  arch/arm/mach-at91/include/mach/at91_isi.h |  454 ++++++++++++
>  drivers/media/video/Kconfig                |   10 +
>  drivers/media/video/Makefile               |    1 +
>  drivers/media/video/atmel-isi.c            | 1089 
> ++++++++++++++++++++++++++++
>  4 files changed, 1554 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-at91/include/mach/at91_isi.h
>  create mode 100644 drivers/media/video/atmel-isi.c
> 
> diff --git a/arch/arm/mach-at91/include/mach/at91_isi.h 
> b/arch/arm/mach-at91/include/mach/at91_isi.h
> new file mode 100644
> index 0000000..3219358
> --- /dev/null
> +++ b/arch/arm/mach-at91/include/mach/at91_isi.h
> @@ -0,0 +1,454 @@
> +/*
> + * Register definitions for the Atmel Image Sensor Interface.
> + *
> + * Copyright (C) 2011 Atmel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __AT91_ISI_H__
> +#define __AT91_ISI_H__
> +
> +#include <linux/videodev2.h>
> +#include <linux/i2c.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-common.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-chip-ident.h>
> +
> +/* ISI register offsets */
> +#define ISI_CR1                                      0x0000
> +#define ISI_CR2                                      0x0004
> +#define ISI_SR                                       0x0008
> +#define ISI_IER                                      0x000c
> +#define ISI_IDR                                      0x0010
> +#define ISI_IMR                                      0x0014
> +#define ISI_PSIZE                            0x0020
> +#define ISI_PDECF                            0x0024
> +#define ISI_PPFBD                            0x0028
> +#define ISI_CDBA                             0x002c
> +#define ISI_Y2R_SET0                         0x0030
> +#define ISI_Y2R_SET1                         0x0034
> +#define ISI_R2Y_SET0                         0x0038
> +#define ISI_R2Y_SET1                         0x003c
> +#define ISI_R2Y_SET2                         0x0040
> +
> +/* ISI_V2 register offsets */
> +#define ISI_V2_CFG1                          0x0000
> +#define ISI_V2_CFG2                          0x0004
> +#define ISI_V2_PSIZE                         0x0008
> +#define ISI_V2_PDECF                         0x000c
> +#define ISI_V2_Y2R_SET0                              0x0010
> +#define ISI_V2_Y2R_SET1                              0x0014
> +#define ISI_V2_R2Y_SET0                              0x0018
> +#define ISI_V2_R2Y_SET1                              0x001C
> +#define ISI_V2_R2Y_SET2                              0x0020
> +#define ISI_V2_CTRL                          0x0024
> +#define ISI_V2_STATUS                                0x0028
> +#define ISI_V2_INTEN                         0x002C
> +#define ISI_V2_INTDIS                                0x0030
> +#define ISI_V2_INTMASK                               0x0034
> +#define ISI_V2_DMA_CHER                              0x0038
> +#define ISI_V2_DMA_CHDR                              0x003C
> +#define ISI_V2_DMA_CHSR                              0x0040
> +#define ISI_V2_DMA_P_ADDR                    0x0044
> +#define ISI_V2_DMA_P_CTRL                    0x0048
> +#define ISI_V2_DMA_P_DSCR                    0x004C
> +#define ISI_V2_DMA_C_ADDR                    0x0050
> +#define ISI_V2_DMA_C_CTRL                    0x0054
> +#define ISI_V2_DMA_C_DSCR                    0x0058
> +
> +/* Bitfields in CR1 */
> +#define ISI_RST_OFFSET                               0
> +#define ISI_RST_SIZE                         1
> +#define ISI_DIS_OFFSET                               1
> +#define ISI_DIS_SIZE                         1
> +#define ISI_HSYNC_POL_OFFSET                 2
> +#define ISI_HSYNC_POL_SIZE                   1
> +#define ISI_VSYNC_POL_OFFSET                 3
> +#define ISI_VSYNC_POL_SIZE                   1
> +#define ISI_PIXCLK_POL_OFFSET                        4
> +#define ISI_PIXCLK_POL_SIZE                  1
> +#define ISI_EMB_SYNC_OFFSET                  6
> +#define ISI_EMB_SYNC_SIZE                    1
> +#define ISI_CRC_SYNC_OFFSET                  7
> +#define ISI_CRC_SYNC_SIZE                    1
> +#define ISI_FRATE_OFFSET                     8
> +#define ISI_FRATE_SIZE                               3
> +#define ISI_FULL_OFFSET                              12
> +#define ISI_FULL_SIZE                                1
> +#define ISI_THMASK_OFFSET                    13
> +#define ISI_THMASK_SIZE                              2
> +#define ISI_CODEC_ON_OFFSET                  15
> +#define ISI_CODEC_ON_SIZE                    1
> +#define ISI_SLD_OFFSET                               16
> +#define ISI_SLD_SIZE                         8
> +#define ISI_SFD_OFFSET                               24
> +#define ISI_SFD_SIZE                         8
> +
> +/* Bitfields in CFG1 */
> +#define ISI_V2_HSYNC_POL_OFFSET                      2
> +#define ISI_V2_HSYNC_POL_SIZE                        1
> +#define ISI_V2_VSYNC_POL_OFFSET                      3
> +#define ISI_V2_VSYNC_POL_SIZE                        1
> +#define ISI_V2_PIXCLK_POL_OFFSET             4
> +#define ISI_V2_PIXCLK_POL_SIZE                       1
> +#define ISI_V2_EMB_SYNC_OFFSET                       6
> +#define ISI_V2_EMB_SYNC_SIZE                 1
> +#define ISI_V2_CRC_SYNC_OFFSET                       7
> +#define ISI_V2_CRC_SYNC_SIZE                 1
> +#define ISI_V2_FRATE_OFFSET                  8
> +#define ISI_V2_FRATE_SIZE                    3
> +#define ISI_V2_DISCR_OFFSET                  11
> +#define ISI_V2_DISCR_SIZE                    1
> +#define ISI_V2_FULL_OFFSET                   12
> +#define ISI_V2_FULL_SIZE                     1
> +#define ISI_V2_THMASK_OFFSET                 13
> +#define ISI_V2_THMASK_SIZE                   2
> +#define ISI_V2_SLD_OFFSET                    16
> +#define ISI_V2_SLD_SIZE                              8
> +#define ISI_V2_SFD_OFFSET                    24
> +#define ISI_V2_SFD_SIZE                              8
> +
> +/* Bitfields in CR2 */
> +#define ISI_IM_VSIZE_OFFSET                  0
> +#define ISI_IM_VSIZE_SIZE                    11
> +#define ISI_GS_MODE_OFFSET                   11
> +#define ISI_GS_MODE_SIZE                     1
> +#define ISI_RGB_MODE_OFFSET                  12
> +#define ISI_RGB_MODE_SIZE                    1
> +#define ISI_GRAYSCALE_OFFSET                 13
> +#define ISI_GRAYSCALE_SIZE                   1
> +#define ISI_RGB_SWAP_OFFSET                  14
> +#define ISI_RGB_SWAP_SIZE                    1
> +#define ISI_COL_SPACE_OFFSET                 15
> +#define ISI_COL_SPACE_SIZE                   1
> +#define ISI_IM_HSIZE_OFFSET                  16
> +#define ISI_IM_HSIZE_SIZE                    11
> +#define ISI_YCC_SWAP_OFFSET                  28
> +#define ISI_YCC_SWAP_SIZE                    2
> +#define ISI_RGB_CFG_OFFSET                   30
> +#define ISI_RGB_CFG_SIZE                     2
> +
> +/* Bitfields in CFG2 */
> +#define ISI_V2_IM_VSIZE_OFFSET                       0
> +#define ISI_V2_IM_VSIZE_SIZE                 11
> +#define ISI_V2_GS_MODE_OFFSET                        11
> +#define ISI_V2_GS_MODE_SIZE                  1
> +#define ISI_V2_RGB_MODE_OFFSET                       12
> +#define ISI_V2_RGB_MODE_SIZE                 1
> +#define ISI_V2_GRAYSCALE_OFFSET                      13
> +#define ISI_V2_GRAYSCALE_SIZE                        1
> +#define ISI_V2_RGB_SWAP_OFFSET                       14
> +#define ISI_V2_RGB_SWAP_SIZE                 1
> +#define ISI_V2_COL_SPACE_OFFSET                      15
> +#define ISI_V2_COL_SPACE_SIZE                        1
> +#define ISI_V2_IM_HSIZE_OFFSET                       16
> +#define ISI_V2_IM_HSIZE_SIZE                 11
> +#define ISI_V2_YCC_SWAP_OFFSET                       28
> +#define ISI_V2_YCC_SWAP_SIZE                 2
> +#define ISI_V2_RGB_CFG_OFFSET                        30
> +#define ISI_V2_RGB_CFG_SIZE                  2
> +
> +/* Bitfields in CTRL */
> +#define ISI_V2_EN_OFFSET                     0
> +#define ISI_V2_EN_SIZE                               1
> +#define ISI_V2_DIS_OFFSET                    1
> +#define ISI_V2_DIS_SIZE                              1
> +#define ISI_V2_SRST_OFFSET                   2
> +#define ISI_V2_SRST_SIZE                     1
> +#define ISI_V2_CDC_OFFSET                    8
> +#define ISI_V2_CDC_SIZE                              1
> +
> +/* Bitfields in SR/IER/IDR/IMR */
> +#define ISI_SOF_OFFSET                               0
> +#define ISI_SOF_SIZE                         1
> +#define ISI_SOFTRST_OFFSET                   2
> +#define ISI_SOFTRST_SIZE                     1
> +#define ISI_CDC_STATUS_OFFSET                        3
> +#define ISI_CDC_STATUS_SIZE                  1
> +#define ISI_CRC_ERR_OFFSET                   4
> +#define ISI_CRC_ERR_SIZE                     1
> +#define ISI_FO_C_OVF_OFFSET                  5
> +#define ISI_FO_C_OVF_SIZE                    1
> +#define ISI_FO_P_OVF_OFFSET                  6
> +#define ISI_FO_P_OVF_SIZE                    1
> +#define ISI_FO_P_EMP_OFFSET                  7
> +#define ISI_FO_P_EMP_SIZE                    1
> +#define ISI_FO_C_EMP_OFFSET                  8
> +#define ISI_FO_C_EMP_SIZE                    1
> +#define ISI_FR_OVR_OFFSET                    9
> +#define ISI_FR_OVR_SIZE                              1
> +
> +/* Bitfields in SR/IER/IDR/IMR(ISI_V2) */
> +#define ISI_V2_ENABLE_OFFSET                 0
> +#define ISI_V2_ENABLE_SIZE                   1
> +#define ISI_V2_DIS_DONE_OFFSET                       1
> +#define ISI_V2_DIS_DONE_SIZE                 1
> +#define ISI_V2_SRST_OFFSET                   2
> +#define ISI_V2_SRST_SIZE                     1
> +#define ISI_V2_CDC_STATUS_OFFSET             8
> +#define ISI_V2_CDC_STATUS_SIZE                       1
> +#define ISI_V2_VSYNC_OFFSET                  10
> +#define ISI_V2_VSYNC_SIZE                    1
> +#define ISI_V2_PXFR_DONE_OFFSET                      16
> +#define ISI_V2_PXFR_DONE_SIZE                        1
> +#define ISI_V2_CXFR_DONE_OFFSET                      17
> +#define ISI_V2_CXFR_DONE_SIZE                        1
> +#define ISI_V2_P_OVR_OFFSET                  24
> +#define ISI_V2_P_OVR_SIZE                    1
> +#define ISI_V2_C_OVR_OFFSET                  25
> +#define ISI_V2_C_OVR_SIZE                    1
> +#define ISI_V2_CRC_ERR_OFFSET                        26
> +#define ISI_V2_CRC_ERR_SIZE                  1
> +#define ISI_V2_FR_OVR_OFFSET                 27
> +#define ISI_V2_FR_OVR_SIZE                   1
> +
> +/* Bitfields in PSIZE */
> +#define ISI_PREV_VSIZE_OFFSET                        0
> +#define ISI_PREV_VSIZE_SIZE                  10
> +#define ISI_PREV_HSIZE_OFFSET                        16
> +#define ISI_PREV_HSIZE_SIZE                  10
> +
> +/* Bitfields in PSIZE(ISI_V2) */
> +#define ISI_V2_PREV_VSIZE_OFFSET             0
> +#define ISI_V2_PREV_VSIZE_SIZE                       10
> +#define ISI_V2_PREV_HSIZE_OFFSET             16
> +#define ISI_V2_PREV_HSIZE_SIZE                       10
> +
> +/* Bitfields in PCDEF */
> +#define ISI_DEC_FACTOR_OFFSET                        0
> +#define ISI_DEC_FACTOR_SIZE                  8
> +
> +/* Bitfields in PCDEF */
> +#define ISI_V2_DEC_FACTOR_OFFSET             0
> +#define ISI_V2_DEC_FACTOR_SIZE                       8
> +
> +/* Bitfields in PPFBD */
> +#define ISI_PREV_FBD_ADDR_OFFSET             0
> +#define ISI_PREV_FBD_ADDR_SIZE                       32
> +
> +/* Bitfields in CDBA */
> +#define ISI_CODEC_DMA_ADDR_OFFSET            0
> +#define ISI_CODEC_DMA_ADDR_SIZE                      32
> +
> +/* Bitfields in DMA_C_ADDR */
> +#define ISI_V2_DMA_ADDR_OFFSET                       0
> +#define ISI_V2_DMA_ADDR_SIZE                 32
> +
> +/* Bitfields in DMA_C_CTRL & in DMA_P_CTRL */
> +#define ISI_V2_DMA_FETCH_OFFSET                      0
> +#define ISI_V2_DMA_FETCH_SIZE                        1
> +#define ISI_V2_DMA_WB_OFFSET                 1
> +#define ISI_V2_DMA_WB_SIZE                   1
> +#define ISI_V2_DMA_IEN_OFFSET                        2
> +#define ISI_V2_DMA_IEN_SIZE                  1
> +#define ISI_V2_DMA_DONE_OFFSET                       3
> +#define ISI_V2_DMA_DONE_SIZE                 1
> +
> +/* Bitfields in DMA_CHER */
> +#define ISI_V2_DMA_P_CH_EN_OFFSET            0
> +#define ISI_V2_DMA_P_CH_EN_SIZE                      1
> +#define ISI_V2_DMA_C_CH_EN_OFFSET            1
> +#define ISI_V2_DMA_C_CH_EN_SIZE                      1
> +
> +/* Bitfields in Y2R_SET0 */
> +#define ISI_Y2R_SET0_C3_OFFSET                       24
> +#define ISI_Y2R_SET0_C3_SIZE                 8
> +
> +/* Bitfields in Y2R_SET0(ISI_V2) */
> +#define ISI_V2_Y2R_SET0_C3_OFFSET            24
> +#define ISI_V2_Y2R_SET0_C3_SIZE                      8
> +
> +/* Bitfields in Y2R_SET1 */
> +#define ISI_Y2R_SET1_C4_OFFSET                       0
> +#define ISI_Y2R_SET1_C4_SIZE                 9
> +#define ISI_YOFF_OFFSET                              12
> +#define ISI_YOFF_SIZE                                1
> +#define ISI_CROFF_OFFSET                     13
> +#define ISI_CROFF_SIZE                               1
> +#define ISI_CBOFF_OFFSET                     14
> +#define ISI_CBOFF_SIZE                               1
> +
> +/* Bitfields in Y2R_SET1(ISI_V2) */
> +#define ISI_V2_Y2R_SET1_C4_OFFSET            0
> +#define ISI_V2_Y2R_SET1_C4_SIZE                      9
> +#define ISI_V2_YOFF_OFFSET                   12
> +#define ISI_V2_YOFF_SIZE                     1
> +#define ISI_V2_CROFF_OFFSET                  13
> +#define ISI_V2_CROFF_SIZE                    1
> +#define ISI_V2_CBOFF_OFFSET                  14
> +#define ISI_V2_CBOFF_SIZE                    1
> +
> +/* Bitfields in R2Y_SET0 */
> +#define ISI_C0_OFFSET                                0
> +#define ISI_C0_SIZE                          8
> +#define ISI_C1_OFFSET                                8
> +#define ISI_C1_SIZE                          8
> +#define ISI_C2_OFFSET                                16
> +#define ISI_C2_SIZE                          8
> +#define ISI_ROFF_OFFSET                              24
> +#define ISI_ROFF_SIZE                                1
> +
> +/* Bitfields in R2Y_SET0(ISI_V2) */
> +#define ISI_V2_C0_OFFSET                     0
> +#define ISI_V2_C0_SIZE                               8
> +#define ISI_V2_C1_OFFSET                     8
> +#define ISI_V2_C1_SIZE                               8
> +#define ISI_V2_C2_OFFSET                     16
> +#define ISI_V2_C2_SIZE                               8
> +#define ISI_V2_ROFF_OFFSET                   24
> +#define ISI_V2_ROFF_SIZE                     1
> +
> +/* Bitfields in R2Y_SET1 */
> +#define ISI_R2Y_SET1_C3_OFFSET                       0
> +#define ISI_R2Y_SET1_C3_SIZE                 8
> +#define ISI_R2Y_SET1_C4_OFFSET                       8
> +#define ISI_R2Y_SET1_C4_SIZE                 8
> +#define ISI_C5_OFFSET                                16
> +#define ISI_C5_SIZE                          8
> +#define ISI_GOFF_OFFSET                              24
> +#define ISI_GOFF_SIZE                                1
> +
> +/* Bitfields in R2Y_SET1(ISI_V2) */
> +#define ISI_V2_R2Y_SET1_C3_OFFSET            0
> +#define ISI_V2_R2Y_SET1_C3_SIZE                      8
> +#define ISI_V2_R2Y_SET1_C4_OFFSET            8
> +#define ISI_V2_R2Y_SET1_C4_SIZE                      8
> +#define ISI_V2_C5_OFFSET                     16
> +#define ISI_V2_C5_SIZE                               8
> +#define ISI_V2_GOFF_OFFSET                   24
> +#define ISI_V2_GOFF_SIZE                     1
> +
> +/* Bitfields in R2Y_SET2 */
> +#define ISI_C6_OFFSET                                0
> +#define ISI_C6_SIZE                          8
> +#define ISI_C7_OFFSET                                8
> +#define ISI_C7_SIZE                          8
> +#define ISI_C8_OFFSET                                16
> +#define ISI_C8_SIZE                          8
> +#define ISI_BOFF_OFFSET                              24
> +#define ISI_BOFF_SIZE                                1
> +
> +/* Bitfields in R2Y_SET2(ISI_V2) */
> +#define ISI_V2_C6_OFFSET                     0
> +#define ISI_V2_C6_SIZE                               8
> +#define ISI_V2_C7_OFFSET                     8
> +#define ISI_V2_C7_SIZE                               8
> +#define ISI_V2_C8_OFFSET                     16
> +#define ISI_V2_C8_SIZE                               8
> +#define ISI_V2_BOFF_OFFSET                   24
> +#define ISI_V2_BOFF_SIZE                     1
> +
> +/* Constants for FRATE */
> +#define ISI_FRATE_CAPTURE_ALL                        0
> +
> +/* Constants for FRATE(ISI_V2) */
> +#define ISI_V2_FRATE_CAPTURE_ALL             0
> +
> +/* Constants for YCC_SWAP */
> +#define ISI_YCC_SWAP_DEFAULT                 0
> +#define ISI_YCC_SWAP_MODE_1                  1
> +#define ISI_YCC_SWAP_MODE_2                  2
> +#define ISI_YCC_SWAP_MODE_3                  3
> +
> +/* Constants for YCC_SWAP(ISI_V2) */
> +#define ISI_V2_YCC_SWAP_DEFAULT                      0
> +#define ISI_V2_YCC_SWAP_MODE_1                       1
> +#define ISI_V2_YCC_SWAP_MODE_2                       2
> +#define ISI_V2_YCC_SWAP_MODE_3                       3
> +
> +/* Constants for RGB_CFG */
> +#define ISI_RGB_CFG_DEFAULT                  0
> +#define ISI_RGB_CFG_MODE_1                   1
> +#define ISI_RGB_CFG_MODE_2                   2
> +#define ISI_RGB_CFG_MODE_3                   3
> +
> +/* Constants for RGB_CFG(ISI_V2) */
> +#define ISI_V2_RGB_CFG_DEFAULT                       0
> +#define ISI_V2_RGB_CFG_MODE_1                        1
> +#define ISI_V2_RGB_CFG_MODE_2                        2
> +#define ISI_V2_RGB_CFG_MODE_3                        3
> +
> +/* Bit manipulation macros */
> +#define ISI_BIT(name)                                        \
> +     (1 << ISI_##name##_OFFSET)
> +#define ISI_BF(name, value)                          \
> +     (((value) & ((1 << ISI_##name##_SIZE) - 1))     \
> +      << ISI_##name##_OFFSET)
> +#define ISI_BFEXT(name, value)                               \
> +     (((value) >> ISI_##name##_OFFSET)               \
> +      & ((1 << ISI_##name##_SIZE) - 1))
> +#define ISI_BFINS(name, value, old)                  \
> +     (((old) & ~(((1 << ISI_##name##_SIZE) - 1)      \
> +                 << ISI_##name##_OFFSET))\
> +      | ISI_BF(name, value))

I really dislike this kind of register access magic. Not sure how others
feel about it. These macros are really ugly.

> +/* Register access macros */
> +#define isi_readl(port, reg)                         \
> +     __raw_readl((port)->regs + ISI_##reg)
> +#define isi_writel(port, reg, value)                 \
> +     __raw_writel((value), (port)->regs + ISI_##reg)

If the token pasting stuff gets dropped then these can be static inline
functions which is preferred.

> +
> +#define ATMEL_V4L2_VID_FLAGS (V4L2_CAP_VIDEO_OUTPUT)
> +
> +struct atmel_isi;
> +
> +enum atmel_isi_pixfmt {
> +     ATMEL_ISI_PIXFMT_GREY,          /* Greyscale */
> +     ATMEL_ISI_PIXFMT_CbYCrY,
> +     ATMEL_ISI_PIXFMT_CrYCbY,
> +     ATMEL_ISI_PIXFMT_YCbYCr,
> +     ATMEL_ISI_PIXFMT_YCrYCb,
> +     ATMEL_ISI_PIXFMT_RGB24,
> +     ATMEL_ISI_PIXFMT_BGR24,
> +     ATMEL_ISI_PIXFMT_RGB16,
> +     ATMEL_ISI_PIXFMT_BGR16,
> +     ATMEL_ISI_PIXFMT_GRB16,         /* G[2:0] R[4:0]/B[4:0] G[5:3] */
> +     ATMEL_ISI_PIXFMT_GBR16,         /* G[2:0] B[4:0]/R[4:0] G[5:3] */
> +     ATMEL_ISI_PIXFMT_RGB24_REV,
> +     ATMEL_ISI_PIXFMT_BGR24_REV,
> +     ATMEL_ISI_PIXFMT_RGB16_REV,
> +     ATMEL_ISI_PIXFMT_BGR16_REV,
> +     ATMEL_ISI_PIXFMT_GRB16_REV,     /* G[2:0] R[4:0]/B[4:0] G[5:3] */
> +     ATMEL_ISI_PIXFMT_GBR16_REV,     /* G[2:0] B[4:0]/R[4:0] G[5:3] */
> +};
> +
> +struct isi_platform_data {
> +     u8 has_emb_sync;
> +     u8 emb_crc_sync;
> +     u8 hsync_act_low;
> +     u8 vsync_act_low;
> +     u8 pclk_act_falling;
> +     u8 isi_full_mode;
> +#define ISI_HSYNC_ACT_LOW    0x01
> +#define ISI_VSYNC_ACT_LOW    0x02
> +#define ISI_PXCLK_ACT_FALLING        0x04
> +#define ISI_EMB_SYNC         0x08
> +#define ISI_CRC_SYNC         0x10
> +#define ISI_FULL             0x20
> +#define ISI_DATAWIDTH_8              0x40
> +#define ISI_DATAWIDTH_10     0x80
> +     u32 flags;
> +     u8 gs_mode;
> +#define ISI_GS_2PIX_PER_WORD 0x00
> +#define ISI_GS_1PIX_PER_WORD 0x01
> +     u8 pixfmt;
> +     u8 sfd;
> +     u8 sld;
> +     u8 thmask;
> +#define ISI_BURST_4_8_16     0x00
> +#define ISI_BURST_8_16               0x01
> +#define ISI_BURST_16         0x02
> +     u8 frate;
> +#define ISI_FRATE_DIV_2              0x01
> +#define ISI_FRATE_DIV_3              0x02
> +#define ISI_FRATE_DIV_4              0x03
> +#define ISI_FRATE_DIV_5              0x04
> +#define ISI_FRATE_DIV_6              0x05
> +#define ISI_FRATE_DIV_7              0x06
> +#define ISI_FRATE_DIV_8              0x07
> +};

Might need some comments in this structure so that board file writers
know what each of the fields are for.

> +
> +#endif /* __AT91_ISI_H__ */
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index d61414e..eae6005 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -80,6 +80,16 @@ menuconfig VIDEO_CAPTURE_DRIVERS
>         Some of those devices also supports FM radio.
>  
>  if VIDEO_CAPTURE_DRIVERS && VIDEO_V4L2
> +config VIDEO_ATMEL_ISI
> +     tristate "ATMEL Image Sensor Interface (ISI) support"
> +     depends on VIDEO_DEV && SOC_CAMERA

Depends on AT91/AVR32?

> +     select VIDEOBUF2_DMA_CONTIG
> +     default n
> +     ---help---
> +       This module makes the ATMEL Image Sensor Interface available
> +       as a v4l2 device.
> +       Say Y here to enable selecting the Image Sensor Interface.
> +       When in doubt, say N.
>  
>  config VIDEO_ADV_DEBUG
>       bool "Enable advanced debug functionality"
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index a10e4c3..f734a65 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -166,6 +166,7 @@ obj-$(CONFIG_VIDEO_SH_MOBILE_CSI2)        += 
> sh_mobile_csi2.o
>  obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)    += sh_mobile_ceu_camera.o
>  obj-$(CONFIG_VIDEO_OMAP1)            += omap1_camera.o
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_FIMC)         += s5p-fimc/
> +obj-$(CONFIG_VIDEO_ATMEL_ISI)                += atmel-isi.o
>  
>  obj-$(CONFIG_ARCH_DAVINCI)           += davinci/
>  
> diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
> new file mode 100644
> index 0000000..33d0b83
> --- /dev/null
> +++ b/drivers/media/video/atmel-isi.c
> @@ -0,0 +1,1089 @@
> +/*
> + * Copyright (c) 2011 Atmel Corporation
> + *
> + * Based on previous work by Lars Haring and Sedji Gaouaou
> + *
> + * Based on the bttv driver for Bt848 with respective copyright holders
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/version.h>
> +#include <linux/kfifo.h>
> +
> +#include <mach/board.h>
> +#include <mach/cpu.h>
> +#include <mach/at91_isi.h>
> +
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/soc_camera.h>
> +#include <media/soc_mediabus.h>
> +
> +#define ATMEL_ISI_VERSION    KERNEL_VERSION(1, 0, 0)

Do we really need this version?

> +#define MAX_BUFFER_NUMS              32
> +#define MAX_SUPPORT_WIDTH    2048
> +#define MAX_SUPPORT_HEIGHT   2048
> +
> +static unsigned int vid_limit = 16;

This never gets changed so it can become a const/define. The value is
MB, which is not clear from the name, and it gets multiplied out to
bytes in its only usage, so maybe:

#define VID_LIMIT_BYTES (16 * 1024 * 1024)

> +
> +enum isi_buffer_state {
> +     ISI_BUF_NEEDS_INIT,
> +     ISI_BUF_PREPARED,
> +};

Aren't there v4l2 constants for this already?

        VIDEOBUF_NEEDS_INIT,
        VIDEOBUF_PREPARED,

Just reuse those.

> +
> +/* Single frame capturing states */
> +enum {
> +     STATE_IDLE = 0,
> +     STATE_CAPTURE_READY,
> +     STATE_CAPTURE_WAIT_SOF,
> +     STATE_CAPTURE_IN_PROGRESS,
> +     STATE_CAPTURE_DONE,
> +     STATE_CAPTURE_ERROR,
> +};
> +
> +/* Frame buffer descriptor
> + *  Used by the ISI module as a linked list for the DMA controller.
> + */
> +struct fbd {
> +     /* Physical address of the frame buffer */
> +     u32 fb_address;
> +#if defined(CONFIG_ARCH_AT91SAM9G45) ||\
> +     defined(CONFIG_ARCH_AT91SAM9X5)
> +     /* DMA Control Register(only in HISI2) */
> +     u32 dma_ctrl;
> +#endif

I wouldn't bother with this ifdef. The memory cost is pretty
insignificant and it makes the code easier to read without it. It also
means you don't need to patch it up whenever a new at91 platform with
isi dma support appears.

> +     /* Physical address of the next fbd */
> +     u32 next_fbd_address;
> +};
> +
> +#if defined(CONFIG_ARCH_AT91SAM9G45) ||\
> +     defined(CONFIG_ARCH_AT91SAM9X5)
> +static void set_dma_ctrl(struct fbd *fb_desc, u32 ctrl)
> +{
> +     fb_desc->dma_ctrl = ctrl;
> +}
> +#else
> +static void set_dma_ctrl(struct fbd *fb_desc, u32 ctrl) { }
> +#endif

Same here, get rid of the ifdefs.

> +/* Frame buffer data
> + */
> +struct frame_buffer {
> +     struct vb2_buffer vb;
> +     struct fbd fb_desc;
> +     /* Frame number of the frame  */
> +     unsigned long sequence;
> +
> +     enum isi_buffer_state dma_desc_status;
> +     struct list_head list;
> +};
> +
> +struct atmel_isi {
> +     /* ISI module spin lock. Protects against concurrent access of variables
> +      * that are shared with the ISR */
> +     spinlock_t                      lock;
> +     void __iomem                    *regs;
> +
> +     /*  If set ISI is in still capture mode */
> +     int                             still_capture;
> +     int                             sequence;
> +     /* State of the ISI module in capturing mode */
> +     int                             state;
> +
> +     /* Capture/streaming wait queue for waiting for SOF */
> +     wait_queue_head_t               capture_wq;
> +
> +     struct v4l2_device              v4l2_dev;
> +
> +     struct vb2_alloc_ctx            *alloc_ctx;
> +
> +     struct clk                      *pclk;
> +     struct platform_device          *pdev;
> +     unsigned int                    irq;
> +
> +     struct isi_platform_data        *pdata;
> +     unsigned long                   platform_flags;
> +
> +     struct list_head                video_buffer_list;
> +     struct frame_buffer             *active;
> +
> +     struct soc_camera_device        *icd;
> +     struct soc_camera_host          soc_host;
> +};
> +
> +static int configure_geometry(struct atmel_isi *isi, u32 width,
> +                     u32 height, enum v4l2_mbus_pixelcode code)
> +{
> +     u32 cfg2, cr, ctrl;
> +
> +     cr = 0;
> +     switch (code) {
> +     /* YUV, including grey */
> +     case V4L2_MBUS_FMT_Y8_1X8:
> +             cr = ISI_BIT(GRAYSCALE);
> +             break;
> +     case V4L2_MBUS_FMT_UYVY8_2X8:
> +             cr = ISI_BF(V2_YCC_SWAP, 3);
> +             break;
> +     case V4L2_MBUS_FMT_VYUY8_2X8:
> +             cr = ISI_BF(V2_YCC_SWAP, 2);
> +             break;
> +     case V4L2_MBUS_FMT_YUYV8_2X8:
> +             cr = ISI_BF(V2_YCC_SWAP, 1);
> +             break;
> +     case V4L2_MBUS_FMT_YVYU8_2X8:
> +             cr = ISI_BF(V2_YCC_SWAP, 0);
> +             break;
> +     /* RGB, TODO */
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     ctrl = ISI_BIT(DIS);
> +     isi_writel(isi, V2_CTRL, ctrl);
> +     /* Check if module properly disable */
> +     while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_DIS_DONE))
> +             cpu_relax();
> +
> +     cfg2 = isi_readl(isi, V2_CFG2);
> +     cfg2 |= cr;
> +     cfg2 = ISI_BFINS(V2_IM_VSIZE, height - 1, cfg2);
> +     cfg2 = ISI_BFINS(V2_IM_HSIZE, width - 1, cfg2);
> +     isi_writel(isi, V2_CFG2, cfg2);
> +
> +     return 0;
> +}
> +
> +static irqreturn_t atmel_isi_handle_streaming(struct atmel_isi *isi)
> +{
> +     if (isi->active) {
> +             struct vb2_buffer *vb = &isi->active->vb;
> +             struct frame_buffer *buf = isi->active;
> +
> +             list_del_init(&buf->list);
> +             do_gettimeofday(&vb->v4l2_buf.timestamp);
> +             vb->v4l2_buf.sequence = isi->sequence++;
> +             vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> +     }
> +
> +     if (list_empty(&isi->video_buffer_list)) {
> +             isi->active = NULL;
> +     } else {
> +             /* start next dma frame. */
> +             isi->active = list_entry(isi->video_buffer_list.next,
> +                                     struct frame_buffer, list);
> +             isi_writel(isi, V2_DMA_C_DSCR, (__pa(&(isi->active->fb_desc))));
> +             isi_writel(isi, V2_DMA_C_CTRL,
> +                     ISI_BIT(V2_DMA_FETCH) | ISI_BIT(V2_DMA_DONE));
> +             isi_writel(isi, V2_DMA_CHER, ISI_BIT(V2_DMA_C_CH_EN));
> +     }
> +     return IRQ_HANDLED;
> +}
> +
> +/* ISI interrupt service routine */
> +static irqreturn_t isi_interrupt(int irq, void *dev_id)
> +{
> +     struct atmel_isi *isi = dev_id;
> +     u32 status, mask, pending;
> +     irqreturn_t ret = IRQ_NONE;
> +
> +     spin_lock(&isi->lock);
> +
> +     status = isi_readl(isi, V2_STATUS);
> +     mask = isi_readl(isi, V2_INTMASK);
> +     pending = status & mask;
> +
> +     if (!isi->still_capture) {
> +             if (pending & (ISI_BIT(V2_VSYNC))) {
> +                     switch (isi->state) {
> +                     case STATE_IDLE:
> +                             isi->state = STATE_CAPTURE_READY;
> +                             wake_up_interruptible(&isi->capture_wq);
> +                             break;
> +                     }
> +             } else if (likely(pending & (ISI_BIT(V2_CXFR_DONE)))) {
> +                     ret = atmel_isi_handle_streaming(isi);
> +             }
> +     } else {
> +             while (pending) {
> +                     if (pending & (ISI_BIT(V2_C_OVR) | ISI_BIT(V2_FR_OVR)))
> +                             printk(KERN_ERR "Overrun (status = 0x%x)\n",
> +                                     status);

dev_err(isi->v4l2_dev.dev, "Overrun...");

> +                     else if (pending &
> +                             (ISI_BIT(V2_VSYNC) | ISI_BIT(V2_CDC))) {
> +                             switch (isi->state) {
> +                             case STATE_IDLE:
> +                                     isi->state = STATE_CAPTURE_READY;
> +                                     wake_up_interruptible(&isi->capture_wq);
> +                                     break;
> +                             case STATE_CAPTURE_READY:
> +                                     break;
> +                             case STATE_CAPTURE_WAIT_SOF:
> +                                     isi->state = STATE_CAPTURE_IN_PROGRESS;
> +                                     break;
> +                             }
> +                     }
> +
> +                     if (likely(pending & (ISI_BIT(V2_CXFR_DONE))))
> +                             ret = atmel_isi_handle_streaming(isi);
> +
> +                     status = isi_readl(isi, V2_STATUS);
> +                     mask = isi_readl(isi, V2_INTMASK);
> +                     pending = status & mask;
> +                     ret = IRQ_HANDLED;
> +             }
> +     }
> +     spin_unlock(&isi->lock);
> +
> +     return ret;
> +}
> +
> +static int atmel_isi_init(struct atmel_isi *isi)
> +{
> +     /*
> +      * The reset will only succeed if we have a
> +      * pixel clock from the camera.
> +      */
> +     isi_writel(isi, V2_CTRL, ISI_BIT(V2_SRST));
> +     isi_writel(isi, V2_INTDIS, ~0UL);

Don't you need to wait for the reset bit to clear? Other implementations
I have used of the Atmel ISI driver do a wait_for_completition and have
the interrupt handler set a reset_complete flag.

> +     return 0;
> +}
> +
> +/* ------------------------------------------------------------------
> +     Videobuf operations
> +   ------------------------------------------------------------------*/
> +static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
> +                             unsigned int *nplanes, unsigned long sizes[],
> +                             void *alloc_ctxs[])
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *dev = ici->priv;
> +     unsigned long size;
> +
> +     int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> +                                             icd->current_fmt->host_fmt);
> +
> +     if (bytes_per_line < 0)
> +             return bytes_per_line;
> +
> +     size = bytes_per_line * icd->user_height;
> +
> +     if (0 == *nbuffers)

        if (*nbuffers == 0)

is the more commonly preferred style I believe.

> +             *nbuffers = MAX_BUFFER_NUMS;
> +     if (*nbuffers > MAX_BUFFER_NUMS)
> +             *nbuffers = MAX_BUFFER_NUMS;
> +
> +     while (size * *nbuffers > vid_limit * 1024 * 1024)
> +             (*nbuffers)--;
> +
> +     *nplanes = 1;
> +     sizes[0] = size;
> +     alloc_ctxs[0] = dev->alloc_ctx;
> +
> +     dev->sequence = 0;
> +     dev->active = NULL;
> +
> +     dev_dbg(icd->dev.parent, "%s, count=%d, size=%ld\n", __func__,
> +             *nbuffers, size);
> +
> +     return 0;
> +}
> +
> +static int buffer_init(struct vb2_buffer *vb)
> +{
> +     struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb);
> +
> +     buf->dma_desc_status = ISI_BUF_NEEDS_INIT;
> +     INIT_LIST_HEAD(&buf->list);
> +
> +     return 0;
> +}
> +
> +static int buffer_prepare(struct vb2_buffer *vb)
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
> +     struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb);
> +     unsigned long size;
> +     int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
> +                                             icd->current_fmt->host_fmt);
> +
> +     if (bytes_per_line < 0)
> +             return bytes_per_line;
> +
> +     size = bytes_per_line * icd->user_height;
> +
> +     if (vb2_plane_size(vb, 0) < size) {
> +             dev_err(icd->dev.parent, "%s data will not fit into plane (%lu 
> < %lu)\n",
> +                             __func__, vb2_plane_size(vb, 0), size);
> +             return -EINVAL;
> +     }
> +
> +     vb2_set_plane_payload(&buf->vb, 0, size);
> +
> +     if (buf->dma_desc_status == ISI_BUF_NEEDS_INIT) {
> +             buf->dma_desc_status = ISI_BUF_PREPARED;
> +
> +             /* initialize the dma descriptor */
> +             buf->fb_desc.fb_address = vb2_dma_contig_plane_paddr(vb, 0);
> +             buf->fb_desc.next_fbd_address = 0;
> +             set_dma_ctrl(&buf->fb_desc, ISI_BIT(V2_DMA_WB));
> +     }
> +
> +     return 0;
> +}
> +
> +static int buffer_finish(struct vb2_buffer *vb)
> +{
> +     return 0;
> +}

Does soc-camera support having NULL function callbacks for functions
that are not required?

> +
> +static void buffer_cleanup(struct vb2_buffer *vb)
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb);
> +     unsigned long flags = 0;
> +
> +     spin_lock_irqsave(&isi->lock, flags);
> +     buf->dma_desc_status = ISI_BUF_NEEDS_INIT;
> +     spin_unlock_irqrestore(&isi->lock, flags);
> +}
> +
> +static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
> +{
> +     u32 ctrl, cfg1;
> +     ctrl = isi_readl(isi, V2_CTRL);
> +     cfg1 = isi_readl(isi, V2_CFG1);
> +     /* Enable irq: cxfr for the codec path, pxfr for the preview path */
> +     isi_writel(isi, V2_INTEN,
> +                     ISI_BIT(V2_CXFR_DONE) | ISI_BIT(V2_PXFR_DONE));
> +
> +     /* Enable codec path */
> +     ctrl |= ISI_BIT(V2_CDC);
> +     /* Check if already in a frame */
> +     while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_CDC))
> +             cpu_relax();
> +
> +     /* Write the address of the first frame buffer in the C_ADDR reg
> +     * write the address of the first descriptor(link list of buffer)
> +     * in the C_DSCR reg, and enable dma channel.
> +     */
> +     isi_writel(isi, V2_DMA_C_DSCR, (__pa(&(buffer->fb_desc))));
> +     isi_writel(isi, V2_DMA_C_CTRL,
> +                     ISI_BIT(V2_DMA_FETCH) | ISI_BIT(V2_DMA_DONE));
> +     isi_writel(isi, V2_DMA_CHER, ISI_BIT(V2_DMA_C_CH_EN));
> +
> +     /* Enable linked list */
> +     cfg1 |= ISI_BF(V2_FRATE, isi->pdata->frate) | ISI_BIT(V2_DISCR);
> +
> +     /* Enable ISI module*/
> +     ctrl |= ISI_BIT(V2_ENABLE);
> +     isi_writel(isi, V2_CTRL, ctrl);
> +     isi_writel(isi, V2_CFG1, cfg1);
> +}
> +
> +static void buffer_queue(struct vb2_buffer *vb)
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     struct frame_buffer *buf = container_of(vb, struct frame_buffer, vb);
> +     unsigned long flags = 0;
> +
> +     spin_lock_irqsave(&isi->lock, flags);
> +     list_add_tail(&buf->list, &isi->video_buffer_list);
> +
> +     if (isi->active == NULL) {
> +             isi->active = buf;
> +             start_dma(isi, buf);
> +     }
> +     spin_unlock_irqrestore(&isi->lock, flags);
> +}
> +
> +static int start_streaming(struct vb2_queue *vq)
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +
> +     u32 sr = 0;
> +     int ret;
> +
> +     spin_lock_irq(&isi->lock);
> +     isi->state = STATE_IDLE;
> +
> +     /* Clear any pending SOF interrupt */
> +     sr = isi_readl(isi, V2_STATUS);
> +     /* Enable VSYNC interrupt for SOF */
> +     isi_writel(isi, V2_INTEN, ISI_BIT(V2_VSYNC));
> +     isi_writel(isi, V2_CTRL, isi_readl(isi, V2_CTRL) | ISI_BIT(V2_EN));
> +
> +     spin_unlock_irq(&isi->lock);
> +     dev_dbg(icd->dev.parent, "isi: waiting for SOF\n");
> +     ret = wait_event_interruptible(isi->capture_wq,
> +                                    isi->state != STATE_IDLE);
> +     if (ret)
> +             return ret;
> +
> +     if (isi->state != STATE_CAPTURE_READY)
> +             return -EIO;
> +
> +     spin_lock_irq(&isi->lock);
> +     isi->state = STATE_CAPTURE_WAIT_SOF;
> +     isi_writel(isi, V2_INTDIS, ISI_BIT(V2_VSYNC));
> +     spin_unlock_irq(&isi->lock);
> +
> +     return 0;
> +}
> +
> +/* abort streaming and wait for last buffer */
> +static int stop_streaming(struct vb2_queue *vq)
> +{
> +     struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +
> +     spin_lock_irq(&isi->lock);
> +     isi->still_capture = 0;
> +     isi->active = NULL;
> +
> +     while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_CDC))
> +             cpu_relax();
> +
> +     /* Disble codec path */
> +     isi_writel(isi, V2_CTRL, isi_readl(isi, V2_CTRL) & (~ISI_BIT(V2_CDC)));
> +     /* Disable interrupts */
> +     isi_writel(isi, V2_INTDIS,
> +                     ISI_BIT(V2_CXFR_DONE) | ISI_BIT(V2_PXFR_DONE));
> +     /* Disable ISI module*/
> +     isi_writel(isi, V2_CTRL, isi_readl(isi, V2_CTRL) | ISI_BIT(V2_DIS));
> +
> +     /* Release all active buffers */
> +     while (!list_empty(&isi->video_buffer_list)) {
> +             struct frame_buffer *buf;
> +             buf = list_entry(isi->video_buffer_list.next,
> +                             struct frame_buffer, list);
> +             list_del_init(&buf->list);
> +             vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
> +     }
> +
> +     spin_unlock_irq(&isi->lock);
> +     return 0;
> +}
> +
> +static struct vb2_ops isi_video_qops = {
> +     .queue_setup            = queue_setup,
> +     .buf_init               = buffer_init,
> +     .buf_prepare            = buffer_prepare,
> +     .buf_finish             = buffer_finish,
> +     .buf_cleanup            = buffer_cleanup,
> +     .buf_queue              = buffer_queue,
> +     .start_streaming        = start_streaming,
> +     .stop_streaming         = stop_streaming,
> +     .wait_prepare           = soc_camera_unlock,
> +     .wait_finish            = soc_camera_lock,
> +};
> +
> +/* ------------------------------------------------------------------
> +     SOC camera operations for the device
> +   ------------------------------------------------------------------*/
> +static int isi_camera_init_videobuf(struct vb2_queue *q,
> +                                  struct soc_camera_device *icd)
> +{
> +     q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +     q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;
> +     q->drv_priv = icd;
> +     q->buf_struct_size = sizeof(struct frame_buffer);
> +     q->ops = &isi_video_qops;
> +     q->mem_ops = &vb2_dma_contig_memops;
> +
> +     return vb2_queue_init(q);
> +}
> +static inline void stride_align(u32 *width)
> +{
> +     if (((*width + 7) &  ~7) < 4096)
> +             *width = (*width + 7) &  ~7;
> +     else
> +             *width = *width &  ~7;

This looks like something that may already have a function to do it?

> +}
> +
> +static int isi_camera_set_fmt(struct soc_camera_device *icd,
> +                           struct v4l2_format *f)
> +{
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> +     const struct soc_camera_format_xlate *xlate;
> +     struct v4l2_pix_format *pix = &f->fmt.pix;
> +     struct v4l2_mbus_framefmt mf;
> +     int ret;
> +
> +     xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
> +     if (!xlate) {
> +             dev_warn(icd->dev.parent, "Format %x not found\n",
> +                      pix->pixelformat);
> +             return -EINVAL;
> +     }
> +
> +     stride_align(&pix->width);
> +     dev_dbg(icd->dev.parent, "plan to set format %dx%d\n",
> +                     pix->width, pix->height);
> +
> +     mf.width        = pix->width;
> +     mf.height       = pix->height;
> +     mf.field        = pix->field;
> +     mf.colorspace   = pix->colorspace;
> +     mf.code         = xlate->code;
> +
> +     ret = v4l2_subdev_call(sd, video, s_mbus_fmt, &mf);
> +     if (ret < 0)
> +             return ret;
> +
> +     if (mf.code != xlate->code)
> +             return -EINVAL;
> +
> +     ret = configure_geometry(isi, pix->width, pix->height, xlate->code);
> +     if (ret < 0)
> +             return ret;
> +
> +     pix->width              = mf.width;
> +     pix->height             = mf.height;
> +     pix->field              = mf.field;
> +     pix->colorspace         = mf.colorspace;
> +     icd->current_fmt        = xlate;
> +
> +     pix->bytesperline = soc_mbus_bytes_per_line(pix->width,
> +                                                 xlate->host_fmt);
> +     if (pix->bytesperline < 0)
> +             return pix->bytesperline;
> +     pix->sizeimage = pix->height * pix->bytesperline;
> +
> +     dev_dbg(icd->dev.parent, "Finally set format %dx%d, sizeimage = %d\n",
> +             pix->width, pix->height, pix->sizeimage);
> +
> +     return ret;
> +}
> +
> +static int isi_camera_try_fmt(struct soc_camera_device *icd,
> +                           struct v4l2_format *f)
> +{
> +     struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> +     const struct soc_camera_format_xlate *xlate;
> +     struct v4l2_pix_format *pix = &f->fmt.pix;
> +     struct v4l2_mbus_framefmt mf;
> +     __u32 pixfmt = pix->pixelformat;

u32 inside the kernel, __u32 is for code which is exposed to userspace
(i.e. API headers).

> +     int ret;
> +
> +     xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
> +     if (pixfmt && !xlate) {
> +             dev_warn(icd->dev.parent, "Format %x not found\n", pixfmt);
> +             return -EINVAL;
> +     }
> +
> +     /* limit to Atmel ISI hardware capabilities */
> +     if (pix->height > MAX_SUPPORT_HEIGHT)
> +             pix->height = MAX_SUPPORT_HEIGHT;
> +     if (pix->width > MAX_SUPPORT_WIDTH)
> +             pix->width = MAX_SUPPORT_WIDTH;
> +
> +     pix->bytesperline = soc_mbus_bytes_per_line(pix->width,
> +                                                 xlate->host_fmt);
> +     if (pix->bytesperline < 0)
> +             return pix->bytesperline;
> +     pix->sizeimage = pix->height * pix->bytesperline;
> +
> +     /* limit to sensor capabilities */
> +     mf.width        = pix->width;
> +     mf.height       = pix->height;
> +     mf.field        = pix->field;
> +     mf.colorspace   = pix->colorspace;
> +     mf.code         = xlate->code;
> +
> +     ret = v4l2_subdev_call(sd, video, try_mbus_fmt, &mf);
> +     if (ret < 0)
> +             return ret;
> +
> +     pix->width      = mf.width;
> +     pix->height     = mf.height;
> +     pix->colorspace = mf.colorspace;
> +
> +     switch (mf.field) {
> +     case V4L2_FIELD_ANY:
> +             pix->field = V4L2_FIELD_NONE;
> +             break;
> +     case V4L2_FIELD_NONE:
> +             break;
> +     default:
> +             dev_err(icd->dev.parent, "Field type %d unsupported.\n",
> +                     mf.field);
> +             ret = -EINVAL;
> +     }
> +
> +     return ret;
> +}
> +
> +static const struct soc_mbus_pixelfmt isi_camera_formats[] = {
> +     {
> +             .fourcc                 = V4L2_PIX_FMT_YUYV,
> +             .name                   = "Packed YUV422 16 bit",
> +             .bits_per_sample        = 8,
> +             .packing                = SOC_MBUS_PACKING_2X8_PADHI,
> +             .order                  = SOC_MBUS_ORDER_LE,
> +     },
> +};
> +
> +/* This will be corrected as we get more formats */
> +static bool isi_camera_packing_supported(const struct soc_mbus_pixelfmt *fmt)
> +{
> +     return  fmt->packing == SOC_MBUS_PACKING_NONE ||
> +             (fmt->bits_per_sample == 8 &&
> +              fmt->packing == SOC_MBUS_PACKING_2X8_PADHI) ||
> +             (fmt->bits_per_sample > 8 &&
> +              fmt->packing == SOC_MBUS_PACKING_EXTEND16);
> +}
> +
> +static int test_platform_param(struct atmel_isi *isi,
> +                            unsigned char buswidth, unsigned long *flags)
> +{
> +     /*
> +      * Platform specified synchronization and pixel clock polarities are
> +      * only a recommendation and are only used during probing. Atmel ISI
> +      * camera interface only works in master mode, i.e., uses HSYNC and
> +      * VSYNC signals from the sensor
> +      */
> +     *flags = SOCAM_MASTER |
> +             SOCAM_HSYNC_ACTIVE_HIGH |
> +             SOCAM_HSYNC_ACTIVE_LOW |
> +             SOCAM_VSYNC_ACTIVE_HIGH |
> +             SOCAM_VSYNC_ACTIVE_LOW |
> +             SOCAM_PCLK_SAMPLE_RISING |
> +             SOCAM_PCLK_SAMPLE_FALLING |
> +             SOCAM_DATA_ACTIVE_HIGH;
> +
> +     /* If requested data width is supported by the platform, use it */
> +     switch (buswidth) {
> +     case 10:
> +             if (!(isi->platform_flags & ISI_DATAWIDTH_10))
> +                     return -EINVAL;
> +             *flags |= SOCAM_DATAWIDTH_10;
> +             break;

If you have specified the platform data width as 10 bits can you not do
8 bit transfers on it?

> +     case 8:
> +             if (!(isi->platform_flags & ISI_DATAWIDTH_8))
> +                     return -EINVAL;
> +             *flags |= SOCAM_DATAWIDTH_8;
> +             break;
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     return 0;
> +}
> +
> +static int isi_camera_try_bus_param(struct soc_camera_device *icd,
> +                                 unsigned char buswidth)
> +{
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     unsigned long bus_flags, camera_flags;
> +     int ret = test_platform_param(isi, buswidth, &bus_flags);
> +
> +     if (ret < 0)
> +             return ret;
> +
> +     camera_flags = icd->ops->query_bus_param(icd);
> +
> +     ret = soc_camera_bus_param_compatible(camera_flags, bus_flags);
> +     if (!ret)
> +             return -EINVAL;
> +     return 0;
> +}
> +
> +
> +static int isi_camera_get_formats(struct soc_camera_device *icd,
> +                               unsigned int idx,
> +                               struct soc_camera_format_xlate *xlate)
> +{
> +     struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> +     struct device *dev = icd->dev.parent;
> +     int formats = 0, ret;
> +     /* sensor format */
> +     enum v4l2_mbus_pixelcode code;
> +     /* soc camera host format */
> +     const struct soc_mbus_pixelfmt *fmt;
> +
> +     ret = v4l2_subdev_call(sd, video, enum_mbus_fmt, idx, &code);
> +     if (ret < 0)
> +             /* No more formats */
> +             return 0;
> +
> +     fmt = soc_mbus_get_fmtdesc(code);
> +     if (!fmt) {
> +             dev_err(icd->dev.parent,
> +                     "Invalid format code #%u: %d\n", idx, code);
> +             return 0;
> +     }
> +
> +     /* This also checks support for the requested bits-per-sample */
> +     ret = isi_camera_try_bus_param(icd, fmt->bits_per_sample);
> +     if (ret < 0) {
> +             dev_err(icd->dev.parent,
> +                     "Fail to try the bus parameters.\n");
> +             return 0;
> +     }
> +
> +     switch (code) {
> +     case V4L2_MBUS_FMT_UYVY8_2X8:
> +     case V4L2_MBUS_FMT_VYUY8_2X8:
> +     case V4L2_MBUS_FMT_YUYV8_2X8:
> +     case V4L2_MBUS_FMT_YVYU8_2X8:
> +             formats++;
> +             if (xlate) {
> +                     xlate->host_fmt = &isi_camera_formats[0];
> +                     xlate->code     = code;
> +                     xlate++;
> +                     dev_dbg(icd->dev.parent, "Providing format %s using 
> code %d\n",
> +                             isi_camera_formats[0].name, code);
> +             }
> +             break;
> +     default:
> +             if (!isi_camera_packing_supported(fmt))
> +                     return 0;
> +             if (xlate)
> +                     dev_dbg(dev,
> +                             "Providing format %s in pass-through mode\n",
> +                             fmt->name);
> +     }
> +
> +     /* Generic pass-through */
> +     formats++;
> +     if (xlate) {
> +             xlate->host_fmt = fmt;
> +             xlate->code     = code;
> +             xlate++;
> +     }
> +
> +     return formats;
> +}
> +
> +/* Called with .video_lock held */
> +static int isi_camera_add_device(struct soc_camera_device *icd)
> +{
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     int ret;
> +
> +     if (isi->icd)
> +             return -EBUSY;
> +
> +     ret = atmel_isi_init(isi);
> +     if (ret)
> +             return ret;
> +
> +     isi->icd = icd;
> +     dev_info(icd->dev.parent, "Atmel ISI Camera driver attached to camera 
> %d\n",
> +              icd->devnum);
> +     return 0;
> +}
> +/* Called with .video_lock held */
> +static void isi_camera_remove_device(struct soc_camera_device *icd)
> +{
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +
> +     BUG_ON(icd != isi->icd);
> +
> +     isi_writel(isi, V2_CTRL, ISI_BIT(V2_DIS));
> +     /* Check if module disable */
> +     while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_DIS))
> +             cpu_relax();
> +
> +     isi->icd = NULL;
> +
> +     dev_info(icd->dev.parent, "Atmel ISI Camera driver detached from camera 
> %d\n",
> +              icd->devnum);
> +}
> +
> +static unsigned int isi_camera_poll(struct file *file, poll_table *pt)
> +{
> +     struct soc_camera_device *icd = file->private_data;
> +
> +     return vb2_poll(&icd->vb2_vidq, file, pt);
> +}
> +
> +static int isi_camera_querycap(struct soc_camera_host *ici,
> +                            struct v4l2_capability *cap)
> +{
> +     strcpy(cap->driver, "atmel-isi");
> +     strcpy(cap->card, "Atmel Image Sensor Interface");
> +     cap->version = ATMEL_ISI_VERSION;
> +
> +     cap->capabilities = (V4L2_CAP_VIDEO_CAPTURE
> +                          | V4L2_CAP_READWRITE
> +                          | V4L2_CAP_STREAMING
> +                          );

In other places you have the |'s at the end of the line. Should be
consistent within a file.

> +     return 0;
> +}
> +
> +static int isi_camera_set_bus_param(struct soc_camera_device *icd, __u32 
> pixfmt)

u32.

> +{
> +     struct soc_camera_host *ici = to_soc_camera_host(icd->dev.parent);
> +     struct atmel_isi *isi = ici->priv;
> +     unsigned long bus_flags, camera_flags, common_flags;
> +     const struct soc_mbus_pixelfmt *fmt;
> +     int ret;
> +     u32 cfg1, ctrl;
> +
> +     fmt = soc_mbus_get_fmtdesc(icd->current_fmt->code);
> +     if (!fmt)
> +             return -EINVAL;
> +
> +     ret = test_platform_param(isi, fmt->bits_per_sample, &bus_flags);
> +     if (ret < 0)
> +             return ret;
> +
> +     camera_flags = icd->ops->query_bus_param(icd);
> +
> +     common_flags = soc_camera_bus_param_compatible(camera_flags, bus_flags);
> +     dev_dbg(icd->dev.parent, "Flags cam: 0x%lx host: 0x%lx common: 0x%lx\n",
> +             camera_flags, bus_flags, common_flags);
> +     if (!common_flags)
> +             return -EINVAL;
> +
> +     /* Make choises, based on platform preferences */
> +     if ((common_flags & SOCAM_HSYNC_ACTIVE_HIGH) &&
> +         (common_flags & SOCAM_HSYNC_ACTIVE_LOW)) {
> +             if (isi->pdata->hsync_act_low)
> +                     common_flags &= ~SOCAM_HSYNC_ACTIVE_HIGH;
> +             else
> +                     common_flags &= ~SOCAM_HSYNC_ACTIVE_LOW;
> +     }
> +
> +     if ((common_flags & SOCAM_VSYNC_ACTIVE_HIGH) &&
> +         (common_flags & SOCAM_VSYNC_ACTIVE_LOW)) {
> +             if (isi->pdata->vsync_act_low)
> +                     common_flags &= ~SOCAM_VSYNC_ACTIVE_HIGH;
> +             else
> +                     common_flags &= ~SOCAM_VSYNC_ACTIVE_LOW;
> +     }
> +
> +     if ((common_flags & SOCAM_PCLK_SAMPLE_RISING) &&
> +         (common_flags & SOCAM_PCLK_SAMPLE_FALLING)) {
> +             if (isi->pdata->pclk_act_falling)
> +                     common_flags &= ~SOCAM_PCLK_SAMPLE_RISING;
> +             else
> +                     common_flags &= ~SOCAM_PCLK_SAMPLE_FALLING;
> +     }
> +
> +     ret = icd->ops->set_bus_param(icd, common_flags);
> +     if (ret < 0) {
> +             dev_dbg(icd->dev.parent, "camera set_bus_param(%lx) returned 
> %d\n",
> +                     common_flags, ret);
> +             return ret;
> +     }
> +
> +     /* set bus param for ISI */
> +     if (common_flags & SOCAM_PCLK_SAMPLE_FALLING)
> +             isi->pdata->pclk_act_falling = 1;
> +     if (common_flags & SOCAM_HSYNC_ACTIVE_LOW)
> +             isi->pdata->hsync_act_low = 1;
> +     if (common_flags & SOCAM_VSYNC_ACTIVE_LOW)
> +             isi->pdata->vsync_act_low = 1;
> +
> +     cfg1 = ISI_BF(V2_EMB_SYNC, (isi->pdata->has_emb_sync))
> +             | ISI_BF(V2_HSYNC_POL, isi->pdata->hsync_act_low)
> +             | ISI_BF(V2_VSYNC_POL, isi->pdata->vsync_act_low)
> +             | ISI_BF(V2_PIXCLK_POL, isi->pdata->pclk_act_falling)
> +             | ISI_BF(V2_FULL, isi->pdata->isi_full_mode);
> +
> +     ctrl = ISI_BIT(DIS);
> +     isi_writel(isi, V2_CFG1, cfg1);
> +     isi_writel(isi, V2_CTRL, ctrl);
> +
> +     return 0;
> +}
> +
> +static struct soc_camera_host_ops isi_soc_camera_host_ops = {
> +     .owner          = THIS_MODULE,
> +     .add            = isi_camera_add_device,
> +     .remove         = isi_camera_remove_device,
> +     .set_fmt        = isi_camera_set_fmt,
> +     .try_fmt        = isi_camera_try_fmt,
> +     .get_formats    = isi_camera_get_formats,
> +     .init_videobuf2 = isi_camera_init_videobuf,
> +     .poll           = isi_camera_poll,
> +     .querycap       = isi_camera_querycap,
> +     .set_bus_param  = isi_camera_set_bus_param,
> +};
> +
> +/* -----------------------------------------------------------------------*/
> +static int __exit atmel_isi_remove(struct platform_device *pdev)
> +{
> +     struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
> +     struct atmel_isi *isi = container_of(soc_host,
> +                                     struct atmel_isi, soc_host);
> +
> +     soc_camera_host_unregister(soc_host);
> +     vb2_dma_contig_cleanup_ctx(isi->alloc_ctx);
> +
> +     free_irq(isi->irq, isi);
> +     iounmap(isi->regs);
> +     clk_disable(isi->pclk);
> +     clk_put(isi->pclk);
> +
> +     return 0;
> +}
> +
> +static int __init atmel_isi_probe(struct platform_device *pdev)
> +{
> +     unsigned int irq;
> +     struct atmel_isi *isi;
> +     struct clk *pclk;
> +     struct resource *regs;
> +     int ret;
> +     struct device *dev = &pdev->dev;
> +     struct isi_platform_data *pdata;
> +     struct soc_camera_host *soc_host;
> +
> +     regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     if (!regs)
> +             return -ENXIO;
> +
> +     pclk = clk_get(&pdev->dev, "isi_clk");
> +     if (IS_ERR(pclk))
> +             return PTR_ERR(pclk);
> +
> +     clk_enable(pclk);
> +
> +     isi = kzalloc(sizeof(struct atmel_isi), GFP_KERNEL);
> +     if (!isi) {
> +             ret = -ENOMEM;
> +             dev_err(&pdev->dev, "can't allocate interface!\n");
> +             goto err_alloc_isi;
> +     }
> +
> +     isi->pclk = pclk;
> +
> +     spin_lock_init(&isi->lock);
> +     init_waitqueue_head(&isi->capture_wq);
> +
> +     isi->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
> +     if (IS_ERR(isi->alloc_ctx)) {
> +             ret = PTR_ERR(isi->alloc_ctx);
> +             goto err_alloc_isi;
> +     }
> +
> +     isi->regs = ioremap(regs->start, resource_size(regs));
> +     if (!isi->regs) {
> +             ret = -ENOMEM;
> +             goto err_ioremap;
> +     }
> +
> +     if (dev->platform_data)
> +             pdata = (struct isi_platform_data *) dev->platform_data;
> +     else {
> +             static struct isi_platform_data isi_default_data = {
> +                     .frate          = 0,
> +                     .has_emb_sync   = 0,
> +                     .emb_crc_sync   = 0,
> +                     .hsync_act_low  = 0,
> +                     .vsync_act_low  = 0,
> +                     .pclk_act_falling = 0,
> +                     .isi_full_mode  = 1,
> +                     /* to use codec and preview path simultaneously */
> +                     .flags = ISI_DATAWIDTH_8 |
> +                             ISI_DATAWIDTH_10,
> +             };

Move this struct definition outside of this function.

> +             dev_info(&pdev->dev,
> +                     "No config available using default values\n");
> +             pdata = &isi_default_data;
> +     }
> +
> +     isi->pdata = pdata;
> +     isi->platform_flags = pdata->flags;
> +     if (isi->platform_flags == 0)
> +             isi->platform_flags = ISI_DATAWIDTH_8;

We could be mean here an require that people get the flags correct, e.g.

        if (!((isi->platform_flags & ISI_DATA_WIDTH_8) ||
              (isi->platform_flags & ISI_DATA_WIDTH_8))) {
                dev_err(&pdev->dev, "No data width specified\n");
                err = -ENOMEM;
                goto fail;
        }

Which discourages sloppy coding in the board files.

> +
> +     isi_writel(isi, V2_CTRL, ISI_BIT(V2_DIS));
> +     /* Check if module disable */
> +     while (isi_readl(isi, V2_STATUS) & ISI_BIT(V2_DIS))
> +             cpu_relax();
> +
> +     irq = platform_get_irq(pdev, 0);
> +     ret = request_irq(irq, isi_interrupt, 0, "isi", isi);
> +     if (ret) {
> +             dev_err(&pdev->dev, "unable to request irq %d\n", irq);
> +             goto err_req_irq;
> +     }
> +     isi->irq = irq;
> +
> +     INIT_LIST_HEAD(&isi->video_buffer_list);
> +     isi->still_capture = 0;
> +     isi->active = NULL;
> +
> +     soc_host                = &isi->soc_host;
> +     soc_host->drv_name      = "isi-camera";
> +     soc_host->ops           = &isi_soc_camera_host_ops;
> +     soc_host->priv          = isi;
> +     soc_host->v4l2_dev.dev  = &pdev->dev;
> +     soc_host->nr            = pdev->id;
> +
> +     ret = soc_camera_host_register(soc_host);
> +     if (ret) {
> +             dev_err(&pdev->dev, "unable to register soc camera host\n");
> +             goto err_register_soc_camera_host;
> +     }
> +     return 0;
> +
> +err_register_soc_camera_host:
> +     free_irq(isi->irq, isi);
> +err_req_irq:
> +     iounmap(isi->regs);
> +err_ioremap:
> +     vb2_dma_contig_cleanup_ctx(isi->alloc_ctx);
> +     kfree(isi);
> +err_alloc_isi:
> +     clk_disable(pclk);
> +
> +     return ret;
> +}
> +
> +static struct platform_driver atmel_isi_driver = {
> +     .probe          = atmel_isi_probe,
> +     .remove         = __exit_p(atmel_isi_remove),
> +     .driver         = {
> +             .name = "atmel_isi",
> +             .owner = THIS_MODULE,
> +     },
> +};
> +
> +static int __init atmel_isi_init_module(void)
> +{
> +     return  platform_driver_probe(&atmel_isi_driver, &atmel_isi_probe);
> +}
> +
> +static void __exit atmel_isi_exit(void)
> +{
> +     platform_driver_unregister(&atmel_isi_driver);
> +}
> +module_init(atmel_isi_init_module);
> +module_exit(atmel_isi_exit);
> +
> +MODULE_AUTHOR("Josh Wu <josh...@atmel.com>");
> +MODULE_DESCRIPTION("The V4L2 driver for Atmel Linux");
> +MODULE_LICENSE("GPL");
> +MODULE_SUPPORTED_DEVICE("video");


-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon                     5 Amuri Park, 404 Barbadoes St
r...@bluewatersys.com           PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com     New Zealand
Phone: +64 3 3779127            Freecall: Australia 1800 148 751
Fax:   +64 3 3779135                      USA 1800 261 2934
--
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