[PATCH] media: i2c: ov13858: add regulator, clock and reset GPIO handling

[Sergey Lebedev]

ov13858_probe() reads the chip ID over I2C on the assumption stated in its
own comment:

	/*
	 * Device is already turned on by i2c-core with ACPI domain PM.
	 * Enable runtime PM and turn off the device.
	 */

That holds where the sensor's rails and clock are ACPI power resources. It
does not hold where an INT3472 companion device describes them, because
INT3472 registers them as regulators, a clock and a reset GPIO for the
sensor driver to consume - and this driver consumes none of them. They stay
off, the sensor stays in reset, and the first I2C transaction fails:

  ov13858 i2c-OVTID858:00: failed to find sensor: -5

Add the three standard supplies, the reset GPIO and the clock the driver
already looks up, sequenced in runtime PM callbacks the driver did not have.
The shape follows ov02c10, which handles the same situation: supplies, then
clock, then release reset, and the reverse on the way down.

This depends on POWER1 GPIO support in int3472. On the Surface Pro 11 the
dvdd rail is described as an INT3472 GPIO of type 0x08, so without that patch
the rail is never registered and there is nothing here to consume:

Link: https://patch.msgid.link/20260829-sp7plus-int3472-v3-1-454b50485ce2@xxxxxxx

The same sensor on the Surface Pro 10 was made to work downstream by adding
reset handling to ov13858_probe() and forcing the regulators on for the
lifetime of the driver; the people who did it called that second half too
broad for upstream, and it is. Driving the rails from runtime PM instead
keeps them off while the sensor is idle, which is what the companion device
registered them for:

Link: https://github.com/linux-surface/linux-surface/issues/2153

Measured on a Microsoft Surface Pro 11 for Business (Intel Lunar Lake,
IPU7), kernel 7.0.0-30, with the parts isolated one at a time:

  supplies enabled, clock enabled   sensor identifies, driver binds
  supplies enabled, clock left off  -EIO
  supplies left off, clock enabled  -EIO

so both are needed; a longer settling delay alone is not enough. Verified
across five module unload/load cycles with no probe failure, and the sensor
streams after each one.

dovdd is not described on this machine and resolves to a dummy regulator. It
is listed because it is one of the three supplies these sensors normally
take, and machines that do describe it should get it.

With this patch the sensor probes on every boot and runtime PM powers it
down when idle. Producing a working camera also needs an ipu-bridge entry
for OVTID858, which is a separate patch.

Signed-off-by: Sergey Lebedev <lsa.uz@xxxxx>
---
Hans suggested two patches, one for the regulators and one for the reset
GPIO. They are sent as one here because neither half leaves the sensor in a
working state on its own - with supplies but no reset handling the part stays
held in reset, and vice versa. Happy to split it if you would still prefer
that.

An earlier version of this patch released reset before starting the clock. It
worked on this hardware, but only because the 8192-cycle wait that follows
covered for it, and it is the wrong order. Mentioning it in case anyone is
carrying that version downstream.
---
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -3,9 +3,12 @@
 
 #include <linux/acpi.h>
 #include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-event.h>
@@ -1037,9 +1040,23 @@
 	}
 };
 
+/*
+ * The sensor's rails are described by an INT3472 companion device on ACPI
+ * platforms; the names match the con_ids that driver registers.
+ */
+static const char * const ov13858_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
+#define OV13858_NUM_SUPPLIES ARRAY_SIZE(ov13858_supply_names)
+
 struct ov13858 {
 	struct device *dev;
 	struct clk *clk;
+	struct regulator_bulk_data supplies[OV13858_NUM_SUPPLIES];
+	struct gpio_desc *reset_gpio;
 
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -1699,10 +1716,66 @@
 	mutex_destroy(&ov13858->mutex);
 }
 
+static int ov13858_power_on(struct ov13858 *ov13858)
+{
+	int ret;
+
+	ret = regulator_bulk_enable(OV13858_NUM_SUPPLIES, ov13858->supplies);
+	if (ret) {
+		dev_err(ov13858->dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ov13858->clk);
+	if (ret) {
+		dev_err(ov13858->dev, "failed to enable clock: %d\n", ret);
+		regulator_bulk_disable(OV13858_NUM_SUPPLIES, ov13858->supplies);
+		return ret;
+	}
+
+	if (ov13858->reset_gpio) {
+		/* Hold reset for at least 1 ms on a back to back off-on */
+		usleep_range(1000, 1500);
+		gpiod_set_value_cansleep(ov13858->reset_gpio, 0);
+	}
+
+	/* t4: 8192 XVCLK cycles after reset is released, before the first I2C */
+	usleep_range(5000, 6000);
+
+	return 0;
+}
+
+static void ov13858_power_off(struct ov13858 *ov13858)
+{
+	gpiod_set_value_cansleep(ov13858->reset_gpio, 1);
+	regulator_bulk_disable(OV13858_NUM_SUPPLIES, ov13858->supplies);
+	clk_disable_unprepare(ov13858->clk);
+}
+
+static int ov13858_runtime_resume(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+
+	return ov13858_power_on(to_ov13858(sd));
+}
+
+static int ov13858_runtime_suspend(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+
+	ov13858_power_off(to_ov13858(sd));
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ov13858_pm_ops, ov13858_runtime_suspend,
+				 ov13858_runtime_resume, NULL);
+
 static int ov13858_probe(struct i2c_client *client)
 {
 	struct ov13858 *ov13858;
 	unsigned long freq;
+	unsigned int i;
 	int ret;
 
 	ov13858 = devm_kzalloc(&client->dev, sizeof(*ov13858), GFP_KERNEL);
@@ -1722,14 +1795,34 @@
 				     "external clock %lu is not supported\n",
 				     freq);
 
+	for (i = 0; i < OV13858_NUM_SUPPLIES; i++)
+		ov13858->supplies[i].supply = ov13858_supply_names[i];
+
+	ret = devm_regulator_bulk_get(ov13858->dev, OV13858_NUM_SUPPLIES,
+				      ov13858->supplies);
+	if (ret)
+		return dev_err_probe(ov13858->dev, ret,
+				     "failed to get regulators\n");
+
+	ov13858->reset_gpio = devm_gpiod_get_optional(ov13858->dev, "reset",
+						      GPIOD_OUT_HIGH);
+	if (IS_ERR(ov13858->reset_gpio))
+		return dev_err_probe(ov13858->dev,
+				     PTR_ERR(ov13858->reset_gpio),
+				     "failed to get reset GPIO\n");
+
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&ov13858->sd, client, &ov13858_subdev_ops);
 
+	ret = ov13858_power_on(ov13858);
+	if (ret)
+		return ret;
+
 	/* Check module identity */
 	ret = ov13858_identify_module(ov13858);
 	if (ret) {
 		dev_err(ov13858->dev, "failed to find sensor: %d\n", ret);
-		return ret;
+		goto error_power_off;
 	}
 
 	/* Set default mode to max resolution */
@@ -1737,7 +1830,7 @@
 
 	ret = ov13858_init_controls(ov13858);
 	if (ret)
-		return ret;
+		goto error_power_off;
 
 	/* Initialize subdev */
 	ov13858->sd.internal_ops = &ov13858_internal_ops;
@@ -1773,6 +1866,9 @@
 
 error_handler_free:
 	ov13858_free_controls(ov13858);
+
+error_power_off:
+	ov13858_power_off(ov13858);
 	dev_err(ov13858->dev, "%s failed:%d\n", __func__, ret);
 
 	return ret;
@@ -1788,6 +1884,9 @@
 	ov13858_free_controls(ov13858);
 
 	pm_runtime_disable(ov13858->dev);
+	if (!pm_runtime_status_suspended(ov13858->dev))
+		ov13858_power_off(ov13858);
+	pm_runtime_set_suspended(ov13858->dev);
 }
 
 static const struct i2c_device_id ov13858_id_table[] = {
@@ -1810,6 +1909,7 @@
 	.driver = {
 		.name = "ov13858",
 		.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
+		.pm = pm_ptr(&ov13858_pm_ops),
 	},
 	.probe = ov13858_probe,
 	.remove = ov13858_remove,