[PATCH 0/3] media: ak7375: Add AK7377 support

[PATCH 0/3] media: ak7375: Add AK7377 support

[Jan-Michael Brummer]

The AK7377 drives the focus actuator of the main camera on the Fairphone
5, and is register compatible with the AK7375 already supported by this
driver: a 12 bit position in registers 0x00/0x01 shifted left by four,
and standby control in 0x02.  Patches 1 and 2 add the binding and the
compatible.

Patch 3 is a fix that is independent of the above and applies to every
user of this driver.  probe() calls pm_runtime_set_active() while the
regulators are still off, so the first runtime transition is a suspend
against an unpowered chip; it fails its I2C writes and then disables
regulators that were never enabled.  On the Fairphone 5 that showed up
as an -ENXIO from the first resume, after which focus control stayed
dead for the whole session even though the actuator answered fine on the
bus, and as a WARN from regulator_bulk_free() on unbind.

I can send patch 3 on its own if that is easier to route - it does not
depend on the other two.

Testing: the AK7377 on a Fairphone 5, driving the lens across its range
and checking that the image comes into focus, with an out-of-tree IMX800
sensor driver.  I have no AK7345 or AK7375 hardware, so patch 3 is
untested on those; the change moves the first runtime resume from probe
to the first focus request, and confirmation that this does not regress
them would be welcome.

Jan-Michael Brummer (3):
  media: dt-bindings: ak7375: Add AK7377
  media: i2c: ak7375: Add support for the AK7377
  media: i2c: ak7375: Do not mark the VCM active before powering it

 .../bindings/media/i2c/asahi-kasei,ak7375.yaml |  1 +
 drivers/media/i2c/ak7375.c                     | 45 ++++++++++++++++---
 2 files changed, 41 insertions(+), 5 deletions(-)

-- 
2.51.0

[PATCH] media: dt-bindings: ak7375: Add AK7377

[Jan-Michael Brummer]

The AK7377 is register compatible with the AK7375 and is driven by the
same driver.

Signed-off-by: Jan-Michael Brummer <jan.brummer@tabos.org>
---
 .../devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml        | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
index fe312cc..c040757 100644
--- a/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/asahi-kasei,ak7375.yaml
@@ -18,6 +18,7 @@ properties:
     enum:
       - asahi-kasei,ak7345
       - asahi-kasei,ak7375
+      - asahi-kasei,ak7377
 
   reg:
     maxItems: 1

[PATCH 1/2] media: i2c: ak7375: Add support for the AK7377

[Jan-Michael Brummer]

The AK7377 drives the focus actuator of the main camera on the Fairphone
5.  It uses the same register layout as the AK7375: a 12 bit position in
registers 0x00/0x01 shifted left by four, and standby control in 0x02.
Confirmed on hardware by driving the lens across its range.

Signed-off-by: Jan-Michael Brummer <jan.brummer@tabos.org>
---
 drivers/media/i2c/ak7375.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
index 9a2432c..4f311c4 100644
--- a/drivers/media/i2c/ak7375.c
+++ b/drivers/media/i2c/ak7375.c
@@ -65,6 +65,25 @@ static const struct ak73xx_chipdef ak7375_cdef = {
 	.power_delay_us	= 10000,
 };
 
+/*
+ * The AK7377 is register compatible with the AK7375: 12 bit position in
+ * registers 0x00/0x01, standby via 0x02.  Verified on the Fairphone 5,
+ * where it drives the focus of the IMX800 main camera.
+ */
+static const struct ak73xx_chipdef ak7377_cdef = {
+	.reg_position	= 0x0,
+	.reg_cont	= 0x2,
+	.shift_pos	= 4,	/* 12 bits position values, need to << 4 */
+	.mode_active	= 0x0,
+	.mode_standby	= 0x40,
+	.has_standby	= true,
+	.focus_pos_max	= 4095,
+	.focus_steps	= 1,
+	.ctrl_steps	= 64,
+	.ctrl_delay_us	= 1000,
+	.power_delay_us	= 10000,
+};
+
 static const char * const ak7375_supply_names[] = {
 	"vdd",
 	"vio",
@@ -331,6 +350,7 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
 static const struct of_device_id ak7375_of_table[] = {
 	{ .compatible = "asahi-kasei,ak7345", .data = &ak7345_cdef, },
 	{ .compatible = "asahi-kasei,ak7375", .data = &ak7375_cdef, },
+	{ .compatible = "asahi-kasei,ak7377", .data = &ak7377_cdef, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, ak7375_of_table);
@@ -353,5 +373,5 @@ module_i2c_driver(ak7375_i2c_driver);
 
 MODULE_AUTHOR("Tianshu Qiu <tian.shu.qiu@intel.com>");
 MODULE_AUTHOR("Bingbu Cao <bingbu.cao@intel.com>");
-MODULE_DESCRIPTION("AK7375 VCM driver");
+MODULE_DESCRIPTION("AK7375/AK7377 VCM driver");
 MODULE_LICENSE("GPL v2");

[PATCH 2/2] media: i2c: ak7375: Do not mark the VCM active before powering it

[Jan-Michael Brummer]

probe() calls pm_runtime_set_active() while the regulators are still
off, so runtime PM believes a powered device is present.  The following
pm_runtime_idle() runs the suspend path against an unpowered chip, which
fails its I2C writes and then calls regulator_bulk_disable() on
regulators that were never enabled - later tripping the enable/disable
balance check in regulator_bulk_free() on unbind.

Mark the device suspended instead and drop the idle call, so the first
resume happens when something actually uses the VCM.  While at it,
disable the regulators again when resume fails, which leaked an enable
count for the same reason.

Fixes: 90ee26fb2f50 ("media: ak7375: Add ak7375 lens voice coil driver")
Signed-off-by: Jan-Michael Brummer <jan.brummer@tabos.org>
---
 drivers/media/i2c/ak7375.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c
index 4f311c4..2763bbe 100644
--- a/drivers/media/i2c/ak7375.c
+++ b/drivers/media/i2c/ak7375.c
@@ -235,9 +235,12 @@ static int ak7375_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto err_cleanup;
 
-	pm_runtime_set_active(&client->dev);
+	/*
+	 * The regulators are off at this point, so the device is suspended,
+	 * not active.
+	 */
+	pm_runtime_set_suspended(&client->dev);
 	pm_runtime_enable(&client->dev);
-	pm_runtime_idle(&client->dev);
 
 	return 0;
 
@@ -328,6 +331,8 @@ static int __maybe_unused ak7375_vcm_resume(struct device *dev)
 			       cdef->mode_active, 1);
 	if (ret) {
 		dev_err(dev, "%s I2C failure: %d\n", __func__, ret);
+		regulator_bulk_disable(ARRAY_SIZE(ak7375_supply_names),
+				       ak7375_dev->supplies);
 		return ret;
 	}