[PATCH 0/7] media: i2c: dw9719: add DT compatible and DW9718S support

[PATCH 0/7] media: i2c: dw9719: add DT compatible and DW9718S support

[André Apitzsch via B4 Relay]

The DW9718S voice coil motor is found on various smartphones like
motorola-nora that are currently being worked on in the postmarketOS
community. Since the way it operates is very similar to DW9719, this
patch series adds support for it to the existing dw9719 driver. Because
that driver did not yet support DT, we also add DT bindings and the
dongwoon,dw9719 ofw compatible. With DW9718S, the driver was
tested fully, including runtime PM.

This is a follow-up of [1] and [2].

Changes compared to previous submission:
* Deprecate dongwoon,vcm-freq in favor of dongwoon,vcm-prescale
* Instead of per-device config struct use model ID to handle cases

[1] https://lore.kernel.org/linux-media/20250210082035.8670-1-val@packett.cool/
[2] https://lore.kernel.org/linux-media/20250209-dw9761dts-v3-0-14d3f00f0585@apitzsch.eu/

Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
André Apitzsch (2):
      dt-bindings: media: i2c: Add DW9718S, DW9719 and DW9761 VCM
      media: i2c: dw9719: Deprecate dongwoon,vcm-freq

Val Packett (5):
      media: i2c: dw9719: Add driver_data matching
      media: i2c: dw9719: Add DW9718S support
      media: i2c: dw9719: Update PM last busy time upon close
      media: i2c: dw9719: Add an of_match_table
      media: i2c: dw9719: Fix power on/off sequence

 .../bindings/media/i2c/dongwoon,dw9719.yaml        | 115 +++++++++++++++++++++
 drivers/media/i2c/dw9719.c                         | 111 +++++++++++++++++---
 2 files changed, 209 insertions(+), 17 deletions(-)
---
base-commit: 1357b2649c026b51353c84ddd32bc963e8999603
change-id: 20250709-dw9719-8a8822efc1b1

Best regards,
-- 
André Apitzsch <git@apitzsch.eu>

[PATCH 1/7] dt-bindings: media: i2c: Add DW9718S, DW9719 and DW9761 VCM

[André Apitzsch via B4 Relay]

From: André Apitzsch <git@apitzsch.eu>

Document Dongwoon DW9718S, DW9719 and DW9761 VCM devicetree bindings.

Signed-off-by: André Apitzsch <git@apitzsch.eu>

--

The possible values for sac-mode and vcm-prescale of DW9719 and DW9761
are missing because there is no documentation available.
---
 .../bindings/media/i2c/dongwoon,dw9719.yaml        | 115 +++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..80fd3fd42327fcafe3ff209d1cd6bbe17b8a211b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9719.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dongwoon Anatech DW9719 Voice Coil Motor (VCM) Controller
+
+maintainers:
+  - devicetree@vger.kernel.org
+
+description:
+  The Dongwoon DW9718S/9719/9761 is a single 10-bit digital-to-analog converter
+  with 100 mA output current sink capability, designed for linear control of
+  voice coil motors (VCM) in camera lenses. This chip provides a Smart Actuator
+  Control (SAC) mode intended for driving voice coil lenses in camera modules.
+
+properties:
+  compatible:
+    enum:
+      - dongwoon,dw9718s
+      - dongwoon,dw9719
+      - dongwoon,dw9761
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: VDD power supply
+
+  dongwoon,sac-mode:
+    description: |
+      Slew Rate Control mode to use: direct, LSC (Linear Slope Control) or
+      SAC1-SAC6 (Smart Actuator Control).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum:
+      - 0   # Direct mode
+      - 1   # LSC mode
+      - 2   # SAC1 mode (operation time# 0.32 x Tvib)
+      - 3   # SAC2 mode (operation time# 0.48 x Tvib)
+      - 4   # SAC3 mode (operation time# 0.72 x Tvib)
+      - 5   # SAC4 mode (operation time# 1.20 x Tvib)
+      - 6   # SAC5 mode (operation time# 1.64 x Tvib)
+      - 7   # SAC6 mode (operation time# 1.88 x Tvib)
+    default: 4
+
+  dongwoon,vcm-prescale:
+    description:
+      Indication of VCM switching frequency dividing rate select.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: dongwoon,dw9718s
+    then:
+      properties:
+        dongwoon,sac-mode:
+          default: 4
+        dongwoon,vcm-prescale:
+          description:
+            The final frequency is 10 MHz divided by (value + 2).
+          minimum: 0
+          maximum: 15
+          default: 0
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: dongwoon,dw9719
+    then:
+      properties:
+        dongwoon,sac-mode:
+          default: 4
+        dongwoon,vcm-prescale:
+          default: 96
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: dongwoon,dw9761
+    then:
+      properties:
+        dongwoon,sac-mode:
+          default: 6
+        dongwoon,vcm-prescale:
+          default: 62
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        actuator@c {
+            compatible = "dongwoon,dw9718s";
+            reg = <0x0c>;
+
+            vdd-supply = <&pm8937_l17>;
+
+            dongwoon,sac-mode = <4>;
+            dongwoon,vcm-prescale = <0>;
+        };
+    };

-- 
2.50.1

[PATCH 2/7] media: i2c: dw9719: Deprecate dongwoon,vcm-freq

[André Apitzsch via B4 Relay]

From: André Apitzsch <git@apitzsch.eu>

The name of property "dongwoon,vcm-freq" doesn't match its purpose.
Change the name of the property to "dongwoon,vcm-prescale" to better
describe its purpose and deprecate the old one.

Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 032fbcb981f20f4e93202415e62f67379897a048..5ed0042fce18acd9e6ce9f6cf6c6982e36fed275 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -82,6 +82,7 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 {
 	u64 val;
 	int ret;
+	int err;
 
 	ret = regulator_enable(dw9719->regulator);
 	if (ret)
@@ -123,7 +124,13 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 					 &dw9719->sac_mode);
 
 		/* Optional indication of VCM frequency */
-		device_property_read_u32(dw9719->dev, "dongwoon,vcm-freq",
+		err = device_property_read_u32(dw9719->dev, "dongwoon,vcm-freq",
+					       &dw9719->vcm_freq);
+		if (err == 0)
+			dev_warn(dw9719->dev, "dongwoon,vcm-freq property is deprecated, please use dongwoon,vcm-prescale\n");
+
+		/* Optional indication of VCM prescale */
+		device_property_read_u32(dw9719->dev, "dongwoon,vcm-prescale",
 					 &dw9719->vcm_freq);
 	}
 

-- 
2.50.1

[PATCH 3/7] media: i2c: dw9719: Add driver_data matching

[André Apitzsch via B4 Relay]

From: Val Packett <val@packett.cool>

In preparation for adding models with different register sets, start
assigning the model based on the i2c match data.

Signed-off-by: Val Packett <val@packett.cool>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 5ed0042fce18acd9e6ce9f6cf6c6982e36fed275..7ce66eaede5a2a1ba9c4c30c0efc5fafcca339a0 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -282,6 +282,8 @@ static int dw9719_probe(struct i2c_client *client)
 	if (!dw9719)
 		return -ENOMEM;
 
+	dw9719->model = (enum dw9719_model)i2c_get_match_data(client);
+
 	dw9719->regmap = devm_cci_regmap_init_i2c(client, 8);
 	if (IS_ERR(dw9719->regmap))
 		return PTR_ERR(dw9719->regmap);
@@ -361,8 +363,8 @@ static void dw9719_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id dw9719_id_table[] = {
-	{ "dw9719" },
-	{ "dw9761" },
+	{ "dw9719", .driver_data = DW9719 },
+	{ "dw9761", .driver_data = DW9761 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, dw9719_id_table);

-- 
2.50.1

[PATCH 4/7] media: i2c: dw9719: Add DW9718S support

[André Apitzsch via B4 Relay]

From: Val Packett <val@packett.cool>

The DW9718S is a similar part that uses a different register set but
follows the same method of operation otherwise. Add support for it
to the existing dw9719 driver.

Tested on the Moto E5 (motorola-nora) smartphone.

Signed-off-by: Val Packett <val@packett.cool>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 67 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 7ce66eaede5a2a1ba9c4c30c0efc5fafcca339a0..61758a9450aee20c9226e879a15eccfced9a3e96 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -23,6 +23,25 @@
 #define DW9719_CTRL_STEPS	16
 #define DW9719_CTRL_DELAY_US	1000
 
+#define DW9718S_PD			CCI_REG8(0)
+
+#define DW9718S_CONTROL			CCI_REG8(1)
+#define DW9718S_CONTROL_SW_LINEAR	BIT(0)
+#define DW9718S_CONTROL_SAC_SHIFT	1
+#define DW9718S_CONTROL_SAC_MASK	0x7
+#define DW9718S_CONTROL_OCP_DISABLE	BIT(4)
+#define DW9718S_CONTROL_UVLO_DISABLE	BIT(5)
+#define DW9718S_DEFAULT_SAC		4
+
+#define DW9718S_VCM_CURRENT		CCI_REG16(2)
+
+#define DW9718S_SW			CCI_REG8(4)
+#define DW9718S_SW_VCM_FREQ_MASK	0xF
+#define DW9718S_DEFAULT_VCM_FREQ	0
+
+#define DW9718S_SACT			CCI_REG8(5)
+#define DW9718S_SACT_PERIOD_8_8MS	0x19
+
 #define DW9719_INFO			CCI_REG8(0)
 #define DW9719_ID			0xF1
 #define DW9761_ID			0xF4
@@ -53,6 +72,7 @@
 #define to_dw9719_device(x) container_of(x, struct dw9719_device, sd)
 
 enum dw9719_model {
+	DW9718S,
 	DW9719,
 	DW9761,
 };
@@ -80,6 +100,7 @@ static int dw9719_power_down(struct dw9719_device *dw9719)
 
 static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 {
+	u32 reg_pwr;
 	u64 val;
 	int ret;
 	int err;
@@ -89,13 +110,21 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 		return ret;
 
 	/* Jiggle SCL pin to wake up device */
-	cci_write(dw9719->regmap, DW9719_CONTROL, DW9719_SHUTDOWN, &ret);
+	reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;
+	cci_write(dw9719->regmap, reg_pwr, DW9719_SHUTDOWN, &ret);
 	fsleep(100);
-	cci_write(dw9719->regmap, DW9719_CONTROL, DW9719_STANDBY, &ret);
+	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);
 	/* Need 100us to transit from SHUTDOWN to STANDBY */
 	fsleep(100);
 
 	if (detect) {
+		/* This model does not have an INFO register */
+		if (dw9719->model == DW9718S) {
+			dw9719->sac_mode = DW9718S_DEFAULT_SAC;
+			dw9719->vcm_freq = DW9718S_DEFAULT_VCM_FREQ;
+			goto props;
+		}
+
 		ret = cci_read(dw9719->regmap, DW9719_INFO, &val, NULL);
 		if (ret < 0)
 			return ret;
@@ -119,6 +148,7 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 			return -ENXIO;
 		}
 
+props:
 		/* Optional indication of SAC mode select */
 		device_property_read_u32(dw9719->dev, "dongwoon,sac-mode",
 					 &dw9719->sac_mode);
@@ -134,14 +164,30 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 					 &dw9719->vcm_freq);
 	}
 
-	cci_write(dw9719->regmap, DW9719_CONTROL, DW9719_ENABLE_RINGING, &ret);
-	cci_write(dw9719->regmap, DW9719_MODE, dw9719->mode_low_bits |
-			  (dw9719->sac_mode << DW9719_MODE_SAC_SHIFT), &ret);
-	cci_write(dw9719->regmap, DW9719_VCM_FREQ, dw9719->vcm_freq, &ret);
-
-	if (dw9719->model == DW9761)
+	switch (dw9719->model) {
+	case DW9718S:
+		/* Datasheet says [OCP/UVLO] should be disabled below 2.5V */
+		dw9719->sac_mode &= DW9718S_CONTROL_SAC_MASK;
+		cci_write(dw9719->regmap, DW9718S_CONTROL,
+			  DW9718S_CONTROL_SW_LINEAR |
+			  (dw9719->sac_mode << DW9718S_CONTROL_SAC_SHIFT) |
+			  DW9718S_CONTROL_OCP_DISABLE |
+			  DW9718S_CONTROL_UVLO_DISABLE, &ret);
+		cci_write(dw9719->regmap, DW9718S_SACT,
+			  DW9718S_SACT_PERIOD_8_8MS, &ret);
+		cci_write(dw9719->regmap, DW9718S_SW,
+			  dw9719->vcm_freq & DW9718S_SW_VCM_FREQ_MASK, &ret);
+		break;
+	case DW9761:
 		cci_write(dw9719->regmap, DW9761_VCM_PRELOAD,
 			  DW9761_DEFAULT_VCM_PRELOAD, &ret);
+		fallthrough;
+	case DW9719:
+		cci_write(dw9719->regmap, DW9719_CONTROL, DW9719_ENABLE_RINGING, &ret);
+		cci_write(dw9719->regmap, DW9719_MODE, dw9719->mode_low_bits |
+				  (dw9719->sac_mode << DW9719_MODE_SAC_SHIFT), &ret);
+		cci_write(dw9719->regmap, DW9719_VCM_FREQ, dw9719->vcm_freq, &ret);
+	}
 
 	if (ret)
 		dw9719_power_down(dw9719);
@@ -151,7 +197,9 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 
 static int dw9719_t_focus_abs(struct dw9719_device *dw9719, s32 value)
 {
-	return cci_write(dw9719->regmap, DW9719_VCM_CURRENT, value, NULL);
+	u32 reg = (dw9719->model == DW9718S) ? DW9718S_VCM_CURRENT
+					     : DW9719_VCM_CURRENT;
+	return cci_write(dw9719->regmap, reg, value, NULL);
 }
 
 static int dw9719_set_ctrl(struct v4l2_ctrl *ctrl)
@@ -363,6 +411,7 @@ static void dw9719_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id dw9719_id_table[] = {
+	{ "dw9718s", .driver_data = DW9718S },
 	{ "dw9719", .driver_data = DW9719 },
 	{ "dw9761", .driver_data = DW9761 },
 	{ }

-- 
2.50.1

[PATCH 5/7] media: i2c: dw9719: Update PM last busy time upon close

[André Apitzsch via B4 Relay]

From: Val Packett <val@packett.cool>

Update the close callback to match other similar drivers like dw9768.

Signed-off-by: Val Packett <val@packett.cool>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 61758a9450aee20c9226e879a15eccfced9a3e96..2952d8064899e4ac29f3b1af02692fe8043ccfac 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -284,7 +284,8 @@ static int dw9719_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 
 static int dw9719_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
 {
-	pm_runtime_put(sd->dev);
+	pm_runtime_mark_last_busy(sd->dev);
+	pm_runtime_put_autosuspend(sd->dev);
 
 	return 0;
 }

-- 
2.50.1

[PATCH 6/7] media: i2c: dw9719: Add an of_match_table

[André Apitzsch via B4 Relay]

From: Val Packett <val@packett.cool>

Allow the dw9719 driver to be attached via FDT.

Signed-off-by: Val Packett <val@packett.cool>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 2952d8064899e4ac29f3b1af02692fe8043ccfac..63c7fd4ab70a0e02518252b23b89c45df4ba273d 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -419,6 +419,14 @@ static const struct i2c_device_id dw9719_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, dw9719_id_table);
 
+static const struct of_device_id dw9719_of_table[] = {
+	{ .compatible = "dongwoon,dw9718s", .data = (const void *)DW9718S },
+	{ .compatible = "dongwoon,dw9719", .data = (const void *)DW9719 },
+	{ .compatible = "dongwoon,dw9761", .data = (const void *)DW9761 },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, dw9719_of_table);
+
 static DEFINE_RUNTIME_DEV_PM_OPS(dw9719_pm_ops, dw9719_suspend, dw9719_resume,
 				 NULL);
 
@@ -426,6 +434,7 @@ static struct i2c_driver dw9719_i2c_driver = {
 	.driver = {
 		.name = "dw9719",
 		.pm = pm_sleep_ptr(&dw9719_pm_ops),
+		.of_match_table = dw9719_of_table,
 	},
 	.probe = dw9719_probe,
 	.remove = dw9719_remove,

-- 
2.50.1

[PATCH 7/7] media: i2c: dw9719: Fix power on/off sequence

[André Apitzsch via B4 Relay]

From: Val Packett <val@packett.cool>

The "jiggle" code was not actually expecting failure, which it should
because that's what actually happens when the device wasn't already woken
up by the regulator power-on (i.e. in the case of a shared regulator).

Also, do actually enter the internal suspend mode on shutdown, to save
power in the case of a shared regulator.

Also, wait a bit longer (2x tOPR) on waking up, 1x is not enough at least
on the DW9718S as found on the motorola-nora smartphone.

Signed-off-by: Val Packett <val@packett.cool>
Signed-off-by: André Apitzsch <git@apitzsch.eu>
---
 drivers/media/i2c/dw9719.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
index 63c7fd4ab70a0e02518252b23b89c45df4ba273d..dd28a0223d6ac980084b1f661bd029ea6b0be503 100644
--- a/drivers/media/i2c/dw9719.c
+++ b/drivers/media/i2c/dw9719.c
@@ -95,12 +95,19 @@ struct dw9719_device {
 
 static int dw9719_power_down(struct dw9719_device *dw9719)
 {
+	u32 reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;
+
+	/*
+	 * Worth engaging the internal SHUTDOWN mode especially due to the
+	 * regulator being potentially shared with other devices.
+	 */
+	cci_write(dw9719->regmap, reg_pwr, DW9719_SHUTDOWN, NULL);
 	return regulator_disable(dw9719->regulator);
 }
 
 static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 {
-	u32 reg_pwr;
+	u32 reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;
 	u64 val;
 	int ret;
 	int err;
@@ -109,13 +116,15 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
 	if (ret)
 		return ret;
 
-	/* Jiggle SCL pin to wake up device */
-	reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;
-	cci_write(dw9719->regmap, reg_pwr, DW9719_SHUTDOWN, &ret);
-	fsleep(100);
+	/*
+	 * Need 100us to transition from SHUTDOWN to STANDBY.
+	 * Jiggle the SCL pin to wake up the device (even when the regulator
+	 * is shared) and wait double the time to be sure, then retry the write.
+	 */
+	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);
+	ret = 0; /* the jiggle is expected to fail, don't even log that as error */
+	fsleep(200);
 	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);
-	/* Need 100us to transit from SHUTDOWN to STANDBY */
-	fsleep(100);
 
 	if (detect) {
 		/* This model does not have an INFO register */

-- 
2.50.1

Re: [PATCH 3/7] media: i2c: dw9719: Add driver_data matching

[kernel test robot]

Hi André,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1357b2649c026b51353c84ddd32bc963e8999603]

url:    https://github.com/intel-lab-lkp/linux/commits/Andr-Apitzsch-via-B4-Relay/dt-bindings-media-i2c-Add-DW9718S-DW9719-and-DW9761-VCM/20250818-011316
base:   1357b2649c026b51353c84ddd32bc963e8999603
patch link:    https://lore.kernel.org/r/20250817-dw9719-v1-3-426f46c69a5a%40apitzsch.eu
patch subject: [PATCH 3/7] media: i2c: dw9719: Add driver_data matching
config: riscv-randconfig-002-20250818 (https://download.01.org/0day-ci/archive/20250818/202508180429.GKdrjNK9-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250818/202508180429.GKdrjNK9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508180429.GKdrjNK9-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/media/i2c/dw9719.c:285:18: warning: cast to smaller integer type 'enum dw9719_model' from 'const void *' [-Wvoid-pointer-to-enum-cast]
     285 |         dw9719->model = (enum dw9719_model)i2c_get_match_data(client);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +285 drivers/media/i2c/dw9719.c

   275	
   276	static int dw9719_probe(struct i2c_client *client)
   277	{
   278		struct dw9719_device *dw9719;
   279		int ret;
   280	
   281		dw9719 = devm_kzalloc(&client->dev, sizeof(*dw9719), GFP_KERNEL);
   282		if (!dw9719)
   283			return -ENOMEM;
   284	
 > 285		dw9719->model = (enum dw9719_model)i2c_get_match_data(client);
   286	
   287		dw9719->regmap = devm_cci_regmap_init_i2c(client, 8);
   288		if (IS_ERR(dw9719->regmap))
   289			return PTR_ERR(dw9719->regmap);
   290	
   291		dw9719->dev = &client->dev;
   292	
   293		dw9719->regulator = devm_regulator_get(&client->dev, "vdd");
   294		if (IS_ERR(dw9719->regulator))
   295			return dev_err_probe(&client->dev, PTR_ERR(dw9719->regulator),
   296					     "getting regulator\n");
   297	
   298		v4l2_i2c_subdev_init(&dw9719->sd, client, &dw9719_ops);
   299		dw9719->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
   300		dw9719->sd.internal_ops = &dw9719_internal_ops;
   301	
   302		ret = dw9719_init_controls(dw9719);
   303		if (ret)
   304			return ret;
   305	
   306		ret = media_entity_pads_init(&dw9719->sd.entity, 0, NULL);
   307		if (ret < 0)
   308			goto err_free_ctrl_handler;
   309	
   310		dw9719->sd.entity.function = MEDIA_ENT_F_LENS;
   311	
   312		/*
   313		 * We need the driver to work in the event that pm runtime is disable in
   314		 * the kernel, so power up and verify the chip now. In the event that
   315		 * runtime pm is disabled this will leave the chip on, so that the lens
   316		 * will work.
   317		 */
   318	
   319		ret = dw9719_power_up(dw9719, true);
   320		if (ret)
   321			goto err_cleanup_media;
   322	
   323		pm_runtime_set_active(&client->dev);
   324		pm_runtime_get_noresume(&client->dev);
   325		pm_runtime_enable(&client->dev);
   326	
   327		ret = v4l2_async_register_subdev(&dw9719->sd);
   328		if (ret < 0)
   329			goto err_pm_runtime;
   330	
   331		pm_runtime_set_autosuspend_delay(&client->dev, 1000);
   332		pm_runtime_use_autosuspend(&client->dev);
   333		pm_runtime_put_autosuspend(&client->dev);
   334	
   335		return ret;
   336	
   337	err_pm_runtime:
   338		pm_runtime_disable(&client->dev);
   339		pm_runtime_put_noidle(&client->dev);
   340		dw9719_power_down(dw9719);
   341	err_cleanup_media:
   342		media_entity_cleanup(&dw9719->sd.entity);
   343	err_free_ctrl_handler:
   344		v4l2_ctrl_handler_free(&dw9719->ctrls.handler);
   345	
   346		return ret;
   347	}
   348	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/7] media: i2c: dw9719: Update PM last busy time upon close

[Sakari Ailus]

Hi André,

Thanks for the patchset.

On Sun, Aug 17, 2025 at 07:09:24PM +0200, André Apitzsch via B4 Relay wrote:
> From: Val Packett <val@packett.cool>
> 
> Update the close callback to match other similar drivers like dw9768.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---
>  drivers/media/i2c/dw9719.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
> index 61758a9450aee20c9226e879a15eccfced9a3e96..2952d8064899e4ac29f3b1af02692fe8043ccfac 100644
> --- a/drivers/media/i2c/dw9719.c
> +++ b/drivers/media/i2c/dw9719.c
> @@ -284,7 +284,8 @@ static int dw9719_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>  
>  static int dw9719_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>  {
> -	pm_runtime_put(sd->dev);
> +	pm_runtime_mark_last_busy(sd->dev);

Please drop this line; the pm_runtime_mark_last_busy() is nowadays called
by pm_runtime_put_autosuspend() already.

> +	pm_runtime_put_autosuspend(sd->dev);
>  
>  	return 0;
>  }
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 3/7] media: i2c: dw9719: Add driver_data matching

[Sakari Ailus]

Hi André, Val,

On Sun, Aug 17, 2025 at 07:09:22PM +0200, André Apitzsch via B4 Relay wrote:
> From: Val Packett <val@packett.cool>
> 
> In preparation for adding models with different register sets, start
> assigning the model based on the i2c match data.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---
>  drivers/media/i2c/dw9719.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
> index 5ed0042fce18acd9e6ce9f6cf6c6982e36fed275..7ce66eaede5a2a1ba9c4c30c0efc5fafcca339a0 100644
> --- a/drivers/media/i2c/dw9719.c
> +++ b/drivers/media/i2c/dw9719.c
> @@ -282,6 +282,8 @@ static int dw9719_probe(struct i2c_client *client)
>  	if (!dw9719)
>  		return -ENOMEM;
>  
> +	dw9719->model = (enum dw9719_model)i2c_get_match_data(client);
> +
>  	dw9719->regmap = devm_cci_regmap_init_i2c(client, 8);
>  	if (IS_ERR(dw9719->regmap))
>  		return PTR_ERR(dw9719->regmap);
> @@ -361,8 +363,8 @@ static void dw9719_remove(struct i2c_client *client)
>  }
>  
>  static const struct i2c_device_id dw9719_id_table[] = {
> -	{ "dw9719" },
> -	{ "dw9761" },
> +	{ "dw9719", .driver_data = DW9719 },
> +	{ "dw9761", .driver_data = DW9761 },

Does something still depend on the I²C device ID table? Couldn't we just
remove it?

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, dw9719_id_table);
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 7/7] media: i2c: dw9719: Fix power on/off sequence

[Sakari Ailus]

Hi André,

On Sun, Aug 17, 2025 at 07:09:26PM +0200, André Apitzsch via B4 Relay wrote:
> From: Val Packett <val@packett.cool>
> 
> The "jiggle" code was not actually expecting failure, which it should
> because that's what actually happens when the device wasn't already woken
> up by the regulator power-on (i.e. in the case of a shared regulator).
> 
> Also, do actually enter the internal suspend mode on shutdown, to save
> power in the case of a shared regulator.
> 
> Also, wait a bit longer (2x tOPR) on waking up, 1x is not enough at least
> on the DW9718S as found on the motorola-nora smartphone.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> ---
>  drivers/media/i2c/dw9719.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/i2c/dw9719.c b/drivers/media/i2c/dw9719.c
> index 63c7fd4ab70a0e02518252b23b89c45df4ba273d..dd28a0223d6ac980084b1f661bd029ea6b0be503 100644
> --- a/drivers/media/i2c/dw9719.c
> +++ b/drivers/media/i2c/dw9719.c
> @@ -95,12 +95,19 @@ struct dw9719_device {
>  
>  static int dw9719_power_down(struct dw9719_device *dw9719)
>  {
> +	u32 reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;

Extra parentheses.

> +
> +	/*
> +	 * Worth engaging the internal SHUTDOWN mode especially due to the
> +	 * regulator being potentially shared with other devices.
> +	 */
> +	cci_write(dw9719->regmap, reg_pwr, DW9719_SHUTDOWN, NULL);

I'd still complain if this fails as we don't return the error.

>  	return regulator_disable(dw9719->regulator);
>  }
>  
>  static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
>  {
> -	u32 reg_pwr;
> +	u32 reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;

Extra parentheses.

>  	u64 val;
>  	int ret;
>  	int err;
> @@ -109,13 +116,15 @@ static int dw9719_power_up(struct dw9719_device *dw9719, bool detect)
>  	if (ret)
>  		return ret;
>  
> -	/* Jiggle SCL pin to wake up device */
> -	reg_pwr = (dw9719->model == DW9718S) ? DW9718S_PD : DW9719_CONTROL;
> -	cci_write(dw9719->regmap, reg_pwr, DW9719_SHUTDOWN, &ret);
> -	fsleep(100);
> +	/*
> +	 * Need 100us to transition from SHUTDOWN to STANDBY.
> +	 * Jiggle the SCL pin to wake up the device (even when the regulator
> +	 * is shared) and wait double the time to be sure, then retry the write.

Why double? Isn't the datasheet correct when it comes to the power-on
sequence?

> +	 */
> +	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);
> +	ret = 0; /* the jiggle is expected to fail, don't even log that as error */
> +	fsleep(200);
>  	cci_write(dw9719->regmap, reg_pwr, DW9719_STANDBY, &ret);

Just pass NULL instead of ret as we don't check the value and the ret
assignment above becomes redundant. Please spare the comment though.

> -	/* Need 100us to transit from SHUTDOWN to STANDBY */
> -	fsleep(100);
>  
>  	if (detect) {
>  		/* This model does not have an INFO register */
> 

-- 
Regards,

Sakari Ailus

Re: [PATCH 1/7] dt-bindings: media: i2c: Add DW9718S, DW9719 and DW9761 VCM

[Rob Herring]

On Sun, Aug 17, 2025 at 07:09:20PM +0200, André Apitzsch wrote:
> Document Dongwoon DW9718S, DW9719 and DW9761 VCM devicetree bindings.
> 
> Signed-off-by: André Apitzsch <git@apitzsch.eu>
> 
> --
> 
> The possible values for sac-mode and vcm-prescale of DW9719 and DW9761
> are missing because there is no documentation available.
> ---
>  .../bindings/media/i2c/dongwoon,dw9719.yaml        | 115 +++++++++++++++++++++
>  1 file changed, 115 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..80fd3fd42327fcafe3ff209d1cd6bbe17b8a211b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9719.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dongwoon Anatech DW9719 Voice Coil Motor (VCM) Controller
> +
> +maintainers:
> +  - devicetree@vger.kernel.org

No. Must be someone that has the h/w or cares about it. If there is no 
one, then we don't need the binding.

> +
> +description:
> +  The Dongwoon DW9718S/9719/9761 is a single 10-bit digital-to-analog converter
> +  with 100 mA output current sink capability, designed for linear control of
> +  voice coil motors (VCM) in camera lenses. This chip provides a Smart Actuator
> +  Control (SAC) mode intended for driving voice coil lenses in camera modules.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - dongwoon,dw9718s
> +      - dongwoon,dw9719
> +      - dongwoon,dw9761
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: VDD power supply
> +
> +  dongwoon,sac-mode:
> +    description: |
> +      Slew Rate Control mode to use: direct, LSC (Linear Slope Control) or
> +      SAC1-SAC6 (Smart Actuator Control).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum:
> +      - 0   # Direct mode
> +      - 1   # LSC mode
> +      - 2   # SAC1 mode (operation time# 0.32 x Tvib)
> +      - 3   # SAC2 mode (operation time# 0.48 x Tvib)
> +      - 4   # SAC3 mode (operation time# 0.72 x Tvib)
> +      - 5   # SAC4 mode (operation time# 1.20 x Tvib)
> +      - 6   # SAC5 mode (operation time# 1.64 x Tvib)
> +      - 7   # SAC6 mode (operation time# 1.88 x Tvib)
> +    default: 4
> +
> +  dongwoon,vcm-prescale:
> +    description:
> +      Indication of VCM switching frequency dividing rate select.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: dongwoon,dw9718s
> +    then:
> +      properties:
> +        dongwoon,sac-mode:
> +          default: 4
> +        dongwoon,vcm-prescale:
> +          description:
> +            The final frequency is 10 MHz divided by (value + 2).
> +          minimum: 0

That's already the minimum being unsigned.

> +          maximum: 15
> +          default: 0
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: dongwoon,dw9719
> +    then:
> +      properties:
> +        dongwoon,sac-mode:
> +          default: 4
> +        dongwoon,vcm-prescale:
> +          default: 96
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: dongwoon,dw9761
> +    then:
> +      properties:
> +        dongwoon,sac-mode:
> +          default: 6

At the top-level you already said the default is 4. The if/then is an 
AND operation. 'default' is just an annotation and has no effect on 
validation. I would just drop it from the if/then altogether. It's not 
worth the complexity.

> +        dongwoon,vcm-prescale:
> +          default: 62
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        actuator@c {
> +            compatible = "dongwoon,dw9718s";
> +            reg = <0x0c>;
> +
> +            vdd-supply = <&pm8937_l17>;
> +
> +            dongwoon,sac-mode = <4>;
> +            dongwoon,vcm-prescale = <0>;
> +        };
> +    };
> 
> -- 
> 2.50.1
> 

Re: [PATCH 1/7] dt-bindings: media: i2c: Add DW9718S, DW9719 and DW9761 VCM

[André Apitzsch]

Hi Daniel,

Am Mittwoch, dem 20.08.2025 um 16:56 -0500 schrieb Rob Herring:
> On Sun, Aug 17, 2025 at 07:09:20PM +0200, André Apitzsch wrote:
> > Document Dongwoon DW9718S, DW9719 and DW9761 VCM devicetree
> > bindings.
> > 
> > Signed-off-by: André Apitzsch <git@apitzsch.eu>
> > 
> > --
> > 
> > The possible values for sac-mode and vcm-prescale of DW9719 and
> > DW9761
> > are missing because there is no documentation available.
> > ---
> >  .../bindings/media/i2c/dongwoon,dw9719.yaml        | 115
> > +++++++++++++++++++++
> >  1 file changed, 115 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
> > b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..80fd3fd42327fcafe3ff209d1
> > cd6bbe17b8a211b
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9719.yaml
> > @@ -0,0 +1,115 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9719.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dongwoon Anatech DW9719 Voice Coil Motor (VCM) Controller
> > +
> > +maintainers:
> > +  - devicetree@vger.kernel.org
> 
> No. Must be someone that has the h/w or cares about it. If there is
> no one, then we don't need the binding.

as you are listed as maintainer for DW9719 in MAINTAINERS, is it okay
if I add your name and e-mail here?

Best regards,
André

> 
> > +
> > +description:
> > +  The Dongwoon DW9718S/9719/9761 is a single 10-bit digital-to-
> > analog converter
> > +  with 100 mA output current sink capability, designed for linear
> > control of
> > +  voice coil motors (VCM) in camera lenses. This chip provides a
> > Smart Actuator
> > +  Control (SAC) mode intended for driving voice coil lenses in
> > camera modules.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - dongwoon,dw9718s
> > +      - dongwoon,dw9719
> > +      - dongwoon,dw9761
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vdd-supply:
> > +    description: VDD power supply
> > +
> > +  dongwoon,sac-mode:
> > +    description: |
> > +      Slew Rate Control mode to use: direct, LSC (Linear Slope
> > Control) or
> > +      SAC1-SAC6 (Smart Actuator Control).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum:
> > +      - 0   # Direct mode
> > +      - 1   # LSC mode
> > +      - 2   # SAC1 mode (operation time# 0.32 x Tvib)
> > +      - 3   # SAC2 mode (operation time# 0.48 x Tvib)
> > +      - 4   # SAC3 mode (operation time# 0.72 x Tvib)
> > +      - 5   # SAC4 mode (operation time# 1.20 x Tvib)
> > +      - 6   # SAC5 mode (operation time# 1.64 x Tvib)
> > +      - 7   # SAC6 mode (operation time# 1.88 x Tvib)
> > +    default: 4
> > +
> > +  dongwoon,vcm-prescale:
> > +    description:
> > +      Indication of VCM switching frequency dividing rate select.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - vdd-supply
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: dongwoon,dw9718s
> > +    then:
> > +      properties:
> > +        dongwoon,sac-mode:
> > +          default: 4
> > +        dongwoon,vcm-prescale:
> > +          description:
> > +            The final frequency is 10 MHz divided by (value + 2).
> > +          minimum: 0
> 
> That's already the minimum being unsigned.
> 
> > +          maximum: 15
> > +          default: 0
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: dongwoon,dw9719
> > +    then:
> > +      properties:
> > +        dongwoon,sac-mode:
> > +          default: 4
> > +        dongwoon,vcm-prescale:
> > +          default: 96
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: dongwoon,dw9761
> > +    then:
> > +      properties:
> > +        dongwoon,sac-mode:
> > +          default: 6
> 
> At the top-level you already said the default is 4. The if/then is an
> AND operation. 'default' is just an annotation and has no effect on 
> validation. I would just drop it from the if/then altogether. It's
> not worth the complexity.
> 
> > +        dongwoon,vcm-prescale:
> > +          default: 62
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        actuator@c {
> > +            compatible = "dongwoon,dw9718s";
> > +            reg = <0x0c>;
> > +
> > +            vdd-supply = <&pm8937_l17>;
> > +
> > +            dongwoon,sac-mode = <4>;
> > +            dongwoon,vcm-prescale = <0>;
> > +        };
> > +    };
> > 
> > -- 
> > 2.50.1
> >