Hi Ondrej,
Thank you!
I will fix it.
Thanks
Yong

------------------------------------------------------------------发件人:Ondřej 
Jirman <meg...@megous.com>发送时间:2017年9月21日(星期四) 21:45收件人:邓永 
<yong.d...@magewell.com>; maxime.ripard <maxime.rip...@free-electrons.com>抄 
送:Mauro Carvalho Chehab <mche...@kernel.org>; Rob Herring <robh...@kernel.org>; 
Mark Rutland <mark.rutl...@arm.com>; Chen-Yu Tsai <w...@csie.org>; Greg 
Kroah-Hartman <gre...@linuxfoundation.org>; David S. Miller 
<da...@davemloft.net>; Hans Verkuil <hverk...@xs4all.nl>; Arnd Bergmann 
<a...@arndb.de>; Hugues Fruchet <hugues.fruc...@st.com>; Yannick Fertre 
<yannick.fer...@st.com>; Philipp Zabel <p.za...@pengutronix.de>; Benoit Parrot 
<bpar...@ti.com>; Benjamin Gaignard <benjamin.gaign...@linaro.org>; 
Jean-Christophe Trotin <jean-christophe.tro...@st.com>; Ramesh Shanmugasundaram 
<ramesh.shanmugasunda...@bp.renesas.com>; Minghsiu Tsai 
<minghsiu.t...@mediatek.com>; Krzysztof Kozlowski <k...@kernel.org>; Robert 
Jarzmik <robert.jarz...@free.fr>; linux-media <linux-me...@vger.kernel.org>; 
devicetree <devicet...@vger.kernel.org>; linux-arm-kernel 
<linux-arm-ker...@lists.infradead.org>; linux-kernel 
<linux-ker...@vger.kernel.org>; linux-sunxi <linux-sunxi@googlegroups.com>主 
题:Re: [linux-sunxi] [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Hello Yong,

I noticed one issue in the register macros. See below.

Yong Deng píše v Čt 27. 07. 2017 v 13:01 +0800:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Signed-off-by: Yong Deng <yong.d...@magewell.com>
> ---
> 

[snip]

> +
> +#define CSI_CH_INT_EN_REG  0x70
> +#define CSI_CH_INT_EN_VS_INT_EN   BIT(7)
> +#define CSI_CH_INT_EN_HB_OF_INT_EN  BIT(6)
> +#define CSI_CH_INT_EN_MUL_ERR_INT_EN  BIT(5)
> +#define CSI_CH_INT_EN_FIFO2_OF_INT_EN  BIT(4)
> +#define CSI_CH_INT_EN_FIFO1_OF_INT_EN  BIT(3)
> +#define CSI_CH_INT_EN_FIFO0_OF_INT_EN  BIT(2)
> +#define CSI_CH_INT_EN_FD_INT_EN   BIT(1)
> +#define CSI_CH_INT_EN_CD_INT_EN   BIT(0)
> +
> +#define CSI_CH_INT_STA_REG  0x74
> +#define CSI_CH_INT_STA_VS_PD   BIT(7)
> +#define CSI_CH_INT_STA_HB_OF_PD   BIT(6)
> +#define CSI_CH_INT_STA_MUL_ERR_PD  BIT(5)
> +#define CSI_CH_INT_STA_FIFO2_OF_PD  BIT(4)
> +#define CSI_CH_INT_STA_FIFO1_OF_PD  BIT(3)
> +#define CSI_CH_INT_STA_FIFO0_OF_PD  BIT(2)
> +#define CSI_CH_INT_STA_FD_PD   BIT(1)
> +#define CSI_CH_INT_STA_CD_PD   BIT(0)
> +
> +#define CSI_CH_FLD1_VSIZE_REG  0x74

This register should be 0x78 according to the V3s manual. Though it's
not used in your driver yet, so it is not yet causing any issues.

> +#define CSI_CH_HSIZE_REG  0x80
> +#define CSI_CH_HSIZE_HOR_LEN_MASK  GENMASK(28, 16)
> +#define CSI_CH_HSIZE_HOR_LEN(len)  ((len << 16) & CSI_CH_HSIZE_HOR_LEN_MASK)
> +#define CSI_CH_HSIZE_HOR_START_MASK  GENMASK(12, 0)
> +#define CSI_CH_HSIZE_HOR_START(start)  ((start << 0) & 
>CSI_CH_HSIZE_HOR_START_MASK)
> +
> +#define CSI_CH_VSIZE_REG  0x84
> +#define CSI_CH_VSIZE_VER_LEN_MASK  GENMASK(28, 16)
> +#define CSI_CH_VSIZE_VER_LEN(len)  ((len << 16) & CSI_CH_VSIZE_VER_LEN_MASK)
> +#define CSI_CH_VSIZE_VER_START_MASK  GENMASK(12, 0)
> +#define CSI_CH_VSIZE_VER_START(start)  ((start << 0) & 
>CSI_CH_VSIZE_VER_START_MASK)
> +
> +#define CSI_CH_BUF_LEN_REG  0x88
> +#define CSI_CH_BUF_LEN_BUF_LEN_C_MASK  GENMASK(29, 16)
> +#define CSI_CH_BUF_LEN_BUF_LEN_C(len)  ((len << 16) & 
>CSI_CH_BUF_LEN_BUF_LEN_C_MASK)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y_MASK  GENMASK(13, 0)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y(len)  ((len << 0) & 
>CSI_CH_BUF_LEN_BUF_LEN_Y_MASK)
> +
> +#define CSI_CH_FLIP_SIZE_REG  0x8c
> +#define CSI_CH_FLIP_SIZE_VER_LEN_MASK  GENMASK(28, 16)
> +#define CSI_CH_FLIP_SIZE_VER_LEN(len)  ((len << 16) & 
>CSI_CH_FLIP_SIZE_VER_LEN_MASK)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN_MASK  GENMASK(12, 0)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN(len)  ((len << 0) & 
>CSI_CH_FLIP_SIZE_VALID_LEN_MASK)
> +
> +#define CSI_CH_FRM_CLK_CNT_REG  0x90
> +#define CSI_CH_ACC_ITNL_CLK_CNT_REG 0x94
> +#define CSI_CH_FIFO_STAT_REG  0x98
> +#define CSI_CH_PCLK_STAT_REG  0x9c
> +
> +/*
> + * csi input data format
> + */
> +enum csi_input_fmt
> +{
> + CSI_INPUT_FORMAT_RAW  = 0,
> + CSI_INPUT_FORMAT_YUV422  = 3,
> + CSI_INPUT_FORMAT_YUV420  = 4,
> +};
> +
> +/*
> + * csi output data format
> + */
> +enum csi_output_fmt
> +{
> + /* only when input format is RAW */
> + CSI_FIELD_RAW_8   = 0,
> + CSI_FIELD_RAW_10  = 1,
> + CSI_FIELD_RAW_12  = 2,
> + CSI_FIELD_RGB565  = 4,
> + CSI_FIELD_RGB888  = 5,
> + CSI_FIELD_PRGB888  = 6,
> + CSI_FRAME_RAW_8   = 8,
> + CSI_FRAME_RAW_10  = 9,
> + CSI_FRAME_RAW_12  = 10,
> + CSI_FRAME_RGB565  = 12,
> + CSI_FRAME_RGB888  = 13,
> + CSI_FRAME_PRGB888  = 14,
> +
> + /* only when input format is YUV422/YUV420 */

Other limitation is that when input is YUV420 output can only be YUV420
and not YUV422.

> + CSI_FIELD_PLANAR_YUV422  = 0,
> + CSI_FIELD_PLANAR_YUV420  = 1,
> + CSI_FRAME_PLANAR_YUV420  = 2,
> + CSI_FRAME_PLANAR_YUV422  = 3,
> + CSI_FIELD_UV_CB_YUV422  = 4,
> + CSI_FIELD_UV_CB_YUV420  = 5,
> + CSI_FRAME_UV_CB_YUV420  = 6,
> + CSI_FRAME_UV_CB_YUV422  = 7,
> + CSI_FIELD_MB_YUV422  = 8,
> + CSI_FIELD_MB_YUV420  = 9,
> + CSI_FRAME_MB_YUV420  = 10,
> + CSI_FRAME_MB_YUV422  = 11,
> + CSI_FIELD_UV_CB_YUV422_10 = 12,
> + CSI_FIELD_UV_CB_YUV420_10 = 13,
> +};
> +

thank you and regards,
  Ondrej

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to