Re: [PATCH] drivers: media: video: Add support for Aptina ar0130 sensor
Hello,
I've spotted a minor issue while glancing through the code.
Cheers,
Nicolas
On 2012-09-07 11:30, Prashanth Subramanya wrote:
> This driver adds basic support for Aptina ar0130 1.2M sensor.
>
> Signed-off-by: Prashanth Subramanya
> ---
[snip]
> +/***
> + v4l2_subdev_video_ops
> +/
> +static int ar0130_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> + struct ar0130_priv *ar0130 = container_of(sd,
> + struct ar0130_priv, subdev);
> + int ret;
> +
> + if (!enable) {
> + ret = ar0130_write(client, AR0130_RESET_REG,
> AR0130_STREAM_OFF);
> + return 0;
> + }
> +
> + ret = ar0130_linear_mode_setup(client);
> + if (ret < 0) {
> + dev_err(ar0130->subdev.v4l2_dev->dev,
> + "Failed to setup linear mode: %d\n", ret);
> + return ret;
> + }
> +
> + ret = ar0130_set_resolution(client, ar0130->res_index);
> + if (ret < 0) {
> + dev_err(ar0130->subdev.v4l2_dev->dev,
> + "Failed to setup resolution: %d\n", ret);
> + return ret;
> + }
> +
> + ret |= ar0130_write(client, AR0130_RESET_REG, AR0130_STREAM_OFF);
> + ret |= ar0130_write(client, AR0130_HDR_COMP, 0x0001);
> +
> + ret = ar0130_pll_enable(ar0130);
The previous value of ret is overwritten here.
> + if (ret < 0) {
> + dev_err(ar0130->subdev.v4l2_dev->dev,
> + "Failed to enable pll: %d\n", ret);
> + return ret;
> + }
> +
> + ret = ar0130_set_autoexposure(client, AR0130_ENABLE);
> + ret |= ar0130_write(client, AR0130_RESET_REG, AR0130_STREAM_ON);
> +
> + return ret;
> +}
Re: [PATCH] drivers: media: video: Add support for Aptina ar0130 sensor
Hi Prashanth On Fri, 7 Sep 2012, Prashanth Subramanya wrote: > This driver adds basic support for Aptina ar0130 1.2M sensor. > > Signed-off-by: Prashanth Subramanya > --- > drivers/media/video/Kconfig |7 + > drivers/media/video/Makefile |1 + > drivers/media/video/ar0130.c | 1114 > + > drivers/media/video/ar0130_regs.h | 107 > include/media/ar0130.h| 52 ++ > include/media/v4l2-chip-ident.h |1 + > 6 files changed, 1282 insertions(+) > create mode 100644 drivers/media/video/ar0130.c > create mode 100644 drivers/media/video/ar0130_regs.h > create mode 100644 include/media/ar0130.h > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > index 99937c9..54d7063 100644 > --- a/drivers/media/video/Kconfig > +++ b/drivers/media/video/Kconfig > @@ -493,6 +493,13 @@ config VIDEO_VS6624 > To compile this driver as a module, choose M here: the > module will be called vs6624. > > +config VIDEO_AR0130 > + tristate "Aptina AR0130 support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + ---help--- > + This is a Video4Linux2 sensor-level driver for the Aptina > + ar0130 1.2 Mpixel camera. > + > config VIDEO_MT9M032 > tristate "MT9M032 camera sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile > index d209de0..a208911 100644 > --- a/drivers/media/video/Makefile > +++ b/drivers/media/video/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o > obj-$(CONFIG_VIDEO_OV7670) += ov7670.o > obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o > obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o > +obj-$(CONFIG_VIDEO_AR0130) += ar0130.o > obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o > obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o > obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o > diff --git a/drivers/media/video/ar0130.c b/drivers/media/video/ar0130.c > new file mode 100644 > index 000..d257fe8 > --- /dev/null > +++ b/drivers/media/video/ar0130.c > @@ -0,0 +1,1114 @@ > +/* > + * drivers/media/video/ar0130.c > + * > + * Aptina AR0130 sensor driver > + * > + * Copyright (C) 2012 Aptina Imaging > + * > + * Contributor Prashanth Subramanya > + * > + * Based on MT9P031 driver > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include Do you really need the soc_camera.h header? From a quick glance I didn't find any uses of the soc-camera API. If I missed them and you really are using the API, the driver should probably go under drivers/media/i2c/soc_camera/ and be submitted to the mainline via my tree. Since you're submitting your patch against an older tree, it is not clear, what your intended destination is. Further, since your driver is using the pad API, it very much looks like you don't need soc-camera. If this is the case, please, remove the header. 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-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers: media: video: Add support for Aptina ar0130 sensor
Hi Prashanth! Thanks for working on this driver. I do have a few comments about this driver, mostly coding style and v4l2 API related things. On Fri September 7 2012 11:30:26 Prashanth Subramanya wrote: > This driver adds basic support for Aptina ar0130 1.2M sensor. > > Signed-off-by: Prashanth Subramanya > --- > drivers/media/video/Kconfig |7 + > drivers/media/video/Makefile |1 + > drivers/media/video/ar0130.c | 1114 > + > drivers/media/video/ar0130_regs.h | 107 > include/media/ar0130.h| 52 ++ > include/media/v4l2-chip-ident.h |1 + > 6 files changed, 1282 insertions(+) > create mode 100644 drivers/media/video/ar0130.c > create mode 100644 drivers/media/video/ar0130_regs.h > create mode 100644 include/media/ar0130.h > > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig > index 99937c9..54d7063 100644 > --- a/drivers/media/video/Kconfig > +++ b/drivers/media/video/Kconfig > @@ -493,6 +493,13 @@ config VIDEO_VS6624 > To compile this driver as a module, choose M here: the > module will be called vs6624. > > +config VIDEO_AR0130 > + tristate "Aptina AR0130 support" > + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > + ---help--- > + This is a Video4Linux2 sensor-level driver for the Aptina > + ar0130 1.2 Mpixel camera. > + > config VIDEO_MT9M032 > tristate "MT9M032 camera sensor support" > depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile > index d209de0..a208911 100644 > --- a/drivers/media/video/Makefile > +++ b/drivers/media/video/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o > obj-$(CONFIG_VIDEO_OV7670) += ov7670.o > obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o > obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o > +obj-$(CONFIG_VIDEO_AR0130) += ar0130.o > obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o > obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o > obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o > diff --git a/drivers/media/video/ar0130.c b/drivers/media/video/ar0130.c > new file mode 100644 > index 000..d257fe8 > --- /dev/null > +++ b/drivers/media/video/ar0130.c > @@ -0,0 +1,1114 @@ > +/* > + * drivers/media/video/ar0130.c > + * > + * Aptina AR0130 sensor driver > + * > + * Copyright (C) 2012 Aptina Imaging > + * > + * Contributor Prashanth Subramanya > + * > + * Based on MT9P031 driver > + * > + * 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. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include "ar0130_regs.h" > + > +#define AR0130_ROW_START_MIN 0 > +#define AR0130_ROW_START_MAX 1280 > +#define AR0130_ROW_START_DEF 0 > +#define AR0130_COLUMN_START_MIN 0 > +#define AR0130_COLUMN_START_MAX 960 > +#define AR0130_COLUMN_START_DEF 0 > +#define AR0130_WINDOW_HEIGHT_MIN 360 > +#define AR0130_WINDOW_HEIGHT_MAX 960 > +#define AR0130_WINDOW_HEIGHT_DEF 960 > +#define AR0130_WINDOW_WIDTH_MIN 640 > +#define AR0130_WINDOW_WIDTH_MAX 1280 > +#define AR0130_WINDOW_WIDTH_DEF 1280 > + > +#define AR0130_VGA_WIDTH 640 > +#define AR0130_VGA_HEIGHT480 > +#define AR0130_ENABLE1 > +#define AR0130_DISABLE 0 > + > +#define AR0130_CHIP_VERSION_REG 0x3000 > +#define AR0130_CHIP_ID 0x2402 > +#define AR0130_RESET_REG 0x301A > +#define AR0130_STREAM_ON 0x10DC > +#define AR0130_STREAM_OFF0x10D8 > +#define AR0130_SEQ_PORT 0x3086 > +#define AR0130_SEQ_PORT_CTRL 0x3088 > +#define AR0130_TEST_REG 0x3070 > +#define AR0130_TEST_PATTERN_DISABLE 0x > +#define AR0130_TEST_PATTERN_ENABLE 0x0002 > +/* > +@AR0130_TEST_PATTERN > +0 = Normal operation. Generate output data from pixel array > +1 = Solid color test pattern. > +2 = Full color bar test pattern > +3 = Fade to grey color bar test pattern > +256 = Marching 1 test pattern (12 bit) > +*/ > + > +#define AR0130_DCDS_PROG_START_ADDR 0x309E > +#define AR0130_ADC_BITS_6_7 0x30E4 > +#define A
[PATCH] drivers: media: video: Add support for Aptina ar0130 sensor
This driver adds basic support for Aptina ar0130 1.2M sensor. Signed-off-by: Prashanth Subramanya --- drivers/media/video/Kconfig |7 + drivers/media/video/Makefile |1 + drivers/media/video/ar0130.c | 1114 + drivers/media/video/ar0130_regs.h | 107 include/media/ar0130.h| 52 ++ include/media/v4l2-chip-ident.h |1 + 6 files changed, 1282 insertions(+) create mode 100644 drivers/media/video/ar0130.c create mode 100644 drivers/media/video/ar0130_regs.h create mode 100644 include/media/ar0130.h diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 99937c9..54d7063 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -493,6 +493,13 @@ config VIDEO_VS6624 To compile this driver as a module, choose M here: the module will be called vs6624. +config VIDEO_AR0130 + tristate "Aptina AR0130 support" + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API + ---help--- + This is a Video4Linux2 sensor-level driver for the Aptina + ar0130 1.2 Mpixel camera. + config VIDEO_MT9M032 tristate "MT9M032 camera sensor support" depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile index d209de0..a208911 100644 --- a/drivers/media/video/Makefile +++ b/drivers/media/video/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o obj-$(CONFIG_VIDEO_TVEEPROM) += tveeprom.o +obj-$(CONFIG_VIDEO_AR0130) += ar0130.o obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o obj-$(CONFIG_VIDEO_MT9P031) += mt9p031.o obj-$(CONFIG_VIDEO_MT9T001) += mt9t001.o diff --git a/drivers/media/video/ar0130.c b/drivers/media/video/ar0130.c new file mode 100644 index 000..d257fe8 --- /dev/null +++ b/drivers/media/video/ar0130.c @@ -0,0 +1,1114 @@ +/* + * drivers/media/video/ar0130.c + * + * Aptina AR0130 sensor driver + * + * Copyright (C) 2012 Aptina Imaging + * + * Contributor Prashanth Subramanya + * + * Based on MT9P031 driver + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include "ar0130_regs.h" + +#define AR0130_ROW_START_MIN 0 +#define AR0130_ROW_START_MAX 1280 +#define AR0130_ROW_START_DEF 0 +#define AR0130_COLUMN_START_MIN0 +#define AR0130_COLUMN_START_MAX960 +#define AR0130_COLUMN_START_DEF0 +#define AR0130_WINDOW_HEIGHT_MIN 360 +#define AR0130_WINDOW_HEIGHT_MAX 960 +#define AR0130_WINDOW_HEIGHT_DEF 960 +#define AR0130_WINDOW_WIDTH_MIN640 +#define AR0130_WINDOW_WIDTH_MAX1280 +#define AR0130_WINDOW_WIDTH_DEF1280 + +#define AR0130_VGA_WIDTH 640 +#define AR0130_VGA_HEIGHT 480 +#define AR0130_ENABLE 1 +#define AR0130_DISABLE 0 + +#define AR0130_CHIP_VERSION_REG0x3000 +#define AR0130_CHIP_ID 0x2402 +#define AR0130_RESET_REG 0x301A +#define AR0130_STREAM_ON 0x10DC +#define AR0130_STREAM_OFF 0x10D8 +#define AR0130_SEQ_PORT0x3086 +#define AR0130_SEQ_PORT_CTRL 0x3088 +#define AR0130_TEST_REG0x3070 +#define AR0130_TEST_PATTERN_DISABLE0x +#define AR0130_TEST_PATTERN_ENABLE 0x0002 +/* +@AR0130_TEST_PATTERN +0 = Normal operation. Generate output data from pixel array +1 = Solid color test pattern. +2 = Full color bar test pattern +3 = Fade to grey color bar test pattern +256 = Marching 1 test pattern (12 bit) +*/ + +#define AR0130_DCDS_PROG_START_ADDR0x309E +#define AR0130_ADC_BITS_6_70x30E4 +#define AR0130_ADC_BITS_4_50x30E2 +#define AR0130_ADC_BITS_2_30x30E0 +#define AR0130_ADC_CONFIG1 0x30E6 +#define AR0130_ADC_CONFIG2 0x30E8 + +#define AR0130_VT_SYS_CLK_DIV 0x302C +#define AR0130_VT_PIX_CLK_DIV 0x302A +#define AR0130_PRE_PLL_CLK_DIV 0x302E +#define AR0130_PLL_MULTIPLIER 0x3030 +#define AR0130_DIGITAL_TEST0x30B0 + +#define A
