* [V3, 0/2] media: i2c: add support for DW9768 VCM driver
@ 2020-02-28 15:59 Dongchun Zhu
2020-02-28 15:59 ` [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry Dongchun Zhu
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
0 siblings, 2 replies; 17+ messages in thread
From: Dongchun Zhu @ 2020-02-28 15:59 UTC (permalink / raw)
To: mchehab, andriy.shevchenko, robh+dt, mark.rutland, sakari.ailus,
drinkcat, tfiga, matthias.bgg, bingbu.cao
Cc: devicetree, srv_heupstream, shengnan.wang, sj.huang,
linux-mediatek, dongchun.zhu, louis.kuo, linux-arm-kernel,
linux-media
Hello,
Add DT bindings in YAML and v4l2 driver for DW9768 lens voice coil actuator.
This is a 10-bit DAC with 100mA output current sink capability from Dongwoon,
designed for linear control of voice coil motor, and controlled via I2C serial
interface to set the desired focus position.
The DW9768 controls the position with 10-bit DAC data D[9:0] and seperates
two 8-bit registers to control the VCM position as belows.
DAC_MSB: D[9:8] (ADD: 0x03)
+---+---+---+---+---+---+---+---+
|---|---|---|---|---|---|D09|D08|
+---+---+---+---+---+---+---+---+
DAC_LSB: D[7:0] (ADD: 0x04)
+---+---+---+---+---+---+---+---+
|D07|D06|D05|D04|D03|D02|D01|D00|
+---+---+---+---+---+---+---+---+
This driver supports:
- set DW9768 to standby mode once suspend and turn it back to active if resume
- set the position via V4L2_CID_FOCUS_ABSOLUTE ctrl
Changes of v3 are mainly addressing comments from Andy, Rob, Sakari, Tomasz,
compared to v2:
- Rebase onto 5.6-rc1
- Convert text documentation to YAML schema
- Add documents for the register addresses and bits in the registers
- Merge _power_off/on with runtime PM suspend/resume function
- Drop the I2C ID table
- Refine DW9768 power sequencing timing
- Use the regulator bulk API to enable/disable regulators
- Change i2c_smbus_write_block_data() to i2c_smbus_write_word_data()
- Fixup coding style and improve code quality
- Fix other reviewed issues in V2
Mainly changes of v2 are addressing the comments from Tomasz, Bingbu, Andy,
including,
- Use i2c_smbus_write_byte_data to replace of the custom dw9768_i2c_write
- Use i2c_smbus_write_block_data to set vcm postion
- Use the runtime PM suspend/resume callbacks to power off/on
- Check the PM runtime status before powering off in dw9768_remove function
- Add one more regulator vin for the I2C interface
- Remove or refine log print
- Fix other reviewed issues in v1
Dongchun Zhu (2):
media: i2c: dw9768: Add DT support and MAINTAINERS entry
media: i2c: Add DW9768 VCM driver
.../bindings/media/i2c/dongwoon,dw9768.yaml | 55 +++
MAINTAINERS | 8 +
drivers/media/i2c/Kconfig | 10 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++
5 files changed, 511 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
create mode 100644 drivers/media/i2c/dw9768.c
--
2.9.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry
2020-02-28 15:59 [V3, 0/2] media: i2c: add support for DW9768 VCM driver Dongchun Zhu
@ 2020-02-28 15:59 ` Dongchun Zhu
2020-03-02 18:54 ` Rob Herring
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
1 sibling, 1 reply; 17+ messages in thread
From: Dongchun Zhu @ 2020-02-28 15:59 UTC (permalink / raw)
To: mchehab, andriy.shevchenko, robh+dt, mark.rutland, sakari.ailus,
drinkcat, tfiga, matthias.bgg, bingbu.cao
Cc: devicetree, srv_heupstream, shengnan.wang, sj.huang,
linux-mediatek, dongchun.zhu, louis.kuo, linux-arm-kernel,
linux-media
This patch is to add the Devicetree binding documentation and
MAINTAINERS entry for dw9768 actuator.
Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
.../bindings/media/i2c/dongwoon,dw9768.yaml | 55 ++++++++++++++++++++++
MAINTAINERS | 7 +++
2 files changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
new file mode 100644
index 0000000..55f7c29
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (c) 2020 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
+
+maintainers:
+ - Dongchun Zhu <dongchun.zhu@mediatek.com>
+
+description: |-
+ The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
+ with 100 mA output current sink capability. VCM current is controlled with
+ a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
+ serial interface that operates at clock rates up to 1MHz. This chip
+ integrates Advanced Actuator Control (AAC) technology and is intended for
+ driving voice coil lenses in camera modules.
+
+properties:
+ compatible:
+ const: dongwoon,dw9768
+
+ reg:
+ maxItems: 1
+
+ vin-supply:
+ description:
+ Definition of the regulator used as I2C I/O interface power supply.
+ maxItems: 1
+
+ vdd-supply:
+ description:
+ Definition of the regulator used as VCM chip power supply.
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - vin-supply
+ - vdd-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ dw9768: camera-lens@0c {
+ compatible = "dongwoon,dw9768";
+ reg = <0x0c>;
+ vin-supply = <&mt6358_vcamio_reg>;
+ vdd-supply = <&mt6358_vcama2_reg>;
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 38fe2f3..b805e29 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5134,6 +5134,13 @@ S: Maintained
F: drivers/media/i2c/dw9714.c
F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9714.txt
+DONGWOON DW9768 LENS VOICE COIL DRIVER
+M: Dongchun Zhu <dongchun.zhu@mediatek.com>
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
+
DONGWOON DW9807 LENS VOICE COIL DRIVER
M: Sakari Ailus <sakari.ailus@linux.intel.com>
L: linux-media@vger.kernel.org
--
2.9.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-02-28 15:59 [V3, 0/2] media: i2c: add support for DW9768 VCM driver Dongchun Zhu
2020-02-28 15:59 ` [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry Dongchun Zhu
@ 2020-02-28 15:59 ` Dongchun Zhu
2020-03-02 11:07 ` Andy Shevchenko
` (2 more replies)
1 sibling, 3 replies; 17+ messages in thread
From: Dongchun Zhu @ 2020-02-28 15:59 UTC (permalink / raw)
To: mchehab, andriy.shevchenko, robh+dt, mark.rutland, sakari.ailus,
drinkcat, tfiga, matthias.bgg, bingbu.cao
Cc: devicetree, srv_heupstream, shengnan.wang, sj.huang,
linux-mediatek, dongchun.zhu, louis.kuo, linux-arm-kernel,
linux-media
This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
and provides control to set the desired focus via I2C serial interface.
Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
MAINTAINERS | 1 +
drivers/media/i2c/Kconfig | 10 ++
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 449 insertions(+)
create mode 100644 drivers/media/i2c/dw9768.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b805e29..0bb894a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5139,6 +5139,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.yaml
DONGWOON DW9807 LENS VOICE COIL DRIVER
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index c68e002..aa60781 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
--- /dev/null
+++ b/drivers/media/i2c/dw9768.c
@@ -0,0 +1,437 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 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-async.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+#define DW9768_NAME "dw9768"
+#define DW9768_MAX_FOCUS_POS 1023
+/*
+ * This sets the minimum granularity for the focus positions.
+ * A value of 1 gives maximum accuracy for a desired focus position
+ */
+#define DW9768_FOCUS_STEPS 1
+
+/*
+ * Ring control and Power control register
+ * Bit[1] RING_EN
+ * 0: Direct mode
+ * 1: AAC mode (ringing control mode)
+ * Bit[0] PD
+ * 0: Normal operation mode
+ * 1: Power down mode
+ * DW9768 requires waiting time of Topr after PD reset takes place.
+ */
+#define DW9768_RING_PD_CONTROL_REG 0x02
+#define DW9768_PD_MODE_OFF 0x00
+#define DW9768_PD_MODE_EN BIT(0)
+#define DW9768_AAC_MODE_EN BIT(1)
+
+/*
+ * DW9768 separates two registers to control the VCM position.
+ * One for MSB value, another is LSB value.
+ * DAC_MSB: D[9:8] (ADD: 0x03)
+ * DAC_LSB: D[7:0] (ADD: 0x04)
+ * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
+ */
+#define DW9768_MSB_ADDR 0x03
+#define DW9768_LSB_ADDR 0x04
+#define DW9768_STATUS_ADDR 0x05
+
+/*
+ * AAC mode control & prescale register
+ * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
+ * 000 Direct(default)
+ * 001 AAC2 0.48xTvib
+ * 010 AAC3 0.70xTvib
+ * 011 AAC4 0.75xTvib
+ * 100 Reserved
+ * 101 AAC8 1.13xTvib
+ * 110 Reserved
+ * 111 Reserved
+ * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
+ * 000 2
+ * 001 1(default)
+ * 010 1/2
+ * 011 1/4
+ * 100 8
+ * 101 4
+ * 110 Reserved
+ * 111 Reserved
+ */
+#define DW9768_AAC_PRESC_REG 0x06
+#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
+
+/*
+ * VCM period of vibration register
+ * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
+ * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
+ * Dividing Rate is the internal clock dividing rate that is deifned at
+ * PRESCALE register (ADD: 0x06)
+ */
+#define DW9768_AAC_TIME_REG 0x07
+#define DW9768_AACT_CNT 0x39
+
+/*
+ * DW9768 requires waiting time (delay time) of t_OPR after power-up,
+ * or in the case of PD reset taking place.
+ */
+#define DW9768_T_OPR_US 1000
+
+/*
+ * 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 DW9768_MOVE_STEPS 16
+
+/*
+ * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
+ * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
+ * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
+ */
+#define DW9768_MOVE_DELAY_US 8400
+#define DW9768_STABLE_TIME_US 20000
+
+static const char * const dw9768_supply_names[] = {
+ "vin", /* I2C I/O interface power */
+ "vdd", /* VCM power */
+};
+
+#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
+
+/* dw9768 device structure */
+struct dw9768 {
+ struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
+ struct v4l2_ctrl_handler ctrls;
+ struct v4l2_ctrl *focus;
+ struct v4l2_subdev sd;
+};
+
+static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
+{
+ return container_of(ctrl->handler, struct dw9768, ctrls);
+}
+
+static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
+{
+ return container_of(subdev, struct dw9768, sd);
+}
+
+struct regval_list {
+ u8 reg_num;
+ u8 value;
+};
+
+static struct regval_list dw9768_init_regs[] = {
+ {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
+ {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
+ {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
+};
+
+static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
+ size_t len)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < len; i++) {
+ ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
+ vals[i].value);
+ if (ret < 0)
+ return ret;
+ }
+ return 0;
+}
+
+static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+
+ /* Write VCM position to registers */
+ return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
+ swab16(val));
+}
+
+static int dw9768_init(struct dw9768 *dw9768)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ int ret, val;
+
+ /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
+ ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+ DW9768_PD_MODE_OFF);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * DW9769 requires waiting delay time of t_OPR
+ * after PD reset takes place.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ ret = dw9768_write_array(dw9768, dw9768_init_regs,
+ ARRAY_SIZE(dw9768_init_regs));
+ if (ret)
+ return ret;
+
+ for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
+ val <= dw9768->focus->val;
+ val += DW9768_MOVE_STEPS) {
+ ret = dw9768_set_dac(dw9768, val);
+ if (ret) {
+ dev_err(&client->dev, "%s I2C failure: %d",
+ __func__, ret);
+ return ret;
+ }
+ usleep_range(DW9768_MOVE_DELAY_US,
+ DW9768_MOVE_DELAY_US + 1000);
+ }
+
+ return 0;
+}
+
+static int dw9768_release(struct dw9768 *dw9768)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
+ int ret, val;
+
+ for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
+ val >= 0; val -= DW9768_MOVE_STEPS) {
+ ret = dw9768_set_dac(dw9768, val);
+ if (ret) {
+ dev_err(&client->dev, "%s I2C failure: %d",
+ __func__, ret);
+ return ret;
+ }
+ usleep_range(DW9768_MOVE_DELAY_US,
+ DW9768_MOVE_DELAY_US + 1000);
+ }
+
+ /*
+ * Wait for the motor to stabilize after the last movement
+ * to prevent the motor from shaking.
+ */
+ usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
+ DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
+
+ ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
+ DW9768_PD_MODE_EN);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * DW9769 requires waiting delay time of t_OPR
+ * after PD reset takes place.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ return 0;
+}
+
+/* Power handling */
+static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+ dw9768_release(dw9768);
+ regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
+
+ return 0;
+}
+
+static int __maybe_unused dw9768_runtime_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+ int ret;
+
+ ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable regulators\n");
+ return ret;
+ }
+
+ /*
+ * The datasheet refers to t_OPR that needs to be waited before sending
+ * I2C commands after power-up.
+ */
+ usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
+
+ ret = dw9768_init(dw9768);
+ if (ret < 0)
+ goto disable_regulator;
+
+ return 0;
+
+disable_regulator:
+ regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
+
+ return ret;
+}
+
+static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+ struct dw9768 *dw9768 = to_dw9768(ctrl);
+
+ if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
+ return dw9768_set_dac(dw9768, ctrl->val);
+
+ return 0;
+}
+
+static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
+ .s_ctrl = dw9768_set_ctrl,
+};
+
+static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ int ret;
+
+ ret = pm_runtime_get_sync(sd->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(sd->dev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+ pm_runtime_put(sd->dev);
+
+ return 0;
+}
+
+static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
+ .open = dw9768_open,
+ .close = dw9768_close,
+};
+
+static const struct v4l2_subdev_ops dw9768_ops = { };
+
+static int dw9768_init_controls(struct dw9768 *dw9768)
+{
+ struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
+ const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
+
+ v4l2_ctrl_handler_init(hdl, 1);
+
+ dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
+ 0, DW9768_MAX_FOCUS_POS,
+ DW9768_FOCUS_STEPS, 0);
+
+ if (hdl->error)
+ return hdl->error;
+
+ dw9768->sd.ctrl_handler = hdl;
+
+ return 0;
+}
+
+static int dw9768_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct dw9768 *dw9768;
+ unsigned int i;
+ int ret;
+
+ dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
+ if (!dw9768)
+ return -ENOMEM;
+
+ v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
+
+ for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
+ dw9768->supplies[i].supply = dw9768_supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
+ dw9768->supplies);
+ if (ret) {
+ dev_err(dev, "failed to get regulators\n");
+ return ret;
+ }
+
+ ret = dw9768_init_controls(dw9768);
+ if (ret)
+ goto entity_cleanup;
+
+ dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+ dw9768->sd.internal_ops = &dw9768_int_ops;
+
+ ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
+ if (ret < 0)
+ goto entity_cleanup;
+
+ dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
+
+ ret = v4l2_async_register_subdev(&dw9768->sd);
+ if (ret < 0)
+ goto entity_cleanup;
+
+ pm_runtime_enable(dev);
+
+ return 0;
+
+entity_cleanup:
+ v4l2_ctrl_handler_free(&dw9768->ctrls);
+ media_entity_cleanup(&dw9768->sd.entity);
+ return ret;
+}
+
+static int dw9768_remove(struct i2c_client *client)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct dw9768 *dw9768 = sd_to_dw9768(sd);
+
+ pm_runtime_disable(&client->dev);
+ v4l2_async_unregister_subdev(&dw9768->sd);
+ v4l2_ctrl_handler_free(&dw9768->ctrls);
+ media_entity_cleanup(&dw9768->sd.entity);
+ if (!pm_runtime_status_suspended(&client->dev))
+ dw9768_runtime_suspend(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
+
+ return 0;
+}
+
+static const struct of_device_id dw9768_of_table[] = {
+ { .compatible = "dongwoon,dw9768" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, dw9768_of_table);
+
+static const struct dev_pm_ops dw9768_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
+};
+
+static struct i2c_driver dw9768_i2c_driver = {
+ .driver = {
+ .name = DW9768_NAME,
+ .pm = &dw9768_pm_ops,
+ .of_match_table = dw9768_of_table,
+ },
+ .probe_new = dw9768_probe,
+ .remove = dw9768_remove,
+};
+
+module_i2c_driver(dw9768_i2c_driver);
+
+MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
+MODULE_DESCRIPTION("DW9768 VCM driver");
+MODULE_LICENSE("GPL v2");
--
2.9.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
@ 2020-03-02 11:07 ` Andy Shevchenko
2020-03-05 12:05 ` Sakari Ailus
2020-03-10 10:00 ` Dongchun Zhu
2 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-02 11:07 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream,
shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, sakari.ailus, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> and provides control to set the desired focus via I2C serial interface.
...
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5139,6 +5139,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.yaml
Had you run parse-maintainers.pl?
I believe the order is wrong here.
...
> +#define DW9768_MAX_FOCUS_POS 1023
Is this value being dictated by amount of bits available in the hardware?
If so, I would rather put it in a form (1024 - 1) or alike to show that it has
10 bit resolution.
...
> +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> +
> + /* Write VCM position to registers */
> + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> + swab16(val));
i2c_smbus_write_word_swapped() ?
> +}
...
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret, val;
> +
> + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + val >= 0; val -= DW9768_MOVE_STEPS) {
Perhaps
val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
for ( ; val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
Do you need __func__? What for?
> + return ret;
> + }
> + usleep_range(DW9768_MOVE_DELAY_US,
> + DW9768_MOVE_DELAY_US + 1000);
It's exactly one line. Perhaps you have to check your editor settings.
And check entire code for a such.
> + }
> +
> + /*
> + * Wait for the motor to stabilize after the last movement
> + * to prevent the motor from shaking.
> + */
> + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> +
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry
2020-02-28 15:59 ` [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry Dongchun Zhu
@ 2020-03-02 18:54 ` Rob Herring
0 siblings, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-03-02 18:54 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, dongchun.zhu, sakari.ailus, matthias.bgg,
bingbu.cao, mchehab, linux-arm-kernel, linux-media
On Fri, 28 Feb 2020 23:59:57 +0800, Dongchun Zhu wrote:
> This patch is to add the Devicetree binding documentation and
> MAINTAINERS entry for dw9768 actuator.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> .../bindings/media/i2c/dongwoon,dw9768.yaml | 55 ++++++++++++++++++++++
> MAINTAINERS | 7 +++
> 2 files changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.example.dts:19.13-26: Warning (reg_format): /example-0/camera-lens@0c:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
See https://patchwork.ozlabs.org/patch/1246607
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
2020-03-02 11:07 ` Andy Shevchenko
@ 2020-03-05 12:05 ` Sakari Ailus
2020-03-10 10:10 ` Andy Shevchenko
2020-03-18 10:20 ` Dongchun Zhu
2020-03-10 10:00 ` Dongchun Zhu
2 siblings, 2 replies; 17+ messages in thread
From: Sakari Ailus @ 2020-03-05 12:05 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
Hi Dongchun,
Thanks for the update.
On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> and provides control to set the desired focus via I2C serial interface.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 10 ++
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 449 insertions(+)
> create mode 100644 drivers/media/i2c/dw9768.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b805e29..0bb894a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5139,6 +5139,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.yaml
>
> DONGWOON DW9807 LENS VOICE COIL DRIVER
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c68e002..aa60781 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> --- /dev/null
> +++ b/drivers/media/i2c/dw9768.c
> @@ -0,0 +1,437 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 MediaTek Inc.
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pm_runtime.h>
Alphabetical order would be nice.
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define DW9768_NAME "dw9768"
> +#define DW9768_MAX_FOCUS_POS 1023
> +/*
> + * This sets the minimum granularity for the focus positions.
> + * A value of 1 gives maximum accuracy for a desired focus position
> + */
> +#define DW9768_FOCUS_STEPS 1
> +
> +/*
> + * Ring control and Power control register
> + * Bit[1] RING_EN
> + * 0: Direct mode
> + * 1: AAC mode (ringing control mode)
> + * Bit[0] PD
> + * 0: Normal operation mode
> + * 1: Power down mode
> + * DW9768 requires waiting time of Topr after PD reset takes place.
> + */
> +#define DW9768_RING_PD_CONTROL_REG 0x02
> +#define DW9768_PD_MODE_OFF 0x00
> +#define DW9768_PD_MODE_EN BIT(0)
> +#define DW9768_AAC_MODE_EN BIT(1)
> +
> +/*
> + * DW9768 separates two registers to control the VCM position.
> + * One for MSB value, another is LSB value.
> + * DAC_MSB: D[9:8] (ADD: 0x03)
> + * DAC_LSB: D[7:0] (ADD: 0x04)
> + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> + */
> +#define DW9768_MSB_ADDR 0x03
> +#define DW9768_LSB_ADDR 0x04
> +#define DW9768_STATUS_ADDR 0x05
> +
> +/*
> + * AAC mode control & prescale register
> + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> + * 000 Direct(default)
> + * 001 AAC2 0.48xTvib
> + * 010 AAC3 0.70xTvib
> + * 011 AAC4 0.75xTvib
> + * 100 Reserved
> + * 101 AAC8 1.13xTvib
> + * 110 Reserved
> + * 111 Reserved
> + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> + * 000 2
> + * 001 1(default)
> + * 010 1/2
> + * 011 1/4
> + * 100 8
> + * 101 4
> + * 110 Reserved
> + * 111 Reserved
> + */
> +#define DW9768_AAC_PRESC_REG 0x06
> +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> +
> +/*
> + * VCM period of vibration register
> + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> + * Dividing Rate is the internal clock dividing rate that is deifned at
> + * PRESCALE register (ADD: 0x06)
> + */
> +#define DW9768_AAC_TIME_REG 0x07
> +#define DW9768_AACT_CNT 0x39
I wonder how hardware specific are these values, this and the divider?
The DW9807 has similar configuration but the defaults seem to be just
fine. What are the defaults in this case?
> +
> +/*
> + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> + * or in the case of PD reset taking place.
> + */
> +#define DW9768_T_OPR_US 1000
> +
> +/*
> + * 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 DW9768_MOVE_STEPS 16
> +
> +/*
> + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> + */
> +#define DW9768_MOVE_DELAY_US 8400
> +#define DW9768_STABLE_TIME_US 20000
> +
> +static const char * const dw9768_supply_names[] = {
> + "vin", /* I2C I/O interface power */
> + "vdd", /* VCM power */
> +};
> +
> +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
Please use ARRAY_SIZE() directly instead.
> +
> +/* dw9768 device structure */
> +struct dw9768 {
> + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *focus;
> + struct v4l2_subdev sd;
> +};
> +
> +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> +{
> + return container_of(ctrl->handler, struct dw9768, ctrls);
> +}
> +
> +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> +{
> + return container_of(subdev, struct dw9768, sd);
> +}
> +
> +struct regval_list {
> + u8 reg_num;
> + u8 value;
> +};
> +
> +static struct regval_list dw9768_init_regs[] = {
const
> + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> +};
> +
> +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> + size_t len)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < len; i++) {
> + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> + vals[i].value);
> + if (ret < 0)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> +
> + /* Write VCM position to registers */
> + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> + swab16(val));
> +}
> +
> +static int dw9768_init(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret, val;
> +
> + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_OFF);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> + ARRAY_SIZE(dw9768_init_regs));
> + if (ret)
> + return ret;
> +
> + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> + val <= dw9768->focus->val;
> + val += DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(DW9768_MOVE_DELAY_US,
> + DW9768_MOVE_DELAY_US + 1000);
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret, val;
> +
> + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(DW9768_MOVE_DELAY_US,
> + DW9768_MOVE_DELAY_US + 1000);
> + }
> +
> + /*
> + * Wait for the motor to stabilize after the last movement
> + * to prevent the motor from shaking.
> + */
> + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> +
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + return 0;
> +}
> +
> +/* Power handling */
> +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + dw9768_release(dw9768);
> + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /*
> + * The datasheet refers to t_OPR that needs to be waited before sending
> + * I2C commands after power-up.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_init(dw9768);
> + if (ret < 0)
> + goto disable_regulator;
> +
> + return 0;
> +
> +disable_regulator:
> + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> +
> + return ret;
> +}
> +
> +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9768 *dw9768 = to_dw9768(ctrl);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> + return dw9768_set_dac(dw9768, ctrl->val);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> + .s_ctrl = dw9768_set_ctrl,
> +};
> +
> +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + int ret;
> +
> + ret = pm_runtime_get_sync(sd->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(sd->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + pm_runtime_put(sd->dev);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> + .open = dw9768_open,
> + .close = dw9768_close,
> +};
> +
> +static const struct v4l2_subdev_ops dw9768_ops = { };
> +
> +static int dw9768_init_controls(struct dw9768 *dw9768)
> +{
> + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> +
> + v4l2_ctrl_handler_init(hdl, 1);
> +
> + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> + 0, DW9768_MAX_FOCUS_POS,
> + DW9768_FOCUS_STEPS, 0);
> +
> + if (hdl->error)
> + return hdl->error;
> +
> + dw9768->sd.ctrl_handler = hdl;
> +
> + return 0;
> +}
> +
> +static int dw9768_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct dw9768 *dw9768;
> + unsigned int i;
> + int ret;
> +
> + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> + if (!dw9768)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> +
> + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> + dw9768->supplies[i].supply = dw9768_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> + dw9768->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
I'd try to see the chip is accessible in probe().
> + ret = dw9768_init_controls(dw9768);
> + if (ret)
> + goto entity_cleanup;
> +
> + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + dw9768->sd.internal_ops = &dw9768_int_ops;
> +
> + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> + if (ret < 0)
> + goto entity_cleanup;
> +
> + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> + ret = v4l2_async_register_subdev(&dw9768->sd);
> + if (ret < 0)
> + goto entity_cleanup;
> +
> + pm_runtime_enable(dev);
Your driver appears to depend on runtime PM on DT based systems.
You should either add a dependency to CONFIG_PM, or much more preferrably
make it work without runtime PM.
> +
> + return 0;
> +
> +entity_cleanup:
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + return ret;
> +}
> +
> +static int dw9768_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + pm_runtime_disable(&client->dev);
> + v4l2_async_unregister_subdev(&dw9768->sd);
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + if (!pm_runtime_status_suspended(&client->dev))
> + dw9768_runtime_suspend(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dw9768_of_table[] = {
> + { .compatible = "dongwoon,dw9768" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> +
> +static const struct dev_pm_ops dw9768_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> +};
> +
> +static struct i2c_driver dw9768_i2c_driver = {
> + .driver = {
> + .name = DW9768_NAME,
> + .pm = &dw9768_pm_ops,
> + .of_match_table = dw9768_of_table,
> + },
> + .probe_new = dw9768_probe,
> + .remove = dw9768_remove,
> +};
> +
> +module_i2c_driver(dw9768_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("DW9768 VCM driver");
> +MODULE_LICENSE("GPL v2");
--
Kind regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
2020-03-02 11:07 ` Andy Shevchenko
2020-03-05 12:05 ` Sakari Ailus
@ 2020-03-10 10:00 ` Dongchun Zhu
2020-03-10 10:05 ` Sakari Ailus
2 siblings, 1 reply; 17+ messages in thread
From: Dongchun Zhu @ 2020-03-10 10:00 UTC (permalink / raw)
To: mchehab
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream,
shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, sakari.ailus, matthias.bgg, bingbu.cao,
andriy.shevchenko, linux-arm-kernel, linux-media
Hi Sakari, Rob, Andy, Tomasz,
On Fri, 2020-02-28 at 23:59 +0800, Dongchun Zhu wrote:
> This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> and provides control to set the desired focus via I2C serial interface.
>
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
> MAINTAINERS | 1 +
> drivers/media/i2c/Kconfig | 10 ++
> drivers/media/i2c/Makefile | 1 +
> drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 449 insertions(+)
> create mode 100644 drivers/media/i2c/dw9768.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b805e29..0bb894a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5139,6 +5139,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.yaml
>
> DONGWOON DW9807 LENS VOICE COIL DRIVER
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index c68e002..aa60781 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> --- /dev/null
> +++ b/drivers/media/i2c/dw9768.c
> @@ -0,0 +1,437 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2020 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-async.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +
> +#define DW9768_NAME "dw9768"
> +#define DW9768_MAX_FOCUS_POS 1023
> +/*
> + * This sets the minimum granularity for the focus positions.
> + * A value of 1 gives maximum accuracy for a desired focus position
> + */
> +#define DW9768_FOCUS_STEPS 1
> +
> +/*
> + * Ring control and Power control register
> + * Bit[1] RING_EN
> + * 0: Direct mode
> + * 1: AAC mode (ringing control mode)
> + * Bit[0] PD
> + * 0: Normal operation mode
> + * 1: Power down mode
> + * DW9768 requires waiting time of Topr after PD reset takes place.
> + */
> +#define DW9768_RING_PD_CONTROL_REG 0x02
> +#define DW9768_PD_MODE_OFF 0x00
> +#define DW9768_PD_MODE_EN BIT(0)
> +#define DW9768_AAC_MODE_EN BIT(1)
> +
> +/*
> + * DW9768 separates two registers to control the VCM position.
> + * One for MSB value, another is LSB value.
> + * DAC_MSB: D[9:8] (ADD: 0x03)
> + * DAC_LSB: D[7:0] (ADD: 0x04)
> + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> + */
> +#define DW9768_MSB_ADDR 0x03
> +#define DW9768_LSB_ADDR 0x04
> +#define DW9768_STATUS_ADDR 0x05
> +
> +/*
> + * AAC mode control & prescale register
> + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> + * 000 Direct(default)
> + * 001 AAC2 0.48xTvib
> + * 010 AAC3 0.70xTvib
> + * 011 AAC4 0.75xTvib
> + * 100 Reserved
> + * 101 AAC8 1.13xTvib
> + * 110 Reserved
> + * 111 Reserved
> + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> + * 000 2
> + * 001 1(default)
> + * 010 1/2
> + * 011 1/4
> + * 100 8
> + * 101 4
> + * 110 Reserved
> + * 111 Reserved
> + */
> +#define DW9768_AAC_PRESC_REG 0x06
> +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> +
> +/*
> + * VCM period of vibration register
> + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> + * Dividing Rate is the internal clock dividing rate that is deifned at
> + * PRESCALE register (ADD: 0x06)
> + */
> +#define DW9768_AAC_TIME_REG 0x07
> +#define DW9768_AACT_CNT 0x39
> +
> +/*
> + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> + * or in the case of PD reset taking place.
> + */
> +#define DW9768_T_OPR_US 1000
> +
> +/*
> + * 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 DW9768_MOVE_STEPS 16
> +
> +/*
> + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> + */
> +#define DW9768_MOVE_DELAY_US 8400
> +#define DW9768_STABLE_TIME_US 20000
> +
> +static const char * const dw9768_supply_names[] = {
> + "vin", /* I2C I/O interface power */
> + "vdd", /* VCM power */
> +};
> +
> +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
> +
> +/* dw9768 device structure */
> +struct dw9768 {
> + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> + struct v4l2_ctrl_handler ctrls;
> + struct v4l2_ctrl *focus;
> + struct v4l2_subdev sd;
> +};
> +
> +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> +{
> + return container_of(ctrl->handler, struct dw9768, ctrls);
> +}
> +
> +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> +{
> + return container_of(subdev, struct dw9768, sd);
> +}
> +
> +struct regval_list {
> + u8 reg_num;
> + u8 value;
> +};
> +
> +static struct regval_list dw9768_init_regs[] = {
> + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> +};
> +
> +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> + size_t len)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < len; i++) {
> + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> + vals[i].value);
> + if (ret < 0)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> +
> + /* Write VCM position to registers */
> + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> + swab16(val));
> +}
> +
> +static int dw9768_init(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret, val;
> +
> + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_OFF);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> + ARRAY_SIZE(dw9768_init_regs));
> + if (ret)
> + return ret;
> +
> + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> + val <= dw9768->focus->val;
> + val += DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(DW9768_MOVE_DELAY_US,
> + DW9768_MOVE_DELAY_US + 1000);
> + }
> +
What do you think about the approach taken by this patch?
From the view of VCM hardware, the collision sound of lens should only
happen when moving position back to zero.
When opening camera, one should be able to move lens to the position
directly.
I tried to replace this code to a single dw9768_set_dac(dw9768,
dw9768->focus->val),
there is no collision sound when open camera and it could reduce several
hundred ms when open lens driver fd.
Are we okay with this?
> + return 0;
> +}
> +
> +static int dw9768_release(struct dw9768 *dw9768)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> + int ret, val;
> +
> + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> + val >= 0; val -= DW9768_MOVE_STEPS) {
> + ret = dw9768_set_dac(dw9768, val);
> + if (ret) {
> + dev_err(&client->dev, "%s I2C failure: %d",
> + __func__, ret);
> + return ret;
> + }
> + usleep_range(DW9768_MOVE_DELAY_US,
> + DW9768_MOVE_DELAY_US + 1000);
> + }
> +
> + /*
> + * Wait for the motor to stabilize after the last movement
> + * to prevent the motor from shaking.
> + */
> + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> +
> + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> + DW9768_PD_MODE_EN);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * DW9769 requires waiting delay time of t_OPR
> + * after PD reset takes place.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + return 0;
> +}
> +
> +/* Power handling */
> +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + dw9768_release(dw9768);
> + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> + int ret;
> +
> + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable regulators\n");
> + return ret;
> + }
> +
> + /*
> + * The datasheet refers to t_OPR that needs to be waited before sending
> + * I2C commands after power-up.
> + */
> + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> +
> + ret = dw9768_init(dw9768);
> + if (ret < 0)
> + goto disable_regulator;
> +
> + return 0;
> +
> +disable_regulator:
> + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> +
> + return ret;
> +}
> +
> +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> + struct dw9768 *dw9768 = to_dw9768(ctrl);
> +
> + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> + return dw9768_set_dac(dw9768, ctrl->val);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> + .s_ctrl = dw9768_set_ctrl,
> +};
> +
> +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + int ret;
> +
> + ret = pm_runtime_get_sync(sd->dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(sd->dev);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> + pm_runtime_put(sd->dev);
> +
> + return 0;
> +}
> +
> +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> + .open = dw9768_open,
> + .close = dw9768_close,
> +};
> +
> +static const struct v4l2_subdev_ops dw9768_ops = { };
> +
> +static int dw9768_init_controls(struct dw9768 *dw9768)
> +{
> + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> +
> + v4l2_ctrl_handler_init(hdl, 1);
> +
> + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> + 0, DW9768_MAX_FOCUS_POS,
> + DW9768_FOCUS_STEPS, 0);
> +
> + if (hdl->error)
> + return hdl->error;
> +
> + dw9768->sd.ctrl_handler = hdl;
> +
> + return 0;
> +}
> +
> +static int dw9768_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct dw9768 *dw9768;
> + unsigned int i;
> + int ret;
> +
> + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> + if (!dw9768)
> + return -ENOMEM;
> +
> + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> +
> + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> + dw9768->supplies[i].supply = dw9768_supply_names[i];
> +
> + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> + dw9768->supplies);
> + if (ret) {
> + dev_err(dev, "failed to get regulators\n");
> + return ret;
> + }
> +
> + ret = dw9768_init_controls(dw9768);
> + if (ret)
> + goto entity_cleanup;
> +
> + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> + dw9768->sd.internal_ops = &dw9768_int_ops;
> +
> + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> + if (ret < 0)
> + goto entity_cleanup;
> +
> + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> +
> + ret = v4l2_async_register_subdev(&dw9768->sd);
> + if (ret < 0)
> + goto entity_cleanup;
> +
> + pm_runtime_enable(dev);
> +
> + return 0;
> +
> +entity_cleanup:
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + return ret;
> +}
> +
> +static int dw9768_remove(struct i2c_client *client)
> +{
> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> +
> + pm_runtime_disable(&client->dev);
> + v4l2_async_unregister_subdev(&dw9768->sd);
> + v4l2_ctrl_handler_free(&dw9768->ctrls);
> + media_entity_cleanup(&dw9768->sd.entity);
> + if (!pm_runtime_status_suspended(&client->dev))
> + dw9768_runtime_suspend(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dw9768_of_table[] = {
> + { .compatible = "dongwoon,dw9768" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> +
> +static const struct dev_pm_ops dw9768_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> +};
> +
> +static struct i2c_driver dw9768_i2c_driver = {
> + .driver = {
> + .name = DW9768_NAME,
> + .pm = &dw9768_pm_ops,
> + .of_match_table = dw9768_of_table,
> + },
> + .probe_new = dw9768_probe,
> + .remove = dw9768_remove,
> +};
> +
> +module_i2c_driver(dw9768_i2c_driver);
> +
> +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> +MODULE_DESCRIPTION("DW9768 VCM driver");
> +MODULE_LICENSE("GPL v2");
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-10 10:00 ` Dongchun Zhu
@ 2020-03-10 10:05 ` Sakari Ailus
0 siblings, 0 replies; 17+ messages in thread
From: Sakari Ailus @ 2020-03-10 10:05 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
Hi Dongchun,
On Tue, Mar 10, 2020 at 06:00:19PM +0800, Dongchun Zhu wrote:
> Hi Sakari, Rob, Andy, Tomasz,
>
> On Fri, 2020-02-28 at 23:59 +0800, Dongchun Zhu wrote:
...
> > +static int dw9768_init(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret, val;
> > +
> > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > + DW9768_PD_MODE_OFF);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * DW9769 requires waiting delay time of t_OPR
> > + * after PD reset takes place.
> > + */
> > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > +
> > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > + ARRAY_SIZE(dw9768_init_regs));
> > + if (ret)
> > + return ret;
> > +
> > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > + val <= dw9768->focus->val;
> > + val += DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
> > + return ret;
> > + }
> > + usleep_range(DW9768_MOVE_DELAY_US,
> > + DW9768_MOVE_DELAY_US + 1000);
> > + }
> > +
>
> What do you think about the approach taken by this patch?
> From the view of VCM hardware, the collision sound of lens should only
> happen when moving position back to zero.
> When opening camera, one should be able to move lens to the position
> directly.
> I tried to replace this code to a single dw9768_set_dac(dw9768,
> dw9768->focus->val),
> there is no collision sound when open camera and it could reduce several
> hundred ms when open lens driver fd.
> Are we okay with this?
I think so. Usually on VCMs with ringing compensation the only problematic
case is when the power is cut. :-)
--
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-05 12:05 ` Sakari Ailus
@ 2020-03-10 10:10 ` Andy Shevchenko
2020-03-19 10:03 ` Dongchun Zhu
2020-03-18 10:20 ` Dongchun Zhu
1 sibling, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-10 10:10 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
devicetree, shengnan.wang, Tomasz Figa, louis.kuo, sj.huang,
Rob Herring, moderated list:ARM/Mediatek SoC support,
Dongchun Zhu, Matthias Brugger, bingbu.cao,
Mauro Carvalho Chehab, linux-arm Mailing List,
Linux Media Mailing List
On Thu, Mar 5, 2020 at 2:07 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > and provides control to set the desired focus via I2C serial interface.
...
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5139,6 +5139,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.yaml
This has ordering issues.
Run parse-maintainers.pl to fix.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-05 12:05 ` Sakari Ailus
2020-03-10 10:10 ` Andy Shevchenko
@ 2020-03-18 10:20 ` Dongchun Zhu
2020-03-18 10:29 ` Sakari Ailus
1 sibling, 1 reply; 17+ messages in thread
From: Dongchun Zhu @ 2020-03-18 10:20 UTC (permalink / raw)
To: Sakari Ailus
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
Hi Sakari,
On Thu, 2020-03-05 at 14:05 +0200, Sakari Ailus wrote:
> Hi Dongchun,
>
> Thanks for the update.
>
> On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > and provides control to set the desired focus via I2C serial interface.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/i2c/Kconfig | 10 ++
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 449 insertions(+)
> > create mode 100644 drivers/media/i2c/dw9768.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b805e29..0bb894a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5139,6 +5139,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.yaml
> >
> > DONGWOON DW9807 LENS VOICE COIL DRIVER
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index c68e002..aa60781 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> > --- /dev/null
> > +++ b/drivers/media/i2c/dw9768.c
> > @@ -0,0 +1,437 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2020 MediaTek Inc.
> > +
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/pm_runtime.h>
>
> Alphabetical order would be nice.
>
> > +#include <media/v4l2-async.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-device.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define DW9768_NAME "dw9768"
> > +#define DW9768_MAX_FOCUS_POS 1023
> > +/*
> > + * This sets the minimum granularity for the focus positions.
> > + * A value of 1 gives maximum accuracy for a desired focus position
> > + */
> > +#define DW9768_FOCUS_STEPS 1
> > +
> > +/*
> > + * Ring control and Power control register
> > + * Bit[1] RING_EN
> > + * 0: Direct mode
> > + * 1: AAC mode (ringing control mode)
> > + * Bit[0] PD
> > + * 0: Normal operation mode
> > + * 1: Power down mode
> > + * DW9768 requires waiting time of Topr after PD reset takes place.
> > + */
> > +#define DW9768_RING_PD_CONTROL_REG 0x02
> > +#define DW9768_PD_MODE_OFF 0x00
> > +#define DW9768_PD_MODE_EN BIT(0)
> > +#define DW9768_AAC_MODE_EN BIT(1)
> > +
> > +/*
> > + * DW9768 separates two registers to control the VCM position.
> > + * One for MSB value, another is LSB value.
> > + * DAC_MSB: D[9:8] (ADD: 0x03)
> > + * DAC_LSB: D[7:0] (ADD: 0x04)
> > + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> > + */
> > +#define DW9768_MSB_ADDR 0x03
> > +#define DW9768_LSB_ADDR 0x04
> > +#define DW9768_STATUS_ADDR 0x05
> > +
> > +/*
> > + * AAC mode control & prescale register
> > + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> > + * 000 Direct(default)
> > + * 001 AAC2 0.48xTvib
> > + * 010 AAC3 0.70xTvib
> > + * 011 AAC4 0.75xTvib
> > + * 100 Reserved
> > + * 101 AAC8 1.13xTvib
> > + * 110 Reserved
> > + * 111 Reserved
> > + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> > + * 000 2
> > + * 001 1(default)
> > + * 010 1/2
> > + * 011 1/4
> > + * 100 8
> > + * 101 4
> > + * 110 Reserved
> > + * 111 Reserved
> > + */
> > +#define DW9768_AAC_PRESC_REG 0x06
> > +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> > +
> > +/*
> > + * VCM period of vibration register
> > + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> > + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> > + * Dividing Rate is the internal clock dividing rate that is defined at
> > + * PRESCALE register (ADD: 0x06)
> > + */
> > +#define DW9768_AAC_TIME_REG 0x07
> > +#define DW9768_AACT_CNT 0x39
>
> I wonder how hardware specific are these values, this and the divider?
>
> The DW9807 has similar configuration but the defaults seem to be just
> fine. What are the defaults in this case?
>
From hardware specific, the default value of AAC_PRESC_REG is 0x01, and
that value of AAC_TIME_REG is 0x20.
For these two registers, each bit is defined as below.
D[7:0] (D[7:5]-> AC[2:0], D[2:0]-> PRSEC[2:0])
+------+------+------+------+------+------+------+------+
| AC2 | AC1 | AC0 |------|------|PRESC2|PRESC1|PRESC0|
+------+------+------+------+------+------+------+------+
D[7:0] (D[5:0]-> AACT[5:0]).
+------+------+------+------+------+------+------+------+
|------+------+ AACT5| AACT4| AACT3| AACT2| AACT1| AACT0|
+------+------+------+------+------+------+------+------+
> > +
> > +/*
> > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > + * or in the case of PD reset taking place.
> > + */
> > +#define DW9768_T_OPR_US 1000
> > +
> > +/*
> > + * 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 DW9768_MOVE_STEPS 16
> > +
> > +/*
> > + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> > + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> > + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> > + */
> > +#define DW9768_MOVE_DELAY_US 8400
> > +#define DW9768_STABLE_TIME_US 20000
> > +
> > +static const char * const dw9768_supply_names[] = {
> > + "vin", /* I2C I/O interface power */
> > + "vdd", /* VCM power */
> > +};
> > +
> > +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
>
> Please use ARRAY_SIZE() directly instead.
>
> > +
> > +/* dw9768 device structure */
> > +struct dw9768 {
> > + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> > + struct v4l2_ctrl_handler ctrls;
> > + struct v4l2_ctrl *focus;
> > + struct v4l2_subdev sd;
> > +};
> > +
> > +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> > +{
> > + return container_of(ctrl->handler, struct dw9768, ctrls);
> > +}
> > +
> > +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> > +{
> > + return container_of(subdev, struct dw9768, sd);
> > +}
> > +
> > +struct regval_list {
> > + u8 reg_num;
> > + u8 value;
> > +};
> > +
> > +static struct regval_list dw9768_init_regs[] = {
>
> const
>
> > + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> > + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> > + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> > +};
> > +
> > +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> > + size_t len)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + unsigned int i;
> > + int ret;
> > +
> > + for (i = 0; i < len; i++) {
> > + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> > + vals[i].value);
> > + if (ret < 0)
> > + return ret;
> > + }
> > + return 0;
> > +}
> > +
> > +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > +
> > + /* Write VCM position to registers */
> > + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> > + swab16(val));
> > +}
> > +
> > +static int dw9768_init(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret, val;
> > +
> > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > + DW9768_PD_MODE_OFF);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * DW9769 requires waiting delay time of t_OPR
> > + * after PD reset takes place.
> > + */
> > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > +
> > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > + ARRAY_SIZE(dw9768_init_regs));
> > + if (ret)
> > + return ret;
> > +
> > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > + val <= dw9768->focus->val;
> > + val += DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
> > + return ret;
> > + }
> > + usleep_range(DW9768_MOVE_DELAY_US,
> > + DW9768_MOVE_DELAY_US + 1000);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int dw9768_release(struct dw9768 *dw9768)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > + int ret, val;
> > +
> > + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > + val >= 0; val -= DW9768_MOVE_STEPS) {
> > + ret = dw9768_set_dac(dw9768, val);
> > + if (ret) {
> > + dev_err(&client->dev, "%s I2C failure: %d",
> > + __func__, ret);
> > + return ret;
> > + }
> > + usleep_range(DW9768_MOVE_DELAY_US,
> > + DW9768_MOVE_DELAY_US + 1000);
> > + }
> > +
> > + /*
> > + * Wait for the motor to stabilize after the last movement
> > + * to prevent the motor from shaking.
> > + */
> > + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> > + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> > +
> > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > + DW9768_PD_MODE_EN);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * DW9769 requires waiting delay time of t_OPR
> > + * after PD reset takes place.
> > + */
> > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > +
> > + return 0;
> > +}
> > +
> > +/* Power handling */
> > +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > +
> > + dw9768_release(dw9768);
> > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > + int ret;
> > +
> > + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable regulators\n");
> > + return ret;
> > + }
> > +
> > + /*
> > + * The datasheet refers to t_OPR that needs to be waited before sending
> > + * I2C commands after power-up.
> > + */
> > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > +
> > + ret = dw9768_init(dw9768);
> > + if (ret < 0)
> > + goto disable_regulator;
> > +
> > + return 0;
> > +
> > +disable_regulator:
> > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > +
> > + return ret;
> > +}
> > +
> > +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct dw9768 *dw9768 = to_dw9768(ctrl);
> > +
> > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> > + return dw9768_set_dac(dw9768, ctrl->val);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> > + .s_ctrl = dw9768_set_ctrl,
> > +};
> > +
> > +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > + int ret;
> > +
> > + ret = pm_runtime_get_sync(sd->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(sd->dev);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > + pm_runtime_put(sd->dev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> > + .open = dw9768_open,
> > + .close = dw9768_close,
> > +};
> > +
> > +static const struct v4l2_subdev_ops dw9768_ops = { };
> > +
> > +static int dw9768_init_controls(struct dw9768 *dw9768)
> > +{
> > + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> > + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> > +
> > + v4l2_ctrl_handler_init(hdl, 1);
> > +
> > + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> > + 0, DW9768_MAX_FOCUS_POS,
> > + DW9768_FOCUS_STEPS, 0);
> > +
> > + if (hdl->error)
> > + return hdl->error;
> > +
> > + dw9768->sd.ctrl_handler = hdl;
> > +
> > + return 0;
> > +}
> > +
> > +static int dw9768_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct dw9768 *dw9768;
> > + unsigned int i;
> > + int ret;
> > +
> > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > + if (!dw9768)
> > + return -ENOMEM;
> > +
> > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > +
> > + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > +
> > + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> > + dw9768->supplies);
> > + if (ret) {
> > + dev_err(dev, "failed to get regulators\n");
> > + return ret;
> > + }
> > +
>
> I'd try to see the chip is accessible in probe().
>
If probe failed, actuator device node would not be generated.
When user tries to open fd, it should report error also.
> > + ret = dw9768_init_controls(dw9768);
> > + if (ret)
> > + goto entity_cleanup;
> > +
> > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > +
> > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > + if (ret < 0)
> > + goto entity_cleanup;
> > +
> > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > +
> > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > + if (ret < 0)
> > + goto entity_cleanup;
> > +
> > + pm_runtime_enable(dev);
>
> Your driver appears to depend on runtime PM on DT based systems.
>
> You should either add a dependency to CONFIG_PM, or much more preferrably
> make it work without runtime PM.
>
Do you mean using the macro like this:
#ifdef CONFIG_PM
...
#endif
> > +
> > + return 0;
> > +
> > +entity_cleanup:
> > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > + media_entity_cleanup(&dw9768->sd.entity);
> > + return ret;
> > +}
> > +
> > +static int dw9768_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > +
> > + pm_runtime_disable(&client->dev);
> > + v4l2_async_unregister_subdev(&dw9768->sd);
> > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > + media_entity_cleanup(&dw9768->sd.entity);
> > + if (!pm_runtime_status_suspended(&client->dev))
> > + dw9768_runtime_suspend(&client->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id dw9768_of_table[] = {
> > + { .compatible = "dongwoon,dw9768" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> > +
> > +static const struct dev_pm_ops dw9768_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > + pm_runtime_force_resume)
> > + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> > +};
> > +
> > +static struct i2c_driver dw9768_i2c_driver = {
> > + .driver = {
> > + .name = DW9768_NAME,
> > + .pm = &dw9768_pm_ops,
> > + .of_match_table = dw9768_of_table,
> > + },
> > + .probe_new = dw9768_probe,
> > + .remove = dw9768_remove,
> > +};
> > +
> > +module_i2c_driver(dw9768_i2c_driver);
> > +
> > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > +MODULE_DESCRIPTION("DW9768 VCM driver");
> > +MODULE_LICENSE("GPL v2");
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-18 10:20 ` Dongchun Zhu
@ 2020-03-18 10:29 ` Sakari Ailus
2020-03-23 19:09 ` Tomasz Figa
0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2020-03-18 10:29 UTC (permalink / raw)
To: Dongchun Zhu
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, shengnan.wang, tfiga, louis.kuo, sj.huang, robh+dt,
linux-mediatek, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
Hi Dongchun,
On Wed, Mar 18, 2020 at 06:20:24PM +0800, Dongchun Zhu wrote:
> Hi Sakari,
>
> On Thu, 2020-03-05 at 14:05 +0200, Sakari Ailus wrote:
> > Hi Dongchun,
> >
> > Thanks for the update.
> >
> > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > and provides control to set the desired focus via I2C serial interface.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 10 ++
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 449 insertions(+)
> > > create mode 100644 drivers/media/i2c/dw9768.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b805e29..0bb894a 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -5139,6 +5139,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.yaml
> > >
> > > DONGWOON DW9807 LENS VOICE COIL DRIVER
> > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > index c68e002..aa60781 100644
> > > --- a/drivers/media/i2c/Kconfig
> > > +++ b/drivers/media/i2c/Kconfig
> > > @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/dw9768.c
> > > @@ -0,0 +1,437 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2020 MediaTek Inc.
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/module.h>
> > > +#include <linux/regulator/consumer.h>
> > > +#include <linux/pm_runtime.h>
> >
> > Alphabetical order would be nice.
> >
> > > +#include <media/v4l2-async.h>
> > > +#include <media/v4l2-ctrls.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +#define DW9768_NAME "dw9768"
> > > +#define DW9768_MAX_FOCUS_POS 1023
> > > +/*
> > > + * This sets the minimum granularity for the focus positions.
> > > + * A value of 1 gives maximum accuracy for a desired focus position
> > > + */
> > > +#define DW9768_FOCUS_STEPS 1
> > > +
> > > +/*
> > > + * Ring control and Power control register
> > > + * Bit[1] RING_EN
> > > + * 0: Direct mode
> > > + * 1: AAC mode (ringing control mode)
> > > + * Bit[0] PD
> > > + * 0: Normal operation mode
> > > + * 1: Power down mode
> > > + * DW9768 requires waiting time of Topr after PD reset takes place.
> > > + */
> > > +#define DW9768_RING_PD_CONTROL_REG 0x02
> > > +#define DW9768_PD_MODE_OFF 0x00
> > > +#define DW9768_PD_MODE_EN BIT(0)
> > > +#define DW9768_AAC_MODE_EN BIT(1)
> > > +
> > > +/*
> > > + * DW9768 separates two registers to control the VCM position.
> > > + * One for MSB value, another is LSB value.
> > > + * DAC_MSB: D[9:8] (ADD: 0x03)
> > > + * DAC_LSB: D[7:0] (ADD: 0x04)
> > > + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> > > + */
> > > +#define DW9768_MSB_ADDR 0x03
> > > +#define DW9768_LSB_ADDR 0x04
> > > +#define DW9768_STATUS_ADDR 0x05
> > > +
> > > +/*
> > > + * AAC mode control & prescale register
> > > + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> > > + * 000 Direct(default)
> > > + * 001 AAC2 0.48xTvib
> > > + * 010 AAC3 0.70xTvib
> > > + * 011 AAC4 0.75xTvib
> > > + * 100 Reserved
> > > + * 101 AAC8 1.13xTvib
> > > + * 110 Reserved
> > > + * 111 Reserved
> > > + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> > > + * 000 2
> > > + * 001 1(default)
> > > + * 010 1/2
> > > + * 011 1/4
> > > + * 100 8
> > > + * 101 4
> > > + * 110 Reserved
> > > + * 111 Reserved
> > > + */
> > > +#define DW9768_AAC_PRESC_REG 0x06
> > > +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> > > +
> > > +/*
> > > + * VCM period of vibration register
> > > + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> > > + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> > > + * Dividing Rate is the internal clock dividing rate that is defined at
> > > + * PRESCALE register (ADD: 0x06)
> > > + */
> > > +#define DW9768_AAC_TIME_REG 0x07
> > > +#define DW9768_AACT_CNT 0x39
> >
> > I wonder how hardware specific are these values, this and the divider?
> >
> > The DW9807 has similar configuration but the defaults seem to be just
> > fine. What are the defaults in this case?
> >
>
> From hardware specific, the default value of AAC_PRESC_REG is 0x01, and
> that value of AAC_TIME_REG is 0x20.
>
> For these two registers, each bit is defined as below.
> D[7:0] (D[7:5]-> AC[2:0], D[2:0]-> PRSEC[2:0])
> +------+------+------+------+------+------+------+------+
> | AC2 | AC1 | AC0 |------|------|PRESC2|PRESC1|PRESC0|
> +------+------+------+------+------+------+------+------+
>
> D[7:0] (D[5:0]-> AACT[5:0]).
> +------+------+------+------+------+------+------+------+
> |------+------+ AACT5| AACT4| AACT3| AACT2| AACT1| AACT0|
> +------+------+------+------+------+------+------+------+
>
> > > +
> > > +/*
> > > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > > + * or in the case of PD reset taking place.
> > > + */
> > > +#define DW9768_T_OPR_US 1000
> > > +
> > > +/*
> > > + * 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 DW9768_MOVE_STEPS 16
> > > +
> > > +/*
> > > + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> > > + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> > > + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> > > + */
> > > +#define DW9768_MOVE_DELAY_US 8400
> > > +#define DW9768_STABLE_TIME_US 20000
> > > +
> > > +static const char * const dw9768_supply_names[] = {
> > > + "vin", /* I2C I/O interface power */
> > > + "vdd", /* VCM power */
> > > +};
> > > +
> > > +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
> >
> > Please use ARRAY_SIZE() directly instead.
> >
> > > +
> > > +/* dw9768 device structure */
> > > +struct dw9768 {
> > > + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> > > + struct v4l2_ctrl_handler ctrls;
> > > + struct v4l2_ctrl *focus;
> > > + struct v4l2_subdev sd;
> > > +};
> > > +
> > > +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> > > +{
> > > + return container_of(ctrl->handler, struct dw9768, ctrls);
> > > +}
> > > +
> > > +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> > > +{
> > > + return container_of(subdev, struct dw9768, sd);
> > > +}
> > > +
> > > +struct regval_list {
> > > + u8 reg_num;
> > > + u8 value;
> > > +};
> > > +
> > > +static struct regval_list dw9768_init_regs[] = {
> >
> > const
> >
> > > + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> > > + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> > > + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> > > +};
> > > +
> > > +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> > > + size_t len)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + for (i = 0; i < len; i++) {
> > > + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> > > + vals[i].value);
> > > + if (ret < 0)
> > > + return ret;
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > +
> > > + /* Write VCM position to registers */
> > > + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> > > + swab16(val));
> > > +}
> > > +
> > > +static int dw9768_init(struct dw9768 *dw9768)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > + int ret, val;
> > > +
> > > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > + DW9768_PD_MODE_OFF);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /*
> > > + * DW9769 requires waiting delay time of t_OPR
> > > + * after PD reset takes place.
> > > + */
> > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > +
> > > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > > + ARRAY_SIZE(dw9768_init_regs));
> > > + if (ret)
> > > + return ret;
> > > +
> > > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > > + val <= dw9768->focus->val;
> > > + val += DW9768_MOVE_STEPS) {
> > > + ret = dw9768_set_dac(dw9768, val);
> > > + if (ret) {
> > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > + __func__, ret);
> > > + return ret;
> > > + }
> > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > + DW9768_MOVE_DELAY_US + 1000);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dw9768_release(struct dw9768 *dw9768)
> > > +{
> > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > + int ret, val;
> > > +
> > > + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > > + val >= 0; val -= DW9768_MOVE_STEPS) {
> > > + ret = dw9768_set_dac(dw9768, val);
> > > + if (ret) {
> > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > + __func__, ret);
> > > + return ret;
> > > + }
> > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > + DW9768_MOVE_DELAY_US + 1000);
> > > + }
> > > +
> > > + /*
> > > + * Wait for the motor to stabilize after the last movement
> > > + * to prevent the motor from shaking.
> > > + */
> > > + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> > > + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> > > +
> > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > + DW9768_PD_MODE_EN);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /*
> > > + * DW9769 requires waiting delay time of t_OPR
> > > + * after PD reset takes place.
> > > + */
> > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/* Power handling */
> > > +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > +
> > > + dw9768_release(dw9768);
> > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> > > +{
> > > + struct i2c_client *client = to_i2c_client(dev);
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > + int ret;
> > > +
> > > + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to enable regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + /*
> > > + * The datasheet refers to t_OPR that needs to be waited before sending
> > > + * I2C commands after power-up.
> > > + */
> > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > +
> > > + ret = dw9768_init(dw9768);
> > > + if (ret < 0)
> > > + goto disable_regulator;
> > > +
> > > + return 0;
> > > +
> > > +disable_regulator:
> > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> > > +{
> > > + struct dw9768 *dw9768 = to_dw9768(ctrl);
> > > +
> > > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> > > + return dw9768_set_dac(dw9768, ctrl->val);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> > > + .s_ctrl = dw9768_set_ctrl,
> > > +};
> > > +
> > > +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > +{
> > > + int ret;
> > > +
> > > + ret = pm_runtime_get_sync(sd->dev);
> > > + if (ret < 0) {
> > > + pm_runtime_put_noidle(sd->dev);
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > +{
> > > + pm_runtime_put(sd->dev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> > > + .open = dw9768_open,
> > > + .close = dw9768_close,
> > > +};
> > > +
> > > +static const struct v4l2_subdev_ops dw9768_ops = { };
> > > +
> > > +static int dw9768_init_controls(struct dw9768 *dw9768)
> > > +{
> > > + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> > > + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> > > +
> > > + v4l2_ctrl_handler_init(hdl, 1);
> > > +
> > > + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> > > + 0, DW9768_MAX_FOCUS_POS,
> > > + DW9768_FOCUS_STEPS, 0);
> > > +
> > > + if (hdl->error)
> > > + return hdl->error;
> > > +
> > > + dw9768->sd.ctrl_handler = hdl;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int dw9768_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct dw9768 *dw9768;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > > + if (!dw9768)
> > > + return -ENOMEM;
> > > +
> > > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > > +
> > > + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> > > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> > > + dw9768->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> >
> > I'd try to see the chip is accessible in probe().
> >
>
> If probe failed, actuator device node would not be generated.
Yes, this would be preferred.
> When user tries to open fd, it should report error also.
>
> > > + ret = dw9768_init_controls(dw9768);
> > > + if (ret)
> > > + goto entity_cleanup;
> > > +
> > > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > > +
> > > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > > + if (ret < 0)
> > > + goto entity_cleanup;
> > > +
> > > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > > +
> > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > + if (ret < 0)
> > > + goto entity_cleanup;
> > > +
> > > + pm_runtime_enable(dev);
> >
> > Your driver appears to depend on runtime PM on DT based systems.
> >
> > You should either add a dependency to CONFIG_PM, or much more preferrably
> > make it work without runtime PM.
> >
>
> Do you mean using the macro like this:
> #ifdef CONFIG_PM
> ...
> #endif
No. If CONFIG_PM is disabled, the runtime PM functions do nothing. Would
your driver work in that case?
>
> > > +
> > > + return 0;
> > > +
> > > +entity_cleanup:
> > > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > > + media_entity_cleanup(&dw9768->sd.entity);
> > > + return ret;
> > > +}
> > > +
> > > +static int dw9768_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > +
> > > + pm_runtime_disable(&client->dev);
> > > + v4l2_async_unregister_subdev(&dw9768->sd);
> > > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > > + media_entity_cleanup(&dw9768->sd.entity);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + dw9768_runtime_suspend(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id dw9768_of_table[] = {
> > > + { .compatible = "dongwoon,dw9768" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> > > +
> > > +static const struct dev_pm_ops dw9768_pm_ops = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > > + pm_runtime_force_resume)
> > > + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> > > +};
> > > +
> > > +static struct i2c_driver dw9768_i2c_driver = {
> > > + .driver = {
> > > + .name = DW9768_NAME,
> > > + .pm = &dw9768_pm_ops,
> > > + .of_match_table = dw9768_of_table,
> > > + },
> > > + .probe_new = dw9768_probe,
> > > + .remove = dw9768_remove,
> > > +};
> > > +
> > > +module_i2c_driver(dw9768_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("DW9768 VCM driver");
> > > +MODULE_LICENSE("GPL v2");
> >
>
--
Regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-10 10:10 ` Andy Shevchenko
@ 2020-03-19 10:03 ` Dongchun Zhu
2020-03-19 11:36 ` Andy Shevchenko
0 siblings, 1 reply; 17+ messages in thread
From: Dongchun Zhu @ 2020-03-19 10:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Rutland, Nicolas Boichat, Mauro Carvalho Chehab,
srv_heupstream, devicetree, shengnan.wang, Tomasz Figa,
louis.kuo, sj.huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Sakari Ailus,
Matthias Brugger, bingbu.cao, Andy Shevchenko,
linux-arm Mailing List, Linux Media Mailing List
Hi Andy,
On Tue, 2020-03-10 at 12:10 +0200, Andy Shevchenko wrote:
> On Thu, Mar 5, 2020 at 2:07 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > and provides control to set the desired focus via I2C serial interface.
>
> ...
>
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -5139,6 +5139,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.yaml
>
> This has ordering issues.
> Run parse-maintainers.pl to fix.
>
Pardon, how to run parse-maintainers.pl?
Locally I ran this script, it occurs some syntax as below.
$./scripts/parse-maintainers.pl
syntax error at ./scripts/parse-maintainers.pl line 108, near
"$hashref{"
Global symbol "$pattern" requires explicit package name
at ./scripts/parse-maintainers.pl line 109.
syntax error at ./scripts/parse-maintainers.pl line 112, near "}"
Global symbol "$file" requires explicit package name
at ./scripts/parse-maintainers.pl line 113.
Can't use global @_ in "my" at ./scripts/parse-maintainers.pl line 117,
near "(@_"
syntax error at ./scripts/parse-maintainers.pl line 152, near "}"
Execution of ./scripts/parse-maintainers.pl aborted due to compilation
errors.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-19 10:03 ` Dongchun Zhu
@ 2020-03-19 11:36 ` Andy Shevchenko
2020-03-19 11:54 ` Dongchun Zhu
0 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2020-03-19 11:36 UTC (permalink / raw)
To: Dongchun Zhu
Cc: Mark Rutland, devicetree, Nicolas Boichat, srv_heupstream,
shengnan.wang, Tomasz Figa, louis.kuo, sj.huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Sakari Ailus,
Matthias Brugger, bingbu.cao, Mauro Carvalho Chehab,
linux-arm Mailing List, Linux Media Mailing List
On Thu, Mar 19, 2020 at 06:03:35PM +0800, Dongchun Zhu wrote:
> On Tue, 2020-03-10 at 12:10 +0200, Andy Shevchenko wrote:
> > On Thu, Mar 5, 2020 at 2:07 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > > and provides control to set the desired focus via I2C serial interface.
> >
> > ...
> >
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -5139,6 +5139,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.yaml
> >
> > This has ordering issues.
> > Run parse-maintainers.pl to fix.
> >
>
> Pardon, how to run parse-maintainers.pl?
> Locally I ran this script, it occurs some syntax as below.
> $./scripts/parse-maintainers.pl
It's a perl script which is made non-executable by some reason.
So, proper run as a parameter to the language interpreter, i.e.
$ perl scripts/parse-maintainer.pl
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-19 11:36 ` Andy Shevchenko
@ 2020-03-19 11:54 ` Dongchun Zhu
0 siblings, 0 replies; 17+ messages in thread
From: Dongchun Zhu @ 2020-03-19 11:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Mark Rutland, devicetree, Nicolas Boichat, srv_heupstream,
shengnan.wang, Tomasz Figa, louis.kuo, sj.huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Sakari Ailus,
Matthias Brugger, bingbu.cao, Mauro Carvalho Chehab,
linux-arm Mailing List, Linux Media Mailing List
Hi Andy,
On Thu, 2020-03-19 at 13:36 +0200, Andy Shevchenko wrote:
> On Thu, Mar 19, 2020 at 06:03:35PM +0800, Dongchun Zhu wrote:
> > On Tue, 2020-03-10 at 12:10 +0200, Andy Shevchenko wrote:
> > > On Thu, Mar 5, 2020 at 2:07 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > > > and provides control to set the desired focus via I2C serial interface.
> > >
> > > ...
> > >
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -5139,6 +5139,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.yaml
> > >
> > > This has ordering issues.
> > > Run parse-maintainers.pl to fix.
> > >
> >
> > Pardon, how to run parse-maintainers.pl?
> > Locally I ran this script, it occurs some syntax as below.
> > $./scripts/parse-maintainers.pl
>
> It's a perl script which is made non-executable by some reason.
>
> So, proper run as a parameter to the language interpreter, i.e.
> $ perl scripts/parse-maintainer.pl
>
I tried-run again on mtk server, which has perl5(version 18.2).
But it still report the same error.
Is there any requirement for perl version?
$perl scripts/parse-maintainers.pl
syntax error at scripts/parse-maintainers.pl line 108, near "$hashref{"
Global symbol "$pattern" requires explicit package name at
scripts/parse-maintainers.pl line 109.
syntax error at scripts/parse-maintainers.pl line 112, near "}"
Global symbol "$file" requires explicit package name at
scripts/parse-maintainers.pl line 113.
Can't use global @_ in "my" at scripts/parse-maintainers.pl line 117,
near "(@_"
syntax error at scripts/parse-maintainers.pl line 152, near "}"
Execution of scripts/parse-maintainers.pl aborted due to compilation
errors.
Local perl version is as below.
$ls -al /usr/bin/perl
perl perl5.18.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-18 10:29 ` Sakari Ailus
@ 2020-03-23 19:09 ` Tomasz Figa
2020-03-23 20:35 ` Sakari Ailus
0 siblings, 1 reply; 17+ messages in thread
From: Tomasz Figa @ 2020-03-23 19:09 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, andriy.shevchenko, srv_heupstream,
linux-devicetree, Shengnan Wang (王圣男),
Louis Kuo, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Dongchun Zhu,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
On Wed, Mar 18, 2020 at 11:29 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Dongchun,
>
> On Wed, Mar 18, 2020 at 06:20:24PM +0800, Dongchun Zhu wrote:
> > Hi Sakari,
> >
> > On Thu, 2020-03-05 at 14:05 +0200, Sakari Ailus wrote:
> > > Hi Dongchun,
> > >
> > > Thanks for the update.
> > >
> > > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > > and provides control to set the desired focus via I2C serial interface.
> > > >
> > > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > > ---
> > > > MAINTAINERS | 1 +
> > > > drivers/media/i2c/Kconfig | 10 ++
> > > > drivers/media/i2c/Makefile | 1 +
> > > > drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> > > > 4 files changed, 449 insertions(+)
> > > > create mode 100644 drivers/media/i2c/dw9768.c
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index b805e29..0bb894a 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -5139,6 +5139,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.yaml
> > > >
> > > > DONGWOON DW9807 LENS VOICE COIL DRIVER
> > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > > index c68e002..aa60781 100644
> > > > --- a/drivers/media/i2c/Kconfig
> > > > +++ b/drivers/media/i2c/Kconfig
> > > > @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> > > > --- /dev/null
> > > > +++ b/drivers/media/i2c/dw9768.c
> > > > @@ -0,0 +1,437 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +// Copyright (c) 2020 MediaTek Inc.
> > > > +
> > > > +#include <linux/delay.h>
> > > > +#include <linux/i2c.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/regulator/consumer.h>
> > > > +#include <linux/pm_runtime.h>
> > >
> > > Alphabetical order would be nice.
> > >
> > > > +#include <media/v4l2-async.h>
> > > > +#include <media/v4l2-ctrls.h>
> > > > +#include <media/v4l2-device.h>
> > > > +#include <media/v4l2-subdev.h>
> > > > +
> > > > +#define DW9768_NAME "dw9768"
> > > > +#define DW9768_MAX_FOCUS_POS 1023
> > > > +/*
> > > > + * This sets the minimum granularity for the focus positions.
> > > > + * A value of 1 gives maximum accuracy for a desired focus position
> > > > + */
> > > > +#define DW9768_FOCUS_STEPS 1
> > > > +
> > > > +/*
> > > > + * Ring control and Power control register
> > > > + * Bit[1] RING_EN
> > > > + * 0: Direct mode
> > > > + * 1: AAC mode (ringing control mode)
> > > > + * Bit[0] PD
> > > > + * 0: Normal operation mode
> > > > + * 1: Power down mode
> > > > + * DW9768 requires waiting time of Topr after PD reset takes place.
> > > > + */
> > > > +#define DW9768_RING_PD_CONTROL_REG 0x02
> > > > +#define DW9768_PD_MODE_OFF 0x00
> > > > +#define DW9768_PD_MODE_EN BIT(0)
> > > > +#define DW9768_AAC_MODE_EN BIT(1)
> > > > +
> > > > +/*
> > > > + * DW9768 separates two registers to control the VCM position.
> > > > + * One for MSB value, another is LSB value.
> > > > + * DAC_MSB: D[9:8] (ADD: 0x03)
> > > > + * DAC_LSB: D[7:0] (ADD: 0x04)
> > > > + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> > > > + */
> > > > +#define DW9768_MSB_ADDR 0x03
> > > > +#define DW9768_LSB_ADDR 0x04
> > > > +#define DW9768_STATUS_ADDR 0x05
> > > > +
> > > > +/*
> > > > + * AAC mode control & prescale register
> > > > + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> > > > + * 000 Direct(default)
> > > > + * 001 AAC2 0.48xTvib
> > > > + * 010 AAC3 0.70xTvib
> > > > + * 011 AAC4 0.75xTvib
> > > > + * 100 Reserved
> > > > + * 101 AAC8 1.13xTvib
> > > > + * 110 Reserved
> > > > + * 111 Reserved
> > > > + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> > > > + * 000 2
> > > > + * 001 1(default)
> > > > + * 010 1/2
> > > > + * 011 1/4
> > > > + * 100 8
> > > > + * 101 4
> > > > + * 110 Reserved
> > > > + * 111 Reserved
> > > > + */
> > > > +#define DW9768_AAC_PRESC_REG 0x06
> > > > +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> > > > +
> > > > +/*
> > > > + * VCM period of vibration register
> > > > + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> > > > + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> > > > + * Dividing Rate is the internal clock dividing rate that is defined at
> > > > + * PRESCALE register (ADD: 0x06)
> > > > + */
> > > > +#define DW9768_AAC_TIME_REG 0x07
> > > > +#define DW9768_AACT_CNT 0x39
> > >
> > > I wonder how hardware specific are these values, this and the divider?
> > >
> > > The DW9807 has similar configuration but the defaults seem to be just
> > > fine. What are the defaults in this case?
> > >
> >
> > From hardware specific, the default value of AAC_PRESC_REG is 0x01, and
> > that value of AAC_TIME_REG is 0x20.
> >
> > For these two registers, each bit is defined as below.
> > D[7:0] (D[7:5]-> AC[2:0], D[2:0]-> PRSEC[2:0])
> > +------+------+------+------+------+------+------+------+
> > | AC2 | AC1 | AC0 |------|------|PRESC2|PRESC1|PRESC0|
> > +------+------+------+------+------+------+------+------+
> >
> > D[7:0] (D[5:0]-> AACT[5:0]).
> > +------+------+------+------+------+------+------+------+
> > |------+------+ AACT5| AACT4| AACT3| AACT2| AACT1| AACT0|
> > +------+------+------+------+------+------+------+------+
> >
> > > > +
> > > > +/*
> > > > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > > > + * or in the case of PD reset taking place.
> > > > + */
> > > > +#define DW9768_T_OPR_US 1000
> > > > +
> > > > +/*
> > > > + * 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 DW9768_MOVE_STEPS 16
> > > > +
> > > > +/*
> > > > + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> > > > + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> > > > + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> > > > + */
> > > > +#define DW9768_MOVE_DELAY_US 8400
> > > > +#define DW9768_STABLE_TIME_US 20000
> > > > +
> > > > +static const char * const dw9768_supply_names[] = {
> > > > + "vin", /* I2C I/O interface power */
> > > > + "vdd", /* VCM power */
> > > > +};
> > > > +
> > > > +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
> > >
> > > Please use ARRAY_SIZE() directly instead.
> > >
> > > > +
> > > > +/* dw9768 device structure */
> > > > +struct dw9768 {
> > > > + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> > > > + struct v4l2_ctrl_handler ctrls;
> > > > + struct v4l2_ctrl *focus;
> > > > + struct v4l2_subdev sd;
> > > > +};
> > > > +
> > > > +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> > > > +{
> > > > + return container_of(ctrl->handler, struct dw9768, ctrls);
> > > > +}
> > > > +
> > > > +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> > > > +{
> > > > + return container_of(subdev, struct dw9768, sd);
> > > > +}
> > > > +
> > > > +struct regval_list {
> > > > + u8 reg_num;
> > > > + u8 value;
> > > > +};
> > > > +
> > > > +static struct regval_list dw9768_init_regs[] = {
> > >
> > > const
> > >
> > > > + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> > > > + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> > > > + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> > > > +};
> > > > +
> > > > +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> > > > + size_t len)
> > > > +{
> > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > + unsigned int i;
> > > > + int ret;
> > > > +
> > > > + for (i = 0; i < len; i++) {
> > > > + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> > > > + vals[i].value);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > + }
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> > > > +{
> > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > +
> > > > + /* Write VCM position to registers */
> > > > + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> > > > + swab16(val));
> > > > +}
> > > > +
> > > > +static int dw9768_init(struct dw9768 *dw9768)
> > > > +{
> > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > + int ret, val;
> > > > +
> > > > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > + DW9768_PD_MODE_OFF);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + /*
> > > > + * DW9769 requires waiting delay time of t_OPR
> > > > + * after PD reset takes place.
> > > > + */
> > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > +
> > > > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > > > + ARRAY_SIZE(dw9768_init_regs));
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > > > + val <= dw9768->focus->val;
> > > > + val += DW9768_MOVE_STEPS) {
> > > > + ret = dw9768_set_dac(dw9768, val);
> > > > + if (ret) {
> > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > + __func__, ret);
> > > > + return ret;
> > > > + }
> > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int dw9768_release(struct dw9768 *dw9768)
> > > > +{
> > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > + int ret, val;
> > > > +
> > > > + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > > > + val >= 0; val -= DW9768_MOVE_STEPS) {
> > > > + ret = dw9768_set_dac(dw9768, val);
> > > > + if (ret) {
> > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > + __func__, ret);
> > > > + return ret;
> > > > + }
> > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > + }
> > > > +
> > > > + /*
> > > > + * Wait for the motor to stabilize after the last movement
> > > > + * to prevent the motor from shaking.
> > > > + */
> > > > + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> > > > + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> > > > +
> > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > + DW9768_PD_MODE_EN);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + /*
> > > > + * DW9769 requires waiting delay time of t_OPR
> > > > + * after PD reset takes place.
> > > > + */
> > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +/* Power handling */
> > > > +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> > > > +{
> > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > +
> > > > + dw9768_release(dw9768);
> > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> > > > +{
> > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > + int ret;
> > > > +
> > > > + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "failed to enable regulators\n");
> > > > + return ret;
> > > > + }
> > > > +
> > > > + /*
> > > > + * The datasheet refers to t_OPR that needs to be waited before sending
> > > > + * I2C commands after power-up.
> > > > + */
> > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > +
> > > > + ret = dw9768_init(dw9768);
> > > > + if (ret < 0)
> > > > + goto disable_regulator;
> > > > +
> > > > + return 0;
> > > > +
> > > > +disable_regulator:
> > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> > > > +{
> > > > + struct dw9768 *dw9768 = to_dw9768(ctrl);
> > > > +
> > > > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> > > > + return dw9768_set_dac(dw9768, ctrl->val);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> > > > + .s_ctrl = dw9768_set_ctrl,
> > > > +};
> > > > +
> > > > +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + ret = pm_runtime_get_sync(sd->dev);
> > > > + if (ret < 0) {
> > > > + pm_runtime_put_noidle(sd->dev);
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > +{
> > > > + pm_runtime_put(sd->dev);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> > > > + .open = dw9768_open,
> > > > + .close = dw9768_close,
> > > > +};
> > > > +
> > > > +static const struct v4l2_subdev_ops dw9768_ops = { };
> > > > +
> > > > +static int dw9768_init_controls(struct dw9768 *dw9768)
> > > > +{
> > > > + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> > > > + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> > > > +
> > > > + v4l2_ctrl_handler_init(hdl, 1);
> > > > +
> > > > + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> > > > + 0, DW9768_MAX_FOCUS_POS,
> > > > + DW9768_FOCUS_STEPS, 0);
> > > > +
> > > > + if (hdl->error)
> > > > + return hdl->error;
> > > > +
> > > > + dw9768->sd.ctrl_handler = hdl;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int dw9768_probe(struct i2c_client *client)
> > > > +{
> > > > + struct device *dev = &client->dev;
> > > > + struct dw9768 *dw9768;
> > > > + unsigned int i;
> > > > + int ret;
> > > > +
> > > > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > > > + if (!dw9768)
> > > > + return -ENOMEM;
> > > > +
> > > > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > > > +
> > > > + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> > > > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > > > +
> > > > + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> > > > + dw9768->supplies);
> > > > + if (ret) {
> > > > + dev_err(dev, "failed to get regulators\n");
> > > > + return ret;
> > > > + }
> > > > +
> > >
> > > I'd try to see the chip is accessible in probe().
> > >
> >
> > If probe failed, actuator device node would not be generated.
>
> Yes, this would be preferred.
>
> > When user tries to open fd, it should report error also.
> >
> > > > + ret = dw9768_init_controls(dw9768);
> > > > + if (ret)
> > > > + goto entity_cleanup;
> > > > +
> > > > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > > > +
> > > > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > > > + if (ret < 0)
> > > > + goto entity_cleanup;
> > > > +
> > > > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > > > +
> > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > + if (ret < 0)
> > > > + goto entity_cleanup;
> > > > +
> > > > + pm_runtime_enable(dev);
> > >
> > > Your driver appears to depend on runtime PM on DT based systems.
> > >
> > > You should either add a dependency to CONFIG_PM, or much more preferrably
> > > make it work without runtime PM.
> > >
> >
> > Do you mean using the macro like this:
> > #ifdef CONFIG_PM
> > ...
> > #endif
>
> No. If CONFIG_PM is disabled, the runtime PM functions do nothing. Would
> your driver work in that case?
>
If we make the driver handle the case of !IS_ENABLED(CONFIG_PM), I'd
like us to make sure that the driver only does power up in this
specific case to avoid blinking the camera LED on boot-up when
CONFIG_PM is enabled.
Best regards,
Tomasz
> >
> > > > +
> > > > + return 0;
> > > > +
> > > > +entity_cleanup:
> > > > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > > > + media_entity_cleanup(&dw9768->sd.entity);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int dw9768_remove(struct i2c_client *client)
> > > > +{
> > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > +
> > > > + pm_runtime_disable(&client->dev);
> > > > + v4l2_async_unregister_subdev(&dw9768->sd);
> > > > + v4l2_ctrl_handler_free(&dw9768->ctrls);
> > > > + media_entity_cleanup(&dw9768->sd.entity);
> > > > + if (!pm_runtime_status_suspended(&client->dev))
> > > > + dw9768_runtime_suspend(&client->dev);
> > > > + pm_runtime_set_suspended(&client->dev);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static const struct of_device_id dw9768_of_table[] = {
> > > > + { .compatible = "dongwoon,dw9768" },
> > > > + {}
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, dw9768_of_table);
> > > > +
> > > > +static const struct dev_pm_ops dw9768_pm_ops = {
> > > > + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > > > + pm_runtime_force_resume)
> > > > + SET_RUNTIME_PM_OPS(dw9768_runtime_suspend, dw9768_runtime_resume, NULL)
> > > > +};
> > > > +
> > > > +static struct i2c_driver dw9768_i2c_driver = {
> > > > + .driver = {
> > > > + .name = DW9768_NAME,
> > > > + .pm = &dw9768_pm_ops,
> > > > + .of_match_table = dw9768_of_table,
> > > > + },
> > > > + .probe_new = dw9768_probe,
> > > > + .remove = dw9768_remove,
> > > > +};
> > > > +
> > > > +module_i2c_driver(dw9768_i2c_driver);
> > > > +
> > > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > > +MODULE_DESCRIPTION("DW9768 VCM driver");
> > > > +MODULE_LICENSE("GPL v2");
> > >
> >
>
> --
> Regards,
>
> Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-23 19:09 ` Tomasz Figa
@ 2020-03-23 20:35 ` Sakari Ailus
2020-03-25 15:11 ` Tomasz Figa
0 siblings, 1 reply; 17+ messages in thread
From: Sakari Ailus @ 2020-03-23 20:35 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Rutland, Nicolas Boichat, andriy.shevchenko, srv_heupstream,
linux-devicetree, Shengnan Wang (王圣男),
Louis Kuo, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Dongchun Zhu,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
Hi Tomasz,
On Mon, Mar 23, 2020 at 08:09:14PM +0100, Tomasz Figa wrote:
> On Wed, Mar 18, 2020 at 11:29 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Dongchun,
> >
> > On Wed, Mar 18, 2020 at 06:20:24PM +0800, Dongchun Zhu wrote:
> > > Hi Sakari,
> > >
> > > On Thu, 2020-03-05 at 14:05 +0200, Sakari Ailus wrote:
> > > > Hi Dongchun,
> > > >
> > > > Thanks for the update.
> > > >
> > > > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > > > and provides control to set the desired focus via I2C serial interface.
> > > > >
> > > > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > > > ---
> > > > > MAINTAINERS | 1 +
> > > > > drivers/media/i2c/Kconfig | 10 ++
> > > > > drivers/media/i2c/Makefile | 1 +
> > > > > drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > 4 files changed, 449 insertions(+)
> > > > > create mode 100644 drivers/media/i2c/dw9768.c
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index b805e29..0bb894a 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -5139,6 +5139,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.yaml
> > > > >
> > > > > DONGWOON DW9807 LENS VOICE COIL DRIVER
> > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > > > index c68e002..aa60781 100644
> > > > > --- a/drivers/media/i2c/Kconfig
> > > > > +++ b/drivers/media/i2c/Kconfig
> > > > > @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> > > > > --- /dev/null
> > > > > +++ b/drivers/media/i2c/dw9768.c
> > > > > @@ -0,0 +1,437 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +// Copyright (c) 2020 MediaTek Inc.
> > > > > +
> > > > > +#include <linux/delay.h>
> > > > > +#include <linux/i2c.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/regulator/consumer.h>
> > > > > +#include <linux/pm_runtime.h>
> > > >
> > > > Alphabetical order would be nice.
> > > >
> > > > > +#include <media/v4l2-async.h>
> > > > > +#include <media/v4l2-ctrls.h>
> > > > > +#include <media/v4l2-device.h>
> > > > > +#include <media/v4l2-subdev.h>
> > > > > +
> > > > > +#define DW9768_NAME "dw9768"
> > > > > +#define DW9768_MAX_FOCUS_POS 1023
> > > > > +/*
> > > > > + * This sets the minimum granularity for the focus positions.
> > > > > + * A value of 1 gives maximum accuracy for a desired focus position
> > > > > + */
> > > > > +#define DW9768_FOCUS_STEPS 1
> > > > > +
> > > > > +/*
> > > > > + * Ring control and Power control register
> > > > > + * Bit[1] RING_EN
> > > > > + * 0: Direct mode
> > > > > + * 1: AAC mode (ringing control mode)
> > > > > + * Bit[0] PD
> > > > > + * 0: Normal operation mode
> > > > > + * 1: Power down mode
> > > > > + * DW9768 requires waiting time of Topr after PD reset takes place.
> > > > > + */
> > > > > +#define DW9768_RING_PD_CONTROL_REG 0x02
> > > > > +#define DW9768_PD_MODE_OFF 0x00
> > > > > +#define DW9768_PD_MODE_EN BIT(0)
> > > > > +#define DW9768_AAC_MODE_EN BIT(1)
> > > > > +
> > > > > +/*
> > > > > + * DW9768 separates two registers to control the VCM position.
> > > > > + * One for MSB value, another is LSB value.
> > > > > + * DAC_MSB: D[9:8] (ADD: 0x03)
> > > > > + * DAC_LSB: D[7:0] (ADD: 0x04)
> > > > > + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> > > > > + */
> > > > > +#define DW9768_MSB_ADDR 0x03
> > > > > +#define DW9768_LSB_ADDR 0x04
> > > > > +#define DW9768_STATUS_ADDR 0x05
> > > > > +
> > > > > +/*
> > > > > + * AAC mode control & prescale register
> > > > > + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> > > > > + * 000 Direct(default)
> > > > > + * 001 AAC2 0.48xTvib
> > > > > + * 010 AAC3 0.70xTvib
> > > > > + * 011 AAC4 0.75xTvib
> > > > > + * 100 Reserved
> > > > > + * 101 AAC8 1.13xTvib
> > > > > + * 110 Reserved
> > > > > + * 111 Reserved
> > > > > + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> > > > > + * 000 2
> > > > > + * 001 1(default)
> > > > > + * 010 1/2
> > > > > + * 011 1/4
> > > > > + * 100 8
> > > > > + * 101 4
> > > > > + * 110 Reserved
> > > > > + * 111 Reserved
> > > > > + */
> > > > > +#define DW9768_AAC_PRESC_REG 0x06
> > > > > +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> > > > > +
> > > > > +/*
> > > > > + * VCM period of vibration register
> > > > > + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> > > > > + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> > > > > + * Dividing Rate is the internal clock dividing rate that is defined at
> > > > > + * PRESCALE register (ADD: 0x06)
> > > > > + */
> > > > > +#define DW9768_AAC_TIME_REG 0x07
> > > > > +#define DW9768_AACT_CNT 0x39
> > > >
> > > > I wonder how hardware specific are these values, this and the divider?
> > > >
> > > > The DW9807 has similar configuration but the defaults seem to be just
> > > > fine. What are the defaults in this case?
> > > >
> > >
> > > From hardware specific, the default value of AAC_PRESC_REG is 0x01, and
> > > that value of AAC_TIME_REG is 0x20.
> > >
> > > For these two registers, each bit is defined as below.
> > > D[7:0] (D[7:5]-> AC[2:0], D[2:0]-> PRSEC[2:0])
> > > +------+------+------+------+------+------+------+------+
> > > | AC2 | AC1 | AC0 |------|------|PRESC2|PRESC1|PRESC0|
> > > +------+------+------+------+------+------+------+------+
> > >
> > > D[7:0] (D[5:0]-> AACT[5:0]).
> > > +------+------+------+------+------+------+------+------+
> > > |------+------+ AACT5| AACT4| AACT3| AACT2| AACT1| AACT0|
> > > +------+------+------+------+------+------+------+------+
> > >
> > > > > +
> > > > > +/*
> > > > > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > > > > + * or in the case of PD reset taking place.
> > > > > + */
> > > > > +#define DW9768_T_OPR_US 1000
> > > > > +
> > > > > +/*
> > > > > + * 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 DW9768_MOVE_STEPS 16
> > > > > +
> > > > > +/*
> > > > > + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> > > > > + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> > > > > + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> > > > > + */
> > > > > +#define DW9768_MOVE_DELAY_US 8400
> > > > > +#define DW9768_STABLE_TIME_US 20000
> > > > > +
> > > > > +static const char * const dw9768_supply_names[] = {
> > > > > + "vin", /* I2C I/O interface power */
> > > > > + "vdd", /* VCM power */
> > > > > +};
> > > > > +
> > > > > +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
> > > >
> > > > Please use ARRAY_SIZE() directly instead.
> > > >
> > > > > +
> > > > > +/* dw9768 device structure */
> > > > > +struct dw9768 {
> > > > > + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> > > > > + struct v4l2_ctrl_handler ctrls;
> > > > > + struct v4l2_ctrl *focus;
> > > > > + struct v4l2_subdev sd;
> > > > > +};
> > > > > +
> > > > > +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> > > > > +{
> > > > > + return container_of(ctrl->handler, struct dw9768, ctrls);
> > > > > +}
> > > > > +
> > > > > +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> > > > > +{
> > > > > + return container_of(subdev, struct dw9768, sd);
> > > > > +}
> > > > > +
> > > > > +struct regval_list {
> > > > > + u8 reg_num;
> > > > > + u8 value;
> > > > > +};
> > > > > +
> > > > > +static struct regval_list dw9768_init_regs[] = {
> > > >
> > > > const
> > > >
> > > > > + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> > > > > + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> > > > > + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> > > > > +};
> > > > > +
> > > > > +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> > > > > + size_t len)
> > > > > +{
> > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > + unsigned int i;
> > > > > + int ret;
> > > > > +
> > > > > + for (i = 0; i < len; i++) {
> > > > > + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> > > > > + vals[i].value);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > + }
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> > > > > +{
> > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > +
> > > > > + /* Write VCM position to registers */
> > > > > + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> > > > > + swab16(val));
> > > > > +}
> > > > > +
> > > > > +static int dw9768_init(struct dw9768 *dw9768)
> > > > > +{
> > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > + int ret, val;
> > > > > +
> > > > > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > > + DW9768_PD_MODE_OFF);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + /*
> > > > > + * DW9769 requires waiting delay time of t_OPR
> > > > > + * after PD reset takes place.
> > > > > + */
> > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > +
> > > > > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > > > > + ARRAY_SIZE(dw9768_init_regs));
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > > > > + val <= dw9768->focus->val;
> > > > > + val += DW9768_MOVE_STEPS) {
> > > > > + ret = dw9768_set_dac(dw9768, val);
> > > > > + if (ret) {
> > > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > > + __func__, ret);
> > > > > + return ret;
> > > > > + }
> > > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dw9768_release(struct dw9768 *dw9768)
> > > > > +{
> > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > + int ret, val;
> > > > > +
> > > > > + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > > > > + val >= 0; val -= DW9768_MOVE_STEPS) {
> > > > > + ret = dw9768_set_dac(dw9768, val);
> > > > > + if (ret) {
> > > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > > + __func__, ret);
> > > > > + return ret;
> > > > > + }
> > > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > > + }
> > > > > +
> > > > > + /*
> > > > > + * Wait for the motor to stabilize after the last movement
> > > > > + * to prevent the motor from shaking.
> > > > > + */
> > > > > + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> > > > > + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> > > > > +
> > > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > > + DW9768_PD_MODE_EN);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + /*
> > > > > + * DW9769 requires waiting delay time of t_OPR
> > > > > + * after PD reset takes place.
> > > > > + */
> > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +/* Power handling */
> > > > > +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> > > > > +{
> > > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > > +
> > > > > + dw9768_release(dw9768);
> > > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> > > > > +{
> > > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > > + int ret;
> > > > > +
> > > > > + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > + if (ret < 0) {
> > > > > + dev_err(dev, "failed to enable regulators\n");
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > > > + /*
> > > > > + * The datasheet refers to t_OPR that needs to be waited before sending
> > > > > + * I2C commands after power-up.
> > > > > + */
> > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > +
> > > > > + ret = dw9768_init(dw9768);
> > > > > + if (ret < 0)
> > > > > + goto disable_regulator;
> > > > > +
> > > > > + return 0;
> > > > > +
> > > > > +disable_regulator:
> > > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > +
> > > > > + return ret;
> > > > > +}
> > > > > +
> > > > > +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> > > > > +{
> > > > > + struct dw9768 *dw9768 = to_dw9768(ctrl);
> > > > > +
> > > > > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> > > > > + return dw9768_set_dac(dw9768, ctrl->val);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> > > > > + .s_ctrl = dw9768_set_ctrl,
> > > > > +};
> > > > > +
> > > > > +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + ret = pm_runtime_get_sync(sd->dev);
> > > > > + if (ret < 0) {
> > > > > + pm_runtime_put_noidle(sd->dev);
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > > +{
> > > > > + pm_runtime_put(sd->dev);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> > > > > + .open = dw9768_open,
> > > > > + .close = dw9768_close,
> > > > > +};
> > > > > +
> > > > > +static const struct v4l2_subdev_ops dw9768_ops = { };
> > > > > +
> > > > > +static int dw9768_init_controls(struct dw9768 *dw9768)
> > > > > +{
> > > > > + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> > > > > + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> > > > > +
> > > > > + v4l2_ctrl_handler_init(hdl, 1);
> > > > > +
> > > > > + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> > > > > + 0, DW9768_MAX_FOCUS_POS,
> > > > > + DW9768_FOCUS_STEPS, 0);
> > > > > +
> > > > > + if (hdl->error)
> > > > > + return hdl->error;
> > > > > +
> > > > > + dw9768->sd.ctrl_handler = hdl;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int dw9768_probe(struct i2c_client *client)
> > > > > +{
> > > > > + struct device *dev = &client->dev;
> > > > > + struct dw9768 *dw9768;
> > > > > + unsigned int i;
> > > > > + int ret;
> > > > > +
> > > > > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > > > > + if (!dw9768)
> > > > > + return -ENOMEM;
> > > > > +
> > > > > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > > > > +
> > > > > + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> > > > > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > > > > +
> > > > > + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> > > > > + dw9768->supplies);
> > > > > + if (ret) {
> > > > > + dev_err(dev, "failed to get regulators\n");
> > > > > + return ret;
> > > > > + }
> > > > > +
> > > >
> > > > I'd try to see the chip is accessible in probe().
> > > >
> > >
> > > If probe failed, actuator device node would not be generated.
> >
> > Yes, this would be preferred.
> >
> > > When user tries to open fd, it should report error also.
> > >
> > > > > + ret = dw9768_init_controls(dw9768);
> > > > > + if (ret)
> > > > > + goto entity_cleanup;
> > > > > +
> > > > > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > > > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > > > > +
> > > > > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > > > > + if (ret < 0)
> > > > > + goto entity_cleanup;
> > > > > +
> > > > > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > > > > +
> > > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > + if (ret < 0)
> > > > > + goto entity_cleanup;
> > > > > +
> > > > > + pm_runtime_enable(dev);
> > > >
> > > > Your driver appears to depend on runtime PM on DT based systems.
> > > >
> > > > You should either add a dependency to CONFIG_PM, or much more preferrably
> > > > make it work without runtime PM.
> > > >
> > >
> > > Do you mean using the macro like this:
> > > #ifdef CONFIG_PM
> > > ...
> > > #endif
> >
> > No. If CONFIG_PM is disabled, the runtime PM functions do nothing. Would
> > your driver work in that case?
> >
>
> If we make the driver handle the case of !IS_ENABLED(CONFIG_PM), I'd
> like us to make sure that the driver only does power up in this
> specific case to avoid blinking the camera LED on boot-up when
> CONFIG_PM is enabled.
Ah, you have that kind of hardware design again. :-)
I guess that's fine for a VCM driver; just don't power the device on during
probe. On the other hand, for an EEPROM that are common in sensors, that
likely isn't so straightforward.
I wonder what kind of a reception would be for a DT property to tell
powering the device on during probe is undesirable. Which reminds me ---
time to respin the corresponding set for ACPI...
--
Regards,
Sakari Ailus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [V3, 2/2] media: i2c: Add DW9768 VCM driver
2020-03-23 20:35 ` Sakari Ailus
@ 2020-03-25 15:11 ` Tomasz Figa
0 siblings, 0 replies; 17+ messages in thread
From: Tomasz Figa @ 2020-03-25 15:11 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, andriy.shevchenko, srv_heupstream,
linux-devicetree, Shengnan Wang (王圣男),
Louis Kuo, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Dongchun Zhu,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS
<iommu@lists.linux-foundation.org>,
Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
On Mon, Mar 23, 2020 at 9:36 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Tomasz,
>
> On Mon, Mar 23, 2020 at 08:09:14PM +0100, Tomasz Figa wrote:
> > On Wed, Mar 18, 2020 at 11:29 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Dongchun,
> > >
> > > On Wed, Mar 18, 2020 at 06:20:24PM +0800, Dongchun Zhu wrote:
> > > > Hi Sakari,
> > > >
> > > > On Thu, 2020-03-05 at 14:05 +0200, Sakari Ailus wrote:
> > > > > Hi Dongchun,
> > > > >
> > > > > Thanks for the update.
> > > > >
> > > > > On Fri, Feb 28, 2020 at 11:59:58PM +0800, Dongchun Zhu wrote:
> > > > > > This patch adds a V4L2 sub-device driver for DW9768 lens voice coil,
> > > > > > and provides control to set the desired focus via I2C serial interface.
> > > > > >
> > > > > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > > > > ---
> > > > > > MAINTAINERS | 1 +
> > > > > > drivers/media/i2c/Kconfig | 10 ++
> > > > > > drivers/media/i2c/Makefile | 1 +
> > > > > > drivers/media/i2c/dw9768.c | 437 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 4 files changed, 449 insertions(+)
> > > > > > create mode 100644 drivers/media/i2c/dw9768.c
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index b805e29..0bb894a 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -5139,6 +5139,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.yaml
> > > > > >
> > > > > > DONGWOON DW9807 LENS VOICE COIL DRIVER
> > > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > > > > > index c68e002..aa60781 100644
> > > > > > --- a/drivers/media/i2c/Kconfig
> > > > > > +++ b/drivers/media/i2c/Kconfig
> > > > > > @@ -1024,6 +1024,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 c147bb9..ec94434 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..dec1abc
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/media/i2c/dw9768.c
> > > > > > @@ -0,0 +1,437 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +// Copyright (c) 2020 MediaTek Inc.
> > > > > > +
> > > > > > +#include <linux/delay.h>
> > > > > > +#include <linux/i2c.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/regulator/consumer.h>
> > > > > > +#include <linux/pm_runtime.h>
> > > > >
> > > > > Alphabetical order would be nice.
> > > > >
> > > > > > +#include <media/v4l2-async.h>
> > > > > > +#include <media/v4l2-ctrls.h>
> > > > > > +#include <media/v4l2-device.h>
> > > > > > +#include <media/v4l2-subdev.h>
> > > > > > +
> > > > > > +#define DW9768_NAME "dw9768"
> > > > > > +#define DW9768_MAX_FOCUS_POS 1023
> > > > > > +/*
> > > > > > + * This sets the minimum granularity for the focus positions.
> > > > > > + * A value of 1 gives maximum accuracy for a desired focus position
> > > > > > + */
> > > > > > +#define DW9768_FOCUS_STEPS 1
> > > > > > +
> > > > > > +/*
> > > > > > + * Ring control and Power control register
> > > > > > + * Bit[1] RING_EN
> > > > > > + * 0: Direct mode
> > > > > > + * 1: AAC mode (ringing control mode)
> > > > > > + * Bit[0] PD
> > > > > > + * 0: Normal operation mode
> > > > > > + * 1: Power down mode
> > > > > > + * DW9768 requires waiting time of Topr after PD reset takes place.
> > > > > > + */
> > > > > > +#define DW9768_RING_PD_CONTROL_REG 0x02
> > > > > > +#define DW9768_PD_MODE_OFF 0x00
> > > > > > +#define DW9768_PD_MODE_EN BIT(0)
> > > > > > +#define DW9768_AAC_MODE_EN BIT(1)
> > > > > > +
> > > > > > +/*
> > > > > > + * DW9768 separates two registers to control the VCM position.
> > > > > > + * One for MSB value, another is LSB value.
> > > > > > + * DAC_MSB: D[9:8] (ADD: 0x03)
> > > > > > + * DAC_LSB: D[7:0] (ADD: 0x04)
> > > > > > + * D[9:0] DAC data input: positive output current = D[9:0] / 1023 * 100[mA]
> > > > > > + */
> > > > > > +#define DW9768_MSB_ADDR 0x03
> > > > > > +#define DW9768_LSB_ADDR 0x04
> > > > > > +#define DW9768_STATUS_ADDR 0x05
> > > > > > +
> > > > > > +/*
> > > > > > + * AAC mode control & prescale register
> > > > > > + * Bit[7:5] Namely AC[2:0], decide the VCM mode and operation time.
> > > > > > + * 000 Direct(default)
> > > > > > + * 001 AAC2 0.48xTvib
> > > > > > + * 010 AAC3 0.70xTvib
> > > > > > + * 011 AAC4 0.75xTvib
> > > > > > + * 100 Reserved
> > > > > > + * 101 AAC8 1.13xTvib
> > > > > > + * 110 Reserved
> > > > > > + * 111 Reserved
> > > > > > + * Bit[2:0] Namely PRESC[2:0], set the internal clock dividing rate as follow.
> > > > > > + * 000 2
> > > > > > + * 001 1(default)
> > > > > > + * 010 1/2
> > > > > > + * 011 1/4
> > > > > > + * 100 8
> > > > > > + * 101 4
> > > > > > + * 110 Reserved
> > > > > > + * 111 Reserved
> > > > > > + */
> > > > > > +#define DW9768_AAC_PRESC_REG 0x06
> > > > > > +#define DW9768_AAC3_SELECT_DIVIDING_RATE_1 0x41
> > > > > > +
> > > > > > +/*
> > > > > > + * VCM period of vibration register
> > > > > > + * Bit[5:0] Defined as VCM rising periodic time (Tvib) together with PRESC[2:0]
> > > > > > + * Tvib = (6.3ms + AACT[5:0] * 0.1ms) * Dividing Rate
> > > > > > + * Dividing Rate is the internal clock dividing rate that is defined at
> > > > > > + * PRESCALE register (ADD: 0x06)
> > > > > > + */
> > > > > > +#define DW9768_AAC_TIME_REG 0x07
> > > > > > +#define DW9768_AACT_CNT 0x39
> > > > >
> > > > > I wonder how hardware specific are these values, this and the divider?
> > > > >
> > > > > The DW9807 has similar configuration but the defaults seem to be just
> > > > > fine. What are the defaults in this case?
> > > > >
> > > >
> > > > From hardware specific, the default value of AAC_PRESC_REG is 0x01, and
> > > > that value of AAC_TIME_REG is 0x20.
> > > >
> > > > For these two registers, each bit is defined as below.
> > > > D[7:0] (D[7:5]-> AC[2:0], D[2:0]-> PRSEC[2:0])
> > > > +------+------+------+------+------+------+------+------+
> > > > | AC2 | AC1 | AC0 |------|------|PRESC2|PRESC1|PRESC0|
> > > > +------+------+------+------+------+------+------+------+
> > > >
> > > > D[7:0] (D[5:0]-> AACT[5:0]).
> > > > +------+------+------+------+------+------+------+------+
> > > > |------+------+ AACT5| AACT4| AACT3| AACT2| AACT1| AACT0|
> > > > +------+------+------+------+------+------+------+------+
> > > >
> > > > > > +
> > > > > > +/*
> > > > > > + * DW9768 requires waiting time (delay time) of t_OPR after power-up,
> > > > > > + * or in the case of PD reset taking place.
> > > > > > + */
> > > > > > +#define DW9768_T_OPR_US 1000
> > > > > > +
> > > > > > +/*
> > > > > > + * 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 DW9768_MOVE_STEPS 16
> > > > > > +
> > > > > > +/*
> > > > > > + * DW9768_AAC_PRESC_REG & DW9768_AAC_TIME_REG determine VCM operation time.
> > > > > > + * If DW9768_AAC_PRESC_REG set to be 0x41, DW9768_AAC_TIME_REG set to be 0x39,
> > > > > > + * VCM mode would be AAC3, Operation Time would be 0.70xTvib, that is 8.40ms.
> > > > > > + */
> > > > > > +#define DW9768_MOVE_DELAY_US 8400
> > > > > > +#define DW9768_STABLE_TIME_US 20000
> > > > > > +
> > > > > > +static const char * const dw9768_supply_names[] = {
> > > > > > + "vin", /* I2C I/O interface power */
> > > > > > + "vdd", /* VCM power */
> > > > > > +};
> > > > > > +
> > > > > > +#define DW9768_NUM_SUPPLIES ARRAY_SIZE(dw9768_supply_names)
> > > > >
> > > > > Please use ARRAY_SIZE() directly instead.
> > > > >
> > > > > > +
> > > > > > +/* dw9768 device structure */
> > > > > > +struct dw9768 {
> > > > > > + struct regulator_bulk_data supplies[DW9768_NUM_SUPPLIES];
> > > > > > + struct v4l2_ctrl_handler ctrls;
> > > > > > + struct v4l2_ctrl *focus;
> > > > > > + struct v4l2_subdev sd;
> > > > > > +};
> > > > > > +
> > > > > > +static inline struct dw9768 *to_dw9768(struct v4l2_ctrl *ctrl)
> > > > > > +{
> > > > > > + return container_of(ctrl->handler, struct dw9768, ctrls);
> > > > > > +}
> > > > > > +
> > > > > > +static inline struct dw9768 *sd_to_dw9768(struct v4l2_subdev *subdev)
> > > > > > +{
> > > > > > + return container_of(subdev, struct dw9768, sd);
> > > > > > +}
> > > > > > +
> > > > > > +struct regval_list {
> > > > > > + u8 reg_num;
> > > > > > + u8 value;
> > > > > > +};
> > > > > > +
> > > > > > +static struct regval_list dw9768_init_regs[] = {
> > > > >
> > > > > const
> > > > >
> > > > > > + {DW9768_RING_PD_CONTROL_REG, DW9768_AAC_MODE_EN},
> > > > > > + {DW9768_AAC_PRESC_REG, DW9768_AAC3_SELECT_DIVIDING_RATE_1},
> > > > > > + {DW9768_AAC_TIME_REG, DW9768_AACT_CNT},
> > > > > > +};
> > > > > > +
> > > > > > +static int dw9768_write_array(struct dw9768 *dw9768, struct regval_list *vals,
> > > > > > + size_t len)
> > > > > > +{
> > > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > > + unsigned int i;
> > > > > > + int ret;
> > > > > > +
> > > > > > + for (i = 0; i < len; i++) {
> > > > > > + ret = i2c_smbus_write_byte_data(client, vals[i].reg_num,
> > > > > > + vals[i].value);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > + }
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_set_dac(struct dw9768 *dw9768, u16 val)
> > > > > > +{
> > > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > > +
> > > > > > + /* Write VCM position to registers */
> > > > > > + return i2c_smbus_write_word_data(client, DW9768_MSB_ADDR,
> > > > > > + swab16(val));
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_init(struct dw9768 *dw9768)
> > > > > > +{
> > > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > > + int ret, val;
> > > > > > +
> > > > > > + /* Reset DW9768_RING_PD_CONTROL_REG to default status 0x00 */
> > > > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > > > + DW9768_PD_MODE_OFF);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + /*
> > > > > > + * DW9769 requires waiting delay time of t_OPR
> > > > > > + * after PD reset takes place.
> > > > > > + */
> > > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > > +
> > > > > > + ret = dw9768_write_array(dw9768, dw9768_init_regs,
> > > > > > + ARRAY_SIZE(dw9768_init_regs));
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + for (val = dw9768->focus->val % DW9768_MOVE_STEPS;
> > > > > > + val <= dw9768->focus->val;
> > > > > > + val += DW9768_MOVE_STEPS) {
> > > > > > + ret = dw9768_set_dac(dw9768, val);
> > > > > > + if (ret) {
> > > > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > > > + __func__, ret);
> > > > > > + return ret;
> > > > > > + }
> > > > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > > > + }
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_release(struct dw9768 *dw9768)
> > > > > > +{
> > > > > > + struct i2c_client *client = v4l2_get_subdevdata(&dw9768->sd);
> > > > > > + int ret, val;
> > > > > > +
> > > > > > + for (val = round_down(dw9768->focus->val, DW9768_MOVE_STEPS);
> > > > > > + val >= 0; val -= DW9768_MOVE_STEPS) {
> > > > > > + ret = dw9768_set_dac(dw9768, val);
> > > > > > + if (ret) {
> > > > > > + dev_err(&client->dev, "%s I2C failure: %d",
> > > > > > + __func__, ret);
> > > > > > + return ret;
> > > > > > + }
> > > > > > + usleep_range(DW9768_MOVE_DELAY_US,
> > > > > > + DW9768_MOVE_DELAY_US + 1000);
> > > > > > + }
> > > > > > +
> > > > > > + /*
> > > > > > + * Wait for the motor to stabilize after the last movement
> > > > > > + * to prevent the motor from shaking.
> > > > > > + */
> > > > > > + usleep_range(DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US,
> > > > > > + DW9768_STABLE_TIME_US - DW9768_MOVE_DELAY_US + 1000);
> > > > > > +
> > > > > > + ret = i2c_smbus_write_byte_data(client, DW9768_RING_PD_CONTROL_REG,
> > > > > > + DW9768_PD_MODE_EN);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + /*
> > > > > > + * DW9769 requires waiting delay time of t_OPR
> > > > > > + * after PD reset takes place.
> > > > > > + */
> > > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +/* Power handling */
> > > > > > +static int __maybe_unused dw9768_runtime_suspend(struct device *dev)
> > > > > > +{
> > > > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > > > +
> > > > > > + dw9768_release(dw9768);
> > > > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int __maybe_unused dw9768_runtime_resume(struct device *dev)
> > > > > > +{
> > > > > > + struct i2c_client *client = to_i2c_client(dev);
> > > > > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > > > > + struct dw9768 *dw9768 = sd_to_dw9768(sd);
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = regulator_bulk_enable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > > + if (ret < 0) {
> > > > > > + dev_err(dev, "failed to enable regulators\n");
> > > > > > + return ret;
> > > > > > + }
> > > > > > +
> > > > > > + /*
> > > > > > + * The datasheet refers to t_OPR that needs to be waited before sending
> > > > > > + * I2C commands after power-up.
> > > > > > + */
> > > > > > + usleep_range(DW9768_T_OPR_US, DW9768_T_OPR_US + 100);
> > > > > > +
> > > > > > + ret = dw9768_init(dw9768);
> > > > > > + if (ret < 0)
> > > > > > + goto disable_regulator;
> > > > > > +
> > > > > > + return 0;
> > > > > > +
> > > > > > +disable_regulator:
> > > > > > + regulator_bulk_disable(DW9768_NUM_SUPPLIES, dw9768->supplies);
> > > > > > +
> > > > > > + return ret;
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_set_ctrl(struct v4l2_ctrl *ctrl)
> > > > > > +{
> > > > > > + struct dw9768 *dw9768 = to_dw9768(ctrl);
> > > > > > +
> > > > > > + if (ctrl->id == V4L2_CID_FOCUS_ABSOLUTE)
> > > > > > + return dw9768_set_dac(dw9768, ctrl->val);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static const struct v4l2_ctrl_ops dw9768_ctrl_ops = {
> > > > > > + .s_ctrl = dw9768_set_ctrl,
> > > > > > +};
> > > > > > +
> > > > > > +static int dw9768_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > > > +{
> > > > > > + int ret;
> > > > > > +
> > > > > > + ret = pm_runtime_get_sync(sd->dev);
> > > > > > + if (ret < 0) {
> > > > > > + pm_runtime_put_noidle(sd->dev);
> > > > > > + return ret;
> > > > > > + }
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > > > > > +{
> > > > > > + pm_runtime_put(sd->dev);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static const struct v4l2_subdev_internal_ops dw9768_int_ops = {
> > > > > > + .open = dw9768_open,
> > > > > > + .close = dw9768_close,
> > > > > > +};
> > > > > > +
> > > > > > +static const struct v4l2_subdev_ops dw9768_ops = { };
> > > > > > +
> > > > > > +static int dw9768_init_controls(struct dw9768 *dw9768)
> > > > > > +{
> > > > > > + struct v4l2_ctrl_handler *hdl = &dw9768->ctrls;
> > > > > > + const struct v4l2_ctrl_ops *ops = &dw9768_ctrl_ops;
> > > > > > +
> > > > > > + v4l2_ctrl_handler_init(hdl, 1);
> > > > > > +
> > > > > > + dw9768->focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
> > > > > > + 0, DW9768_MAX_FOCUS_POS,
> > > > > > + DW9768_FOCUS_STEPS, 0);
> > > > > > +
> > > > > > + if (hdl->error)
> > > > > > + return hdl->error;
> > > > > > +
> > > > > > + dw9768->sd.ctrl_handler = hdl;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static int dw9768_probe(struct i2c_client *client)
> > > > > > +{
> > > > > > + struct device *dev = &client->dev;
> > > > > > + struct dw9768 *dw9768;
> > > > > > + unsigned int i;
> > > > > > + int ret;
> > > > > > +
> > > > > > + dw9768 = devm_kzalloc(dev, sizeof(*dw9768), GFP_KERNEL);
> > > > > > + if (!dw9768)
> > > > > > + return -ENOMEM;
> > > > > > +
> > > > > > + v4l2_i2c_subdev_init(&dw9768->sd, client, &dw9768_ops);
> > > > > > +
> > > > > > + for (i = 0; i < DW9768_NUM_SUPPLIES; i++)
> > > > > > + dw9768->supplies[i].supply = dw9768_supply_names[i];
> > > > > > +
> > > > > > + ret = devm_regulator_bulk_get(dev, DW9768_NUM_SUPPLIES,
> > > > > > + dw9768->supplies);
> > > > > > + if (ret) {
> > > > > > + dev_err(dev, "failed to get regulators\n");
> > > > > > + return ret;
> > > > > > + }
> > > > > > +
> > > > >
> > > > > I'd try to see the chip is accessible in probe().
> > > > >
> > > >
> > > > If probe failed, actuator device node would not be generated.
> > >
> > > Yes, this would be preferred.
> > >
> > > > When user tries to open fd, it should report error also.
> > > >
> > > > > > + ret = dw9768_init_controls(dw9768);
> > > > > > + if (ret)
> > > > > > + goto entity_cleanup;
> > > > > > +
> > > > > > + dw9768->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > > > > + dw9768->sd.internal_ops = &dw9768_int_ops;
> > > > > > +
> > > > > > + ret = media_entity_pads_init(&dw9768->sd.entity, 0, NULL);
> > > > > > + if (ret < 0)
> > > > > > + goto entity_cleanup;
> > > > > > +
> > > > > > + dw9768->sd.entity.function = MEDIA_ENT_F_LENS;
> > > > > > +
> > > > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > > + if (ret < 0)
> > > > > > + goto entity_cleanup;
> > > > > > +
> > > > > > + pm_runtime_enable(dev);
> > > > >
> > > > > Your driver appears to depend on runtime PM on DT based systems.
> > > > >
> > > > > You should either add a dependency to CONFIG_PM, or much more preferrably
> > > > > make it work without runtime PM.
> > > > >
> > > >
> > > > Do you mean using the macro like this:
> > > > #ifdef CONFIG_PM
> > > > ...
> > > > #endif
> > >
> > > No. If CONFIG_PM is disabled, the runtime PM functions do nothing. Would
> > > your driver work in that case?
> > >
> >
> > If we make the driver handle the case of !IS_ENABLED(CONFIG_PM), I'd
> > like us to make sure that the driver only does power up in this
> > specific case to avoid blinking the camera LED on boot-up when
> > CONFIG_PM is enabled.
>
> Ah, you have that kind of hardware design again. :-)
>
Yeah. Unfortunately it's not easy to avoid, as the sensors today don't
really provide any secure way to drive a LED. We have some ideas on
how to work around it with some additional components, but the
hardware design pipeline is quite deep and we're still going to see
designs like today in the near future.
> I guess that's fine for a VCM driver; just don't power the device on during
> probe. On the other hand, for an EEPROM that are common in sensors, that
> likely isn't so straightforward.
Yeah, we'll have to take care of those as well...
>
> I wonder what kind of a reception would be for a DT property to tell
> powering the device on during probe is undesirable.
My feeling is that it's more a user's decision, rather than a hardware
decision. Also, with DT, there is no problem with the PM subsystem
powering up things implicitly, as the drivers manage all the power
explicitly.
> Which reminds me ---
> time to respin the corresponding set for ACPI...
Thanks!
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-03-25 15:11 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 15:59 [V3, 0/2] media: i2c: add support for DW9768 VCM driver Dongchun Zhu
2020-02-28 15:59 ` [V3, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry Dongchun Zhu
2020-03-02 18:54 ` Rob Herring
2020-02-28 15:59 ` [V3, 2/2] media: i2c: Add DW9768 VCM driver Dongchun Zhu
2020-03-02 11:07 ` Andy Shevchenko
2020-03-05 12:05 ` Sakari Ailus
2020-03-10 10:10 ` Andy Shevchenko
2020-03-19 10:03 ` Dongchun Zhu
2020-03-19 11:36 ` Andy Shevchenko
2020-03-19 11:54 ` Dongchun Zhu
2020-03-18 10:20 ` Dongchun Zhu
2020-03-18 10:29 ` Sakari Ailus
2020-03-23 19:09 ` Tomasz Figa
2020-03-23 20:35 ` Sakari Ailus
2020-03-25 15:11 ` Tomasz Figa
2020-03-10 10:00 ` Dongchun Zhu
2020-03-10 10:05 ` Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).