Hi Laurent,
Thanks for the patch!
On Thu, Nov 15, 2012 at 01:15:46AM +0100, Laurent Pinchart wrote:
> The AD5820 is a voice coil motor controller typically used to control
> lens position in digital cameras.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/i2c/Kconfig | 9 +
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/ad5820.c | 496 ++++++++++++++++++++++++++++++++++++++++++++
> include/media/ad5820.h | 30 +++
> 4 files changed, 536 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/i2c/ad5820.c
> create mode 100644 include/media/ad5820.h
>
> Hi Florian,
>
> This is the ad5820 driver I've told you about. The code is compile-tested only
> as I haven't had time to try it on an N900 (the only device I own that
> includes an ad5820).
>
> It should be quite easy to adapt the driver to support both the ad5820 and the
> ad5821. Would you have time to give it a try ?
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 24d78e2..65597cf 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -534,6 +534,15 @@ config VIDEO_AS3645A
> This is a driver for the AS3645A and LM3555 flash controllers. It has
> build in control for flash, torch and indicator LEDs.
>
> +comment "Lens controllers"
> +
> +config VIDEO_AD5820
> + tristate "AD5820 lens voice coil support"
> + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> + ---help---
> + This is a driver for the AD5820 camera lens voice coil.
> + It is used for example in Nokia RX51.
> +
> comment "Video improvement chips"
>
> config VIDEO_UPD64031A
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index b1d62df..975cfb8 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -58,6 +58,7 @@ obj-$(CONFIG_VIDEO_SR030PC30) += sr030pc30.o
> obj-$(CONFIG_VIDEO_NOON010PC30) += noon010pc30.o
> obj-$(CONFIG_VIDEO_S5K6AA) += s5k6aa.o
> obj-$(CONFIG_VIDEO_S5K4ECGX) += s5k4ecgx.o
> +obj-$(CONFIG_VIDEO_AD5820) += ad5820.o
> obj-$(CONFIG_VIDEO_ADP1653) += adp1653.o
> obj-$(CONFIG_VIDEO_AS3645A) += as3645a.o
> obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
> diff --git a/drivers/media/i2c/ad5820.c b/drivers/media/i2c/ad5820.c
> new file mode 100644
> index 0000000..995774f
> --- /dev/null
> +++ b/drivers/media/i2c/ad5820.c
> @@ -0,0 +1,496 @@
> +/*
> + * AD5820 DAC driver for camera voice coil focus.
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + * Copyright (C) 2007 Texas Instruments
Wow. This is old.
> + * Contact: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + * Sakari Ailus <sakari.ailus@xxxxxx>
> + *
> + * 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.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
Do you need all of these? E.g. delay.h and sched.h are probably not needed.
> +
> +#include <media/ad5820.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +
> +/* Register definitions */
> +#define AD5820_POWER_DOWN (1 << 15)
> +#define AD5820_DAC_SHIFT 4
> +#define AD5820_RAMP_MODE_LINEAR (0 << 3)
> +#define AD5820_RAMP_MODE_64_16 (1 << 3)
> +
> +#define CODE_TO_RAMP_US(s) ((s) == 0 ? 0 : (1 << ((s) - 1)) * 50)
> +#define RAMP_US_TO_CODE(c) fls(((c) + ((c)>>1)) / 50)
> +
> +struct ad5820_device {
> + struct v4l2_subdev subdev;
> +
> + struct regulator *vana;
> + int xshutdown;
> +
> + struct v4l2_ctrl_handler ctrls;
> + u32 focus_absolute;
> + u32 focus_ramp_time;
> + u32 focus_ramp_mode;
> +
> + struct mutex power_lock;
> + int power_count;
> +
> + int standby:1;
> +};
> +
> +#define to_ad5820_device(sd) container_of(sd, struct ad5820_device, subdev)
> +
> +/**
> + * @brief I2C write using i2c_transfer().
> + * @param coil - the driver data structure
> + * @param data - register value to be written
> + * @returns nonnegative on success, negative if failed
> + */
> +static int ad5820_write(struct ad5820_device *coil, u16 data)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&coil->subdev);
> + struct i2c_msg msg;
> + int r;
> +
> + if (!client->adapter)
> + return -ENODEV;
> +
> + data = cpu_to_be16(data);
> + msg.addr = client->addr;
> + msg.flags = 0;
> + msg.len = 2;
> + msg.buf = (u8 *)&data;
> +
> + r = i2c_transfer(client->adapter, &msg, 1);
I know we've gotten used to i2c_transfer(), but wouldn't
i2c_master_{send,recv}() be better for the task?
> + if (r < 0) {
> + dev_err(&client->dev, "write failed, error %d\n", r);
> + return r;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * @brief I2C read using i2c_transfer().
> + * @param coil - the driver data structure
> + * @returns unsigned 16-bit register value on success, negative if failed
> + */
> +static int ad5820_read(struct ad5820_device *coil)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&coil->subdev);
> + struct i2c_msg msg;
> + int r;
> + u16 data = 0;
> +
> + if (!client->adapter)
> + return -ENODEV;
> +
> + msg.addr = client->addr;
> + msg.flags = I2C_M_RD;
> + msg.len = 2;
> + msg.buf = (u8 *)&data;
> +
> + r = i2c_transfer(client->adapter, &msg, 1);
> + if (r < 0) {
> + dev_err(&client->dev, "read failed, error %d\n", r);
> + return r;
> + }
> +
> + return be16_to_cpu(data);
> +}
> +
> +/* Calculate status word and write it to the device based on current
> + * values of V4L2 controls. It is assumed that the stored V4L2 control
> + * values are properly limited and rounded. */
> +static int ad5820_update_hw(struct ad5820_device *coil)
> +{
> + u16 status;
> +
> + status = RAMP_US_TO_CODE(coil->focus_ramp_time);
> + status |= coil->focus_ramp_mode
> + ? AD5820_RAMP_MODE_64_16 : AD5820_RAMP_MODE_LINEAR;
> + status |= coil->focus_absolute << AD5820_DAC_SHIFT;
> +
> + if (coil->standby)
> + status |= AD5820_POWER_DOWN;
> +
> + return ad5820_write(coil, status);
> +}
> +
> +/* --------------------------------------------------------------------------
> + * Power handling
> + */
> +
> +static void ad5820_power_off(struct ad5820_device *coil)
> +{
> + if (coil->xshutdown != -1)
> + gpio_set_value(coil->xshutdown, 0);
> +
> + regulator_disable(coil->vana);
> +}
> +
> +static int ad5820_power_on(struct ad5820_device *coil)
> +{
> + int ret;
> +
> + ret = regulator_enable(coil->vana);
> + if (ret < 0)
> + return ret;
> +
> + if (coil->xshutdown != -1)
> + gpio_set_value(coil->xshutdown, 1);
> +
> + return 0;
> +}
> +
> +/* --------------------------------------------------------------------------
> + * V4L2 controls
> + */
> +
> +#define V4L2_CID_FOCUS_AD5820_RAMP_TIME (V4L2_CID_USER_BASE | 0x1000)
> +#define V4L2_CID_FOCUS_AD5820_RAMP_MODE (V4L2_CID_USER_BASE | 0x1001)
> +
> +static int ad5820_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct ad5820_device *coil =
> + container_of(ctrl->handler, struct ad5820_device, ctrls);
> + u32 code;
> + int r = 0;
> +
> + switch (ctrl->id) {
> + case V4L2_CID_FOCUS_ABSOLUTE:
> + coil->focus_absolute = ctrl->val;
> + return ad5820_update_hw(coil);
> +
> + case V4L2_CID_FOCUS_AD5820_RAMP_TIME:
> + code = RAMP_US_TO_CODE(ctrl->val);
> + ctrl->val = CODE_TO_RAMP_US(code);
> + coil->focus_ramp_time = ctrl->val;
> + break;
> +
> + case V4L2_CID_FOCUS_AD5820_RAMP_MODE:
> + coil->focus_ramp_mode = ctrl->val;
> + break;
> + }
> +
> + return r;
> +}
> +
> +static const struct v4l2_ctrl_ops ad5820_ctrl_ops = {
> + .s_ctrl = ad5820_set_ctrl,
> +};
> +
> +static const char * const ad5820_focus_menu[] = {
> + "Linear ramp",
> + "64/16 ramp",
> +};
> +
> +static const struct v4l2_ctrl_config ad5820_ctrls[] = {
> + {
> + .ops = &ad5820_ctrl_ops,
> + .id = V4L2_CID_FOCUS_AD5820_RAMP_TIME,
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .name = "Focus ramping time [us]",
> + .min = 0,
> + .max = 3200,
> + .step = 50,
> + .def = 0,
> + .flags = 0,
> + },
> + {
> + .ops = &ad5820_ctrl_ops,
> + .id = V4L2_CID_FOCUS_AD5820_RAMP_MODE,
> + .type = V4L2_CTRL_TYPE_MENU,
> + .name = "Focus ramping mode",
> + .min = 0,
> + .max = ARRAY_SIZE(ad5820_focus_menu),
> + .step = 0,
> + .def = 0,
> + .flags = 0,
> + .qmenu = ad5820_focus_menu,
> + },
> +};
> +
> +/* --------------------------------------------------------------------------
> + * V4L2 subdev operations
> + */
> +
> +static int __ad5820_set_power(struct ad5820_device *coil, bool on)
> +{
> + int ret;
> +
> + if (!on) {
> + /* Go to standby first as real power off my be denied by the
> + * hardware (single power line control for both coil and
> + * sensor).
> + */
> + coil->standby = 1;
> + ad5820_update_hw(coil);
> + ad5820_power_off(coil);
> + return 0;
> + }
> +
> + ret = ad5820_power_on(coil);
> + if (ret < 0)
> + return ret;
> +
> + /* Restore the hardware settings. */
> + coil->standby = 0;
> + ret = ad5820_update_hw(coil);
> + if (ret < 0) {
> + coil->standby = 1;
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ad5820_set_power(struct v4l2_subdev *subdev, int on)
> +{
> + struct ad5820_device *coil = to_ad5820_device(subdev);
> + int ret = 0;
> +
> + mutex_lock(&coil->power_lock);
> +
> + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
> + * update the power state.
> + */
> + if (coil->power_count == !on) {
> + ret = __ad5820_set_power(coil, !!on);
> + if (ret < 0)
> + goto done;
> + }
> +
> + /* Update the power count. */
> + coil->power_count += on ? 1 : -1;
> + WARN_ON(coil->power_count < 0);
> +
> +done:
> + mutex_unlock(&coil->power_lock);
> + return ret;
> +}
> +
> +static int ad5820_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + return ad5820_set_power(sd, 1);
> +}
> +
> +static int ad5820_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + return ad5820_set_power(sd, 0);
> +}
> +
> +static const struct v4l2_subdev_core_ops ad5820_core_ops = {
> + .s_power = ad5820_set_power,
> +};
> +
> +static const struct v4l2_subdev_ops ad5820_ops = {
> + .core = &ad5820_core_ops,
> +};
> +
> +static const struct v4l2_subdev_internal_ops ad5820_internal_ops = {
> + .open = ad5820_open,
> + .close = ad5820_close,
> +};
> +
> +/* --------------------------------------------------------------------------
> + * I2C driver
> + */
> +#ifdef CONFIG_PM
> +
> +static int ad5820_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *subdev = i2c_get_clientdata(client);
> + struct ad5820_device *coil = to_ad5820_device(subdev);
> +
> + if (!coil->power_count)
> + return 0;
> +
> + ad5820_power_off(coil);
> + return 0;
> +}
> +
> +static int ad5820_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *subdev = i2c_get_clientdata(client);
> + struct ad5820_device *coil = to_ad5820_device(subdev);
> +
> + if (!coil->power_count)
> + return 0;
> +
> + return __ad5820_set_power(coil, true);
> +}
> +
> +#endif /* CONFIG_PM */
> +
> +static int ad5820_detect(struct ad5820_device *coil)
> +{
> + const u16 status = AD5820_POWER_DOWN | 0x3ff0;
> + int ret;
> +
> + if (ad5820_power_on(coil) < 0)
> + return -ENODEV;
> +
> + if (ad5820_write(coil, status) || ad5820_read(coil) != status) {
> + ad5820_power_off(coil);
> + return -ENODEV;
> + }
> +
> + __ad5820_set_power(coil, false);
> + return ret;
> +}
> +
> +static int ad5820_probe(struct i2c_client *client,
> + const struct i2c_device_id *devid)
> +{
> + struct ad5820_platform_data *pdata = client->dev.platform_data;
> + struct ad5820_device *coil;
> + unsigned int i;
> + int ret = -ENODEV;
> +
> + coil = kzalloc(sizeof(*coil), GFP_KERNEL);
> + if (coil == NULL)
> + return -ENOMEM;
> +
> + mutex_init(&coil->power_lock);
> + coil->focus_absolute = 0;
> + coil->focus_ramp_time = 0;
> + coil->focus_ramp_mode = 0;
> + coil->xshutdown = -1;
> +
> + coil->vana = regulator_get(&client->dev, "VANA");
> + if (IS_ERR(coil->vana)) {
> + dev_err(&client->dev, "could not get regulator for vana\n");
> + goto done;
> + }
> +
> + if (pdata && pdata->xshutdown != -1) {
> + ret = gpio_request_one(pdata->xshutdown, GPIOF_OUT_INIT_LOW,
> + "ad5820_xshutdown");
> + if (ret < 0)
> + goto done;
> +
> + coil->xshutdown = pdata->xshutdown;
> + }
> +
> + ret = ad5820_detect(coil);
> + if (ret < 0) {
> + dev_err(&client->dev, "not detected\n");
> + goto done;
> + }
> +
> + v4l2_ctrl_handler_init(&coil->ctrls, ARRAY_SIZE(ad5820_ctrls) + 1);
> +
> + /* V4L2_CID_FOCUS_ABSOLUTE
> + *
> + * Minimum current is 0 mA, maximum is 100 mA. Thus, 1 code is
> + * equivalent to 100/1023 = 0.0978 mA. Nevertheless, we do not use [mA]
> + * for focus position, because it is meaningless for user. Meaningful
> + * would be to use focus distance or even its inverse, but since the
> + * driver doesn't have sufficiently knowledge to do the conversion, we
> + * will just use abstract codes here. In any case, smaller value = focus
> + * position farther from camera. The default zero value means focus at
> + * infinity, and also least current consumption.
> + */
> + v4l2_ctrl_new_std(&coil->ctrls, &ad5820_ctrl_ops,
> + V4L2_CID_FOCUS_ABSOLUTE, 0, 1023, 1, 0);
> +
> + /* V4L2_CID_TEST_PATTERN and V4L2_CID_MODE_* */
> + for (i = 0; i < ARRAY_SIZE(ad5820_ctrls); ++i)
> + v4l2_ctrl_new_custom(&coil->ctrls, &ad5820_ctrls[i], NULL);
> +
> + if (coil->ctrls.error) {
> + ret = coil->ctrls.error;
> + goto done;
> + }
> +
> + coil->subdev.ctrl_handler = &coil->ctrls;
> +
> + v4l2_i2c_subdev_init(&coil->subdev, client, &ad5820_ops);
> + coil->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + coil->subdev.internal_ops = &ad5820_internal_ops;
> +
> + ret = media_entity_init(&coil->subdev.entity, 0, NULL, 0);
> + if (ret < 0)
> + goto done;
> +
> + ret = 0;
> +
> +done:
> + if (ret < 0) {
> + v4l2_ctrl_handler_free(&coil->ctrls);
> + media_entity_cleanup(&coil->subdev.entity);
> +
> + if (coil->xshutdown != -1)
> + gpio_free(coil->xshutdown);
> + regulator_put(coil->vana);
> +
> + kfree(coil);
> + }
> +
> + return ret;
> +}
> +
> +static int __exit ad5820_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *subdev = i2c_get_clientdata(client);
> + struct ad5820_device *coil = to_ad5820_device(subdev);
> +
> + v4l2_device_unregister_subdev(&coil->subdev);
> + v4l2_ctrl_handler_free(&coil->ctrls);
> + media_entity_cleanup(&coil->subdev.entity);
> +
> + if (coil->xshutdown != -1)
> + gpio_free(coil->xshutdown);
> + regulator_put(coil->vana);
> +
> + kfree(coil);
> + return 0;
> +}
> +
> +static const struct i2c_device_id ad5820_id_table[] = {
> + { AD5820_NAME, 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ad5820_id_table);
> +
> +static SIMPLE_DEV_PM_OPS(ad5820_pm_ops, ad5820_suspend, ad5820_resume);
> +
> +static struct i2c_driver ad5820_i2c_driver = {
> + .driver = {
> + .name = AD5820_NAME,
> + .pm = &ad5820_pm_ops,
> + },
> + .probe = ad5820_probe,
> + .remove = __exit_p(ad5820_remove),
> + .id_table = ad5820_id_table,
> +};
> +
> +module_i2c_driver(ad5820_i2c_driver);
> +
> +MODULE_AUTHOR("Tuukka Toivonen <tuukka.o.toivonen@xxxxxxxxx>");
I think Tuukka's e-mail is wrong here. tuukkat76@xxxxxxxxx? Cc Tuukka.
> +MODULE_DESCRIPTION("AD5820 camera lens driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/media/ad5820.h b/include/media/ad5820.h
> new file mode 100644
> index 0000000..2c28131
> --- /dev/null
> +++ b/include/media/ad5820.h
> @@ -0,0 +1,30 @@
> +/*
> + * AD5820 DAC driver for camera voice coil focus.
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + * Copyright (C) 2007 Texas Instruments
> + *
> + * Contact: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + * Sakari Ailus <sakari.ailus@xxxxxx>
> + *
> + * 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.
> + */
> +
> +#ifndef __AD5820_H__
> +#define __AD5820_H__
> +
> +#define AD5820_NAME "ad5820"
> +#define AD5820_I2C_ADDR (0x18 >> 1)
> +
> +struct ad5820_platform_data {
> + int xshutdown;
> +};
> +
> +#endif /* __AD5820_H__ */
The patch itself looks fine, but I think the bigger question is: what's the
interface for such low-level lens control chips?
Based on what I've seen in the past, the vast majority of these are very
simple but a yet diverse set of devices in terms of how they are controlled.
So until we've got more of them the private controls are probably best we
can do.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
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
