Hi Laurent, Javier,
On 07/27/2011 11:13 AM, Laurent Pinchart wrote:
> From: Javier Martin<javier.martin@xxxxxxxxxxxxxxxxx>
>
> The MT9P031 is a parallel 12-bit 5MP sensor from Aptina (formerly
> Micron) controlled through I2C.
>
> The driver creates a V4L2 subdevice. It currently supports skipping,
> cropping, automatic binning, and gain, exposure, h/v flip and test
> pattern controls.
>
> Signed-off-by: Javier Martin<javier.martin@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart<laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/video/Kconfig | 7 +
> drivers/media/video/Makefile | 1 +
> drivers/media/video/mt9p031.c | 961 +++++++++++++++++++++++++++++++++++++++++
> include/media/mt9p031.h | 19 +
> 4 files changed, 988 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/mt9p031.c
> create mode 100644 include/media/mt9p031.h
>
> Hi Javier,
>
> I've finally been able to spend some time on your MT9P031 driver, and here is
> the result. I would like to push the driver to mainline for v3.2. Could you
> please review it ?
>
> I still need to have a look at the PLL code to see if we can compute the PLL
> registers values at runtime instead of using a table-based approach.
>
> BTW, do you plan to maintain the driver ?
>
...
> diff --git a/drivers/media/video/mt9p031.c b/drivers/media/video/mt9p031.c
> new file mode 100644
> index 0000000..18e9a3c
> --- /dev/null
> +++ b/drivers/media/video/mt9p031.c
> @@ -0,0 +1,961 @@
> +/*
> + * Driver for MT9P031 CMOS Image Sensor from Aptina
> + *
> + * Copyright (C) 2011, Laurent Pinchart<laurent.pinchart@xxxxxxxxxxxxxxxx>
> + * Copyright (C) 2011, Javier Martin<javier.martin@xxxxxxxxxxxxxxxxx>
> + * Copyright (C) 2011, Guennadi Liakhovetski<g.liakhovetski@xxxxxx>
> + *
> + * Based on the MT9V032 driver and Bastian Hecht's code.
> + *
> + * 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.
> + */
> +
...
> +
> +struct mt9p031 {
> + struct v4l2_subdev subdev;
> + struct media_pad pad;
> + struct v4l2_rect crop; /* Sensor window */
> + struct v4l2_mbus_framefmt format;
> + struct v4l2_ctrl_handler ctrls;
> + struct mt9p031_platform_data *pdata;
> + struct mutex power_lock; /* lock to protect power_count */
> + int power_count;
> + u16 xskip;
> + u16 yskip;
> + u32 ext_freq;
> + /* pll dividers */
> + u8 m;
> + u8 n;
> + u8 p1;
> +
> + /* Registers cache */
> + u16 output_control;
> + u16 mode2;
> +};
...
> +/* -----------------------------------------------------------------------------
> + * Driver initialization and probing
> + */
> +
> +static int mt9p031_probe(struct i2c_client *client,
> + const struct i2c_device_id *did)
> +{
> + struct mt9p031_platform_data *pdata = client->dev.platform_data;
> + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> + struct mt9p031 *mt9p031;
> + unsigned int i;
> + int ret;
> +
> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
> + dev_warn(&adapter->dev,
> + "I2C-Adapter doesn't support I2C_FUNC_SMBUS_WORD\n");
> + return -EIO;
> + }
> +
> + mt9p031 = kzalloc(sizeof(*mt9p031), GFP_KERNEL);
> + if (mt9p031 == NULL)
> + return -ENOMEM;
> +
> + mt9p031->pdata = pdata;
> + mt9p031->output_control = MT9P031_OUTPUT_CONTROL_DEF;
> + mt9p031->mode2 = MT9P031_READ_MODE_2_ROW_BLC;
> +
> + v4l2_ctrl_handler_init(&mt9p031->ctrls, ARRAY_SIZE(mt9p031_ctrls) + 4);
> +
> + v4l2_ctrl_new_std(&mt9p031->ctrls,&mt9p031_ctrl_ops,
> + V4L2_CID_EXPOSURE, MT9P031_SHUTTER_WIDTH_MIN,
> + MT9P031_SHUTTER_WIDTH_MAX, 1,
> + MT9P031_SHUTTER_WIDTH_DEF);
> + v4l2_ctrl_new_std(&mt9p031->ctrls,&mt9p031_ctrl_ops,
> + V4L2_CID_GAIN, MT9P031_GLOBAL_GAIN_MIN,
> + MT9P031_GLOBAL_GAIN_MAX, 1, MT9P031_GLOBAL_GAIN_DEF);
> + v4l2_ctrl_new_std(&mt9p031->ctrls,&mt9p031_ctrl_ops,
> + V4L2_CID_HFLIP, 0, 1, 1, 0);
> + v4l2_ctrl_new_std(&mt9p031->ctrls,&mt9p031_ctrl_ops,
> + V4L2_CID_VFLIP, 0, 1, 1, 0);
> +
> + for (i = 0; i< ARRAY_SIZE(mt9p031_ctrls); ++i)
> + v4l2_ctrl_new_custom(&mt9p031->ctrls,&mt9p031_ctrls[i], NULL);
> +
> + mt9p031->subdev.ctrl_handler =&mt9p031->ctrls;
> +
> + if (mt9p031->ctrls.error)
> + printk(KERN_INFO "%s: control initialization error %d\n",
> + __func__, mt9p031->ctrls.error);
> +
> + mutex_init(&mt9p031->power_lock);
> + v4l2_i2c_subdev_init(&mt9p031->subdev, client,&mt9p031_subdev_ops);
> + mt9p031->subdev.internal_ops =&mt9p031_subdev_internal_ops;
> +
> + mt9p031->pad.flags = MEDIA_PAD_FL_SOURCE;
> + ret = media_entity_init(&mt9p031->subdev.entity, 1,&mt9p031->pad, 0);
> + if (ret< 0)
> + goto done;
> +
> + mt9p031->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + mt9p031->crop.width = MT9P031_WINDOW_WIDTH_DEF;
> + mt9p031->crop.height = MT9P031_WINDOW_HEIGHT_DEF;
> + mt9p031->crop.left = MT9P031_COLUMN_START_DEF;
> + mt9p031->crop.top = MT9P031_ROW_START_DEF;
> +
> + if (mt9p031->pdata->version == MT9P031_MONOCHROME_VERSION)
> + mt9p031->format.code = V4L2_MBUS_FMT_Y12_1X12;
> + else
> + mt9p031->format.code = V4L2_MBUS_FMT_SGRBG12_1X12;
> +
> + mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF;
> + mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
> + mt9p031->format.field = V4L2_FIELD_NONE;
> + mt9p031->format.colorspace = V4L2_COLORSPACE_SRGB;
> +
> + ret = mt9p031_pll_get_divs(mt9p031);
> +
> +done:
> + if (ret< 0)
It seems there is a v4l2_ctrl_handler_free() missing here...
> + kfree(mt9p031);
> +
> + return ret;
> +}
> +
> +static int mt9p031_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *subdev = i2c_get_clientdata(client);
> + struct mt9p031 *mt9p031 = to_mt9p031(subdev);
> +
> + v4l2_device_unregister_subdev(subdev);
> + media_entity_cleanup(&subdev->entity);
... and here.
> + kfree(mt9p031);
> +
> + return 0;
> +}
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
