Re: [PATCH 2/2] media: i2c: dw9768: Add DW9768 VCM driver

Hi Dongchun,

On Tue, Sep 3, 2019 at 12:02 AM Dongchun Zhu [off-list ref] wrote:

Hi Tomasz,

On Fri, 2019-08-23 at 17:17 +0900, Tomasz Figa wrote:

quoted

Hi Dongchun,

On Mon, Jul 08, 2019 at 06:06:41PM +0800, dongchun.zhu@mediatek.com wrote:

quoted

From: Dongchun Zhu <redacted>

This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
and provides control to set the desired focus.

The DW9807 is a 10 bit DAC from Dongwoon, designed for linear
control of voice coil motor.

Signed-off-by: Dongchun Zhu <redacted>
---
 MAINTAINERS                |   1 +
 drivers/media/i2c/Kconfig  |  10 +
 drivers/media/i2c/Makefile |   1 +
 drivers/media/i2c/dw9768.c | 458 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 470 insertions(+)
 create mode 100644 drivers/media/i2c/dw9768.c

Thanks for the patch! Please see my comments inline.

quoted

diff --git a/MAINTAINERS b/MAINTAINERS
index 8f6ac93..17152d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS

@@ -4877,6 +4877,7 @@ M:    Dongchun Zhu <dongchun.zhu@mediatek.com>
 L: linux-media@vger.kernel.org
 T: git git://linuxtv.org/media_tree.git
 S: Maintained
+F: drivers/media/i2c/dw9768.c
 F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.txt

 DONGWOON DW9807 LENS VOICE COIL DRIVER

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 7793358..8ff6c95 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig

@@ -1014,6 +1014,16 @@ config VIDEO_DW9714
      capability. This is designed for linear control of
      voice coil motors, controlled via I2C serial interface.

+config VIDEO_DW9768
+   tristate "DW9768 lens voice coil support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   help
+     This is a driver for the DW9768 camera lens voice coil.
+     DW9768 is a 10 bit DAC with 100mA output current sink
+     capability. This is designed for linear control of
+     voice coil motors, controlled via I2C serial interface.
+
 config VIDEO_DW9807_VCM
    tristate "DW9807 lens voice coil support"
    depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER

diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index d8ad9da..944fbf6 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile

@@ -24,6 +24,7 @@ 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_DW9768)  += dw9768.o
 obj-$(CONFIG_VIDEO_DW9807_VCM)  += dw9807-vcm.o
 obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o
 obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o

diff --git a/drivers/media/i2c/dw9768.c b/drivers/media/i2c/dw9768.c
new file mode 100644
index 0000000..f5b5591
--- /dev/null
+++ b/drivers/media/i2c/dw9768.c

@@ -0,0 +1,458 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+#define DW9768_VOLTAGE_ANALOG                      2800000

This is a platform detail and should be defined in the platform data, for
example DTS on platforms using DT.

Thanks for your reminder.
This would be fixed in next release.

quoted

quoted

+#define DW9768_NAME                                "dw9768"

The chip we seem to be using this driver for is called gt9769. Shouldn't we
call the driver the same?

It is also called DW9768 from camera module specification, which was
initially confirmed with vendor.

Okay, thanks for clarifying.

Best regards,
Tomasz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel