[RESEND PATCH V2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[RESEND PATCH V2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

Add device tree bindings for AKM ak7375 voice coil lens
driver. This chip is used to drive a lens in a camera module.

Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com>
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
Changes since v1:
 - remove the vendor prefix change
---
 Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
new file mode 100644
index 000000000000..aa3e24b41241
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ak7375.txt
@@ -0,0 +1,8 @@
+Asahi Kasei Microdevices AK7375 voice coil lens driver
+
+AK7375 is a camera voice coil lens.
+
+Mandatory properties:
+
+- compatible: "asahi-kasei,ak7375"
+- reg: I2C slave address
-- 
1.9.1

[RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[bingbu.cao]

From: Bingbu Cao <bingbu.cao@intel.com>

Add a V4L2 sub-device driver for the ak7375 lens voice coil.
This is a voice coil module using the I2C bus to control the
focus position.

Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com>
Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
---
 MAINTAINERS                |   8 ++
 drivers/media/i2c/Kconfig  |  10 ++
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/ak7375.c | 278 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 297 insertions(+)
 create mode 100644 drivers/media/i2c/ak7375.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea362219c4aa..20379a7baca0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,14 @@ T:	git git://linuxtv.org/anttip/media_tree.git
 S:	Maintained
 F:	drivers/media/usb/airspy/
 
+AKM AK7375 LENS VOICE COIL DRIVER
+M:	Tianshu Qiu <tian.shu.qiu@intel.com>
+L:	linux-media@vger.kernel.org
+T:	git git://linuxtv.org/media_tree.git
+S:	Maintained
+F:	drivers/media/i2c/ak7375.c
+F:	Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
+
 ALACRITECH GIGABIT ETHERNET DRIVER
 M:	Lino Sanfilippo <LinoSanfilippo@gmx.de>
 S:	Maintained
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 341452fe98df..ff3cb5afb0e1 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -326,6 +326,16 @@ config VIDEO_AD5820
 	  This is a driver for the AD5820 camera lens voice coil.
 	  It is used for example in Nokia N900 (RX-51).
 
+config VIDEO_AK7375
+	tristate "AK7375 lens voice coil support"
+	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+	depends on VIDEO_V4L2_SUBDEV_API
+	help
+	  This is a driver for the AK7375 camera lens voice coil.
+	  AK7375 is a 12 bit DAC with 120mA output current sink
+	  capability. This is designed for linear control of
+	  voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9714
 	tristate "DW9714 lens voice coil support"
 	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d679d57cd3b3..05b97e319ea9 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
 obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
 obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
 obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
+obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
 obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
new file mode 100644
index 000000000000..012e673e9ced
--- /dev/null
+++ b/drivers/media/i2c/ak7375.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018 Intel Corporation
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+
+#define AK7375_MAX_FOCUS_POS	4095
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define AK7375_FOCUS_STEPS	1
+/*
+ * This acts as the minimum granularity of lens movement.
+ * Keep this value power of 2, so the control steps can be
+ * uniformly adjusted for gradual lens movement, with desired
+ * number of control steps.
+ */
+#define AK7375_CTRL_STEPS	64
+#define AK7375_CTRL_DELAY_US	1000
+
+#define AK7375_REG_POSITION	0x0
+#define AK7375_REG_CONT		0x2
+#define AK7375_MODE_ACTIVE	0x0
+#define AK7375_MODE_STANDBY	0x40
+
+/* ak7375 device structure */
+struct ak7375_device {
+	struct v4l2_ctrl_handler ctrls_vcm;
+	struct v4l2_subdev sd;
+	struct v4l2_ctrl *focus;
+};
+
+static inline struct ak7375_device *to_ak7375_vcm(struct v4l2_ctrl *ctrl)
+{
+	return container_of(ctrl->handler, struct ak7375_device, ctrls_vcm);
+}
+
+static inline struct ak7375_device *sd_to_ak7375_vcm(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct ak7375_device, sd);
+}
+
+static int ak7375_i2c_write(struct ak7375_device *ak7375,
+	u8 addr, u16 data, int size)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&ak7375->sd);
+	int ret;
+	u8 buf[3];
+
+	if (size != 1 && size != 2)
+		return -EINVAL;
+	buf[0] = addr;
+	buf[2] = data & 0xff;
+	if (size == 2)
+		buf[1] = data >> 8;
+	ret = i2c_master_send(client, (const char *)buf, size + 1);
+	if (ret < 0)
+		return ret;
+	if (ret != size + 1)
+		return -EIO;
+	return 0;
+}
+
+static int ak7375_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct ak7375_device *dev_vcm = to_ak7375_vcm(ctrl);
+
+	if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+		return ak7375_i2c_write(dev_vcm, AK7375_REG_POSITION,
+					ctrl->val << 4, 2);
+
+	return -EINVAL;
+}
+
+static const struct v4l2_ctrl_ops ak7375_vcm_ctrl_ops = {
+	.s_ctrl = ak7375_set_ctrl,
+};
+
+static int ak7375_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	int rval;
+
+	rval = pm_runtime_get_sync(sd->dev);
+	if (rval < 0) {
+		pm_runtime_put_noidle(sd->dev);
+		return rval;
+	}
+
+	return 0;
+}
+
+static int ak7375_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+	pm_runtime_put(sd->dev);
+
+	return 0;
+}
+
+static const struct v4l2_subdev_internal_ops ak7375_int_ops = {
+	.open = ak7375_open,
+	.close = ak7375_close,
+};
+
+static const struct v4l2_subdev_ops ak7375_ops = { };
+
+static void ak7375_subdev_cleanup(struct ak7375_device *ak7375_dev)
+{
+	v4l2_async_unregister_subdev(&ak7375_dev->sd);
+	v4l2_ctrl_handler_free(&ak7375_dev->ctrls_vcm);
+	media_entity_cleanup(&ak7375_dev->sd.entity);
+}
+
+static int ak7375_init_controls(struct ak7375_device *dev_vcm)
+{
+	struct v4l2_ctrl_handler *hdl = &dev_vcm->ctrls_vcm;
+	const struct v4l2_ctrl_ops *ops = &ak7375_vcm_ctrl_ops;
+
+	v4l2_ctrl_handler_init(hdl, 1);
+
+	dev_vcm->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
+		0, AK7375_MAX_FOCUS_POS, AK7375_FOCUS_STEPS, 0);
+
+	if (hdl->error)
+		dev_err(dev_vcm->sd.dev, "%s fail error: 0x%x\n",
+			__func__, hdl->error);
+	dev_vcm->sd.ctrl_handler = hdl;
+	return hdl->error;
+}
+
+static int ak7375_probe(struct i2c_client *client)
+{
+	struct ak7375_device *ak7375_dev;
+	int val;
+
+	ak7375_dev = devm_kzalloc(&client->dev, sizeof(*ak7375_dev),
+				  GFP_KERNEL);
+	if (!ak7375_dev)
+		return -ENOMEM;
+
+	v4l2_i2c_subdev_init(&ak7375_dev->sd, client, &ak7375_ops);
+	ak7375_dev->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	ak7375_dev->sd.internal_ops = &ak7375_int_ops;
+	ak7375_dev->sd.entity.function = MEDIA_ENT_F_LENS;
+
+	val = ak7375_init_controls(ak7375_dev);
+	if (val)
+		goto err_cleanup;
+
+	val = media_entity_pads_init(&ak7375_dev->sd.entity, 0, NULL);
+	if (val < 0)
+		goto err_cleanup;
+
+	val = v4l2_async_register_subdev(&ak7375_dev->sd);
+	if (val < 0)
+		goto err_cleanup;
+
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_idle(&client->dev);
+
+	return 0;
+
+err_cleanup:
+	v4l2_ctrl_handler_free(&ak7375_dev->ctrls_vcm);
+	media_entity_cleanup(&ak7375_dev->sd.entity);
+	dev_err(&client->dev, "Probe failed: %d\n", val);
+	return val;
+}
+
+static int ak7375_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ak7375_device *ak7375_dev = sd_to_ak7375_vcm(sd);
+
+	ak7375_subdev_cleanup(ak7375_dev);
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+
+	return 0;
+}
+
+/*
+ * This function sets the vcm position, so it consumes least current
+ * The lens position is gradually moved in units of AK7375_CTRL_STEPS,
+ * to make the movements smoothly.
+ */
+static int __maybe_unused ak7375_vcm_suspend(struct device *dev)
+{
+
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ak7375_device *ak7375_dev = sd_to_ak7375_vcm(sd);
+	int ret, val;
+
+	for (val = ak7375_dev->focus->val & ~(AK7375_CTRL_STEPS - 1);
+	     val >= 0; val -= AK7375_CTRL_STEPS) {
+		ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_POSITION,
+				       val << 4, 2);
+		if (ret)
+			dev_err_once(dev, "%s I2C failure: %d\n",
+				     __func__, ret);
+		usleep_range(AK7375_CTRL_DELAY_US, AK7375_CTRL_DELAY_US + 10);
+	}
+
+	ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
+			       AK7375_MODE_STANDBY, 1);
+	if (ret) {
+		dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
+		return ret;
+	}
+	return 0;
+}
+
+/*
+ * This function sets the vcm position to the value set by the user
+ * through v4l2_ctrl_ops s_ctrl handler
+ * The lens position is gradually moved in units of AK7375_CTRL_STEPS,
+ * to make the movements smoothly.
+ */
+static int __maybe_unused ak7375_vcm_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ak7375_device *ak7375_dev = sd_to_ak7375_vcm(sd);
+	int ret, val;
+
+	ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_CONT,
+		AK7375_MODE_ACTIVE, 1);
+	if (ret) {
+		dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
+		return ret;
+	}
+
+	for (val = ak7375_dev->focus->val % AK7375_CTRL_STEPS;
+	     val <= ak7375_dev->focus->val;
+	     val += AK7375_CTRL_STEPS) {
+		ret = ak7375_i2c_write(ak7375_dev, AK7375_REG_POSITION,
+				       val << 4, 2);
+		if (ret)
+			dev_err_ratelimited(dev, "%s I2C failure: %d\n",
+						__func__, ret);
+		usleep_range(AK7375_CTRL_DELAY_US, AK7375_CTRL_DELAY_US + 10);
+	}
+	return 0;
+}
+
+static const struct of_device_id ak7375_of_table[] = {
+	{ .compatible = "akm,ak7375" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ak7375_of_table);
+
+static const struct dev_pm_ops ak7375_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ak7375_vcm_suspend, ak7375_vcm_resume)
+	SET_RUNTIME_PM_OPS(ak7375_vcm_suspend, ak7375_vcm_resume, NULL)
+};
+
+static struct i2c_driver ak7375_i2c_driver = {
+	.driver = {
+		.name = "ak7375",
+		.pm = &ak7375_pm_ops,
+		.of_match_table = ak7375_of_table,
+	},
+	.probe_new = ak7375_probe,
+	.remove = ak7375_remove,
+};
+module_i2c_driver(ak7375_i2c_driver);
+
+MODULE_AUTHOR("Tianshu Qiu <tian.shu.qiu@intel.com>");
+MODULE_AUTHOR("Bingbu Cao <bingbu.cao@intel.com>");
+MODULE_DESCRIPTION("AK7375 VCM driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [RESEND PATCH V2 1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens

[Rob Herring]

On Fri, May 25, 2018 at 05:55:34PM +0800, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> Add device tree bindings for AKM ak7375 voice coil lens
> driver. This chip is used to drive a lens in a camera module.
> 
> Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
> Changes since v1:
>  - remove the vendor prefix change
> ---
>  Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[Sakari Ailus]

Hi Bingbu,

A few comments below.

On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> Add a V4L2 sub-device driver for the ak7375 lens voice coil.
> This is a voice coil module using the I2C bus to control the
> focus position.
> 
> Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com>
> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
> ---
>  MAINTAINERS                |   8 ++
>  drivers/media/i2c/Kconfig  |  10 ++
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ak7375.c | 278 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 297 insertions(+)
>  create mode 100644 drivers/media/i2c/ak7375.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ea362219c4aa..20379a7baca0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -625,6 +625,14 @@ T:	git git://linuxtv.org/anttip/media_tree.git
>  S:	Maintained
>  F:	drivers/media/usb/airspy/
>  
> +AKM AK7375 LENS VOICE COIL DRIVER
> +M:	Tianshu Qiu <tian.shu.qiu@intel.com>
> +L:	linux-media@vger.kernel.org
> +T:	git git://linuxtv.org/media_tree.git
> +S:	Maintained
> +F:	drivers/media/i2c/ak7375.c
> +F:	Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt

The name of the file also needs to match. Currently it doesn't. How about
"asahi-kasei,ak7375.txt"?

Could you also move the MAINTAINERS entry to the patch adding the DT
bindings?

> +
>  ALACRITECH GIGABIT ETHERNET DRIVER
>  M:	Lino Sanfilippo <LinoSanfilippo@gmx.de>
>  S:	Maintained
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 341452fe98df..ff3cb5afb0e1 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -326,6 +326,16 @@ config VIDEO_AD5820
>  	  This is a driver for the AD5820 camera lens voice coil.
>  	  It is used for example in Nokia N900 (RX-51).
>  
> +config VIDEO_AK7375
> +	tristate "AK7375 lens voice coil support"
> +	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> +	depends on VIDEO_V4L2_SUBDEV_API
> +	help
> +	  This is a driver for the AK7375 camera lens voice coil.
> +	  AK7375 is a 12 bit DAC with 120mA output current sink
> +	  capability. This is designed for linear control of
> +	  voice coil motors, controlled via I2C serial interface.
> +
>  config VIDEO_DW9714
>  	tristate "DW9714 lens voice coil support"
>  	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index d679d57cd3b3..05b97e319ea9 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
>  obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>  obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
>  obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
> +obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
>  obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
>  obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>  obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
> new file mode 100644
> index 000000000000..012e673e9ced
> --- /dev/null
> +++ b/drivers/media/i2c/ak7375.c

...

> +static const struct of_device_id ak7375_of_table[] = {
> +	{ .compatible = "akm,ak7375" },

"asahi-kasei,ak7375"

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[Sakari Ailus]

On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote:
> +static int ak7375_i2c_write(struct ak7375_device *ak7375,
> +	u8 addr, u16 data, int size)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ak7375->sd);
> +	int ret;
> +	u8 buf[3];
> +
> +	if (size != 1 && size != 2)
> +		return -EINVAL;
> +	buf[0] = addr;
> +	buf[2] = data & 0xff;
> +	if (size == 2)
> +		buf[1] = data >> 8;
> +	ret = i2c_master_send(client, (const char *)buf, size + 1);

I don't have a data datasheet for this thing, but it looks like buf[1] will
be undefined for writes the size of which is 1. And this what appears to be
written to the device as well...

> +	if (ret < 0)
> +		return ret;
> +	if (ret != size + 1)
> +		return -EIO;
> +	return 0;
> +}

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[Bing Bu Cao]

On 2018年06月01日 17:34, Sakari Ailus wrote:
> Hi Bingbu,
>
> A few comments below.
>
> On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> Add a V4L2 sub-device driver for the ak7375 lens voice coil.
>> This is a voice coil module using the I2C bus to control the
>> focus position.
>>
>> Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com>
>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>> ---
>>   MAINTAINERS                |   8 ++
>>   drivers/media/i2c/Kconfig  |  10 ++
>>   drivers/media/i2c/Makefile |   1 +
>>   drivers/media/i2c/ak7375.c | 278 +++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 297 insertions(+)
>>   create mode 100644 drivers/media/i2c/ak7375.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ea362219c4aa..20379a7baca0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -625,6 +625,14 @@ T:	git git://linuxtv.org/anttip/media_tree.git
>>   S:	Maintained
>>   F:	drivers/media/usb/airspy/
>>   
>> +AKM AK7375 LENS VOICE COIL DRIVER
>> +M:	Tianshu Qiu <tian.shu.qiu@intel.com>
>> +L:	linux-media@vger.kernel.org
>> +T:	git git://linuxtv.org/media_tree.git
>> +S:	Maintained
>> +F:	drivers/media/i2c/ak7375.c
>> +F:	Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
> The name of the file also needs to match. Currently it doesn't. How about
> "asahi-kasei,ak7375.txt"?
Ack.
Does it make sense if just change the compatible string to 
"asahi-kasei,ak7375" and keep the file name unchanged?
I have no clear idea about the DT binding rules.
>
> Could you also move the MAINTAINERS entry to the patch adding the DT
> bindings?
Ack.
>
>> +
>>   ALACRITECH GIGABIT ETHERNET DRIVER
>>   M:	Lino Sanfilippo <LinoSanfilippo@gmx.de>
>>   S:	Maintained
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 341452fe98df..ff3cb5afb0e1 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -326,6 +326,16 @@ config VIDEO_AD5820
>>   	  This is a driver for the AD5820 camera lens voice coil.
>>   	  It is used for example in Nokia N900 (RX-51).
>>   
>> +config VIDEO_AK7375
>> +	tristate "AK7375 lens voice coil support"
>> +	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> +	depends on VIDEO_V4L2_SUBDEV_API
>> +	help
>> +	  This is a driver for the AK7375 camera lens voice coil.
>> +	  AK7375 is a 12 bit DAC with 120mA output current sink
>> +	  capability. This is designed for linear control of
>> +	  voice coil motors, controlled via I2C serial interface.
>> +
>>   config VIDEO_DW9714
>>   	tristate "DW9714 lens voice coil support"
>>   	depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index d679d57cd3b3..05b97e319ea9 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o
>>   obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o
>>   obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o
>>   obj-$(CONFIG_VIDEO_AD5820)  += ad5820.o
>> +obj-$(CONFIG_VIDEO_AK7375)  += ak7375.o
>>   obj-$(CONFIG_VIDEO_DW9714)  += dw9714.o
>>   obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
>>   obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o
>> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
>> new file mode 100644
>> index 000000000000..012e673e9ced
>> --- /dev/null
>> +++ b/drivers/media/i2c/ak7375.c
> ...
>
>> +static const struct of_device_id ak7375_of_table[] = {
>> +	{ .compatible = "akm,ak7375" },
> "asahi-kasei,ak7375"
>

Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[Sakari Ailus]

On Fri, Jun 01, 2018 at 05:54:30PM +0800, Bing Bu Cao wrote:
> > > +AKM AK7375 LENS VOICE COIL DRIVER
> > > +M:	Tianshu Qiu <tian.shu.qiu@intel.com>
> > > +L:	linux-media@vger.kernel.org
> > > +T:	git git://linuxtv.org/media_tree.git
> > > +S:	Maintained
> > > +F:	drivers/media/i2c/ak7375.c
> > > +F:	Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt
> > The name of the file also needs to match. Currently it doesn't. How about
> > "asahi-kasei,ak7375.txt"?
> Ack.
> Does it make sense if just change the compatible string to
> "asahi-kasei,ak7375" and keep the file name unchanged?

Most binding files in the directory seem to use the chip name rather than
the full compatible string (including the vendor). I guess both are fine.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RESEND PATCH V2 2/2] media: ak7375: Add ak7375 lens voice coil driver

[Bing Bu Cao]

On 2018年06月01日 17:42, Sakari Ailus wrote:
> On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote:
>> +static int ak7375_i2c_write(struct ak7375_device *ak7375,
>> +	u8 addr, u16 data, int size)
>> +{
>> +	struct i2c_client *client = v4l2_get_subdevdata(&ak7375->sd);
>> +	int ret;
>> +	u8 buf[3];
>> +
>> +	if (size != 1 && size != 2)
>> +		return -EINVAL;
>> +	buf[0] = addr;
>> +	buf[2] = data & 0xff;
>> +	if (size == 2)
>> +		buf[1] = data >> 8;
>> +	ret = i2c_master_send(client, (const char *)buf, size + 1);
> I don't have a data datasheet for this thing, but it looks like buf[1] will
> be undefined for writes the size of which is 1. And this what appears to be
> written to the device as well...
I check the datasheet once again and find out that the logic here for 
size==1 is
not correct, I will change it in next version. Thanks.
Here is the write section in datasheet:

------
After receiving the second byte (register address), AK7375 generates an 
acknowledge then receives the third
byte. The third and the following bytes represent control data. Control 
data consists of 8 bits and is based on
the MSB-first configuration.
AK7375 can write multiple bytes of data at a time. After reception of 
the third byte (control data), AK7375
generates an acknowledge then receives the next data. If additional data 
is received instead of the stop
condition after receiving one byte of data, the address counter inside 
the LSI chip is automatically
incremented and the data is written at the next address.
------

>
>> +	if (ret < 0)
>> +		return ret;
>> +	if (ret != size + 1)
>> +		return -EIO;
>> +	return 0;
>> +}