The driver assumes the sensor is already powered when probe() reads its chip
ID. That holds where the rails and clock are ACPI power resources, but not
where an INT3472 companion device registers them as regulators, a clock and a
reset GPIO for the sensor driver to consume, which this driver does not do.

Request the three supplies and the reset GPIO, and sequence them along with
the clock the driver already looks up, in runtime PM callbacks.

Signed-off-by: Sergey Lebedev <lsa.uz@pm.me>
---
Changes in v3, from Sakari Ailus's review of v2:
- wrapped the one line over 80 columns
- dropped an unrelated hunk in ov13858_id_table. It was not deliberate: v2 was
  regenerated against mainline from a tree based on a distribution kernel, and
  that difference came along with it.
Link to v2: https://patch.msgid.link/20260831124352.86935-1-lsa.uz@pm.me

Changes in v2, from the review of v1:
- commit message cut down; the detail below
- dropped the comment on the supply names; it is a KAPI with int3472
- ARRAY_SIZE() directly instead of a local macro
- fsleep() instead of usleep_range()
- one function each for power on and off, used as the PM callbacks directly,
  instead of a pair of wrappers
- removed the now-useless dev_err() in the probe error path
- i declared inside its loop
Link to v1: https://patch.msgid.link/20260831100404.40463-1-lsa.uz@pm.me

Without the patch the first I2C transaction fails:

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

The power sequence follows ov02c10: supplies, then clock, then release reset,
and the reverse on the way down.

Measured on a Microsoft Surface Pro 11 for Business (Intel Lunar Lake, IPU7),
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 again after
this rework, with the sensor streaming each time.

This needs POWER1 GPIO support in int3472 to be useful on that machine: the
dvdd rail is described there as an INT3472 GPIO of type 0x08, and without that
patch the rail is never registered.
Link: https://patch.msgid.link/20260829-sp7plus-int3472-v3-1-454b50485ce2@berg.pm

The same sensor on the Surface Pro 10 was made to work downstream by forcing
the regulators on for the driver's lifetime, which the people who did it
called too broad for upstream. Runtime PM keeps them off while the sensor is
idle instead.
Link: https://github.com/linux-surface/linux-surface/issues/2153

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.

A working camera also needs an ipu-bridge entry for OVTID858, separately.
---
--- 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>
@@ -1028,9 +1031,17 @@
 	}
 };
 
+static const char * const ov13858_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
 struct ov13858 {
 	struct device *dev;
 	struct clk *clk;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(ov13858_supply_names)];
+	struct gpio_desc *reset_gpio;
 
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -1653,8 +1664,55 @@
 {
 	v4l2_ctrl_handler_free(ov13858->sd.ctrl_handler);
 	mutex_destroy(&ov13858->mutex);
+}
+
+static int ov13858_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov13858 *ov13858 = to_ov13858(sd);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ov13858_supply_names),
+				    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(ARRAY_SIZE(ov13858_supply_names), ov13858->supplies);
+		return ret;
+	}
+
+	if (ov13858->reset_gpio) {
+		/* Hold reset for at least 1 ms on a back to back off-on */
+		fsleep(1000);
+		gpiod_set_value_cansleep(ov13858->reset_gpio, 0);
+	}
+
+	/* t4: 8192 XVCLK cycles after reset is released, before the first I2C */
+	fsleep(5000);
+
+	return 0;
 }
 
+static int ov13858_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov13858 *ov13858 = to_ov13858(sd);
+
+	gpiod_set_value_cansleep(ov13858->reset_gpio, 1);
+	regulator_bulk_disable(ARRAY_SIZE(ov13858_supply_names), ov13858->supplies);
+	clk_disable_unprepare(ov13858->clk);
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(ov13858_pm_ops, ov13858_power_off,
+				 ov13858_power_on, NULL);
+
 static int ov13858_probe(struct i2c_client *client)
 {
 	struct ov13858 *ov13858;
@@ -1678,14 +1736,34 @@
 				     "external clock %lu is not supported\n",
 				     freq);
 
+	for (unsigned int i = 0; i < ARRAY_SIZE(ov13858_supply_names); i++)
+		ov13858->supplies[i].supply = ov13858_supply_names[i];
+
+	ret = devm_regulator_bulk_get(ov13858->dev, ARRAY_SIZE(ov13858_supply_names),
+				      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->dev);
+	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 */
@@ -1693,7 +1771,7 @@
 
 	ret = ov13858_init_controls(ov13858);
 	if (ret)
-		return ret;
+		goto error_power_off;
 
 	/* Initialize subdev */
 	ov13858->sd.internal_ops = &ov13858_internal_ops;
@@ -1729,8 +1807,10 @@
 
 error_handler_free:
 	ov13858_free_controls(ov13858);
-	dev_err(ov13858->dev, "%s failed:%d\n", __func__, ret);
 
+error_power_off:
+	ov13858_power_off(ov13858->dev);
+
 	return ret;
 }
 
@@ -1744,11 +1824,14 @@
 	ov13858_free_controls(ov13858);
 
 	pm_runtime_disable(ov13858->dev);
+	if (!pm_runtime_status_suspended(ov13858->dev))
+		ov13858_power_off(ov13858->dev);
+	pm_runtime_set_suspended(ov13858->dev);
 }
 
 static const struct i2c_device_id ov13858_id_table[] = {
 	{ .name = "ov13858" },
 	{ }
 };
 
 MODULE_DEVICE_TABLE(i2c, ov13858_id_table);
@@ -1766,6 +1849,7 @@
 	.driver = {
 		.name = "ov13858",
 		.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
+		.pm = pm_ptr(&ov13858_pm_ops),
 	},
 	.probe = ov13858_probe,
 	.remove = ov13858_remove,

A ping, two weeks on:

  https://lore.kernel.org/linux-media/20260831130312.26296-1-lsa.uz@pm.me/

It still applies to media/next at aedd77ea81, checked today, and builds
there clean under W=1. Nothing in the patch itself has changed.

One line below the scissors has gone out of date, in the patch's favour. It
says this needs POWER1 GPIO support in int3472 to be useful on this machine.
That landed: discrete.c now maps INT3472_GPIO_TYPE_POWER1 to "dvdd", so the
rail is a registered regulator here rather than a dummy, and the dependency
the patch declared is no longer open.

With the patch applied the sensor identifies, the driver binds, and
v4l2-compliance reports 46 of 46, 0 failed. That run carried this patch, the
gain cap and the flip controls together, since all three touch this driver.

Sergey

New evidence for this patch, from media/next rather than from a distro kernel.

I built and booted a kernel from media/next f9536a8065 on the Surface Pro 11
today - for an unrelated series - and the back camera does not come up on it:

  ov13858 i2c-OVTID858:00: failed to find sensor: -5
  ov13858 i2c-OVTID858:00: probe with driver ov13858 failed with error -5

That is the mechanism Hans named on 2026-08-31 - "the ov13858 problem is not a
problem with the INT3472 driver, but rather with the ov13858 driver" - now
visible on current upstream. Reading f9536a8065: ov13858.c contains no
regulator, no supply and no gpiod reference at all, and while probe does
acquire a clock through devm_v4l2_sensor_clk_get() it never enables it. So
nothing asks INT3472 to power the sensor, and the chip-ID read talks to a part
that is still off.

  https://lore.kernel.org/all/df2f3cb1-1bab-432f-a359-59e6bec9580a@kernel.org/

With this patch applied the same machine enumerates the camera and streams from
it; that has been true since 2026-08-31 and is what the original posting
reported. What is new is only that the failure is now demonstrated on the tree
this patch is aimed at, rather than argued from a 7.0.0 kernel.

Both ov13858 patches still apply to f9536a8065 unchanged - checked, not assumed
- so there is nothing to resend and I am not asking for anything here. Recording
it because the next person to read this thread should not have to take the
"camera still dead" claim on trust.

Sergey