From nobody Sat Sep 26 19:34:29 2026 Received: from mail-10629.protonmail.ch (mail-10629.protonmail.ch [79.135.106.29]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0664D3E8C74 for ; Mon, 31 Aug 2026 10:04:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=79.135.106.29 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788170668; cv=none; b=U+PsmgzMHw+t22mXkrvWqKE10swbV4kzfEQYvZvbO01QdsLt6vSzJntUuJ3k5sJUYRCoSekhh+wogZDidPt3xGG2RdPbmAs9oB5IUkQZB2hhV6p4DDbSDTV5KdMUYEi7GLOgbcjrPd3Z+YHd3vVp/fs/uCSjv1T7uUVIlTwE/X8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788170668; c=relaxed/simple; bh=sGemyPXXBm2FHq+9BCXB0xeIxmOj9PnmpbOQSA1awWE=; h=Date:To:From:Cc:Subject:Message-ID:MIME-Version:Content-Type; b=E88WQMyoiJKQdda6+5rW3hscLCta6qaga6NzI4KEWMw4tissQDWRfgop1Zia+zXj6iepD5B57gysQQkNerO2WmHy2wg9sUMDKkyhsr8sXj0KwKpBy9kt3NTEspXO3IT9XQjDfRzo8pCtBrkSbOLoYHka7oXRb20NZwwO3p6efJs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=pm.me; spf=pass smtp.mailfrom=pm.me; dkim=pass (2048-bit key) header.d=pm.me header.i=@pm.me header.b=QJECRUZG; arc=none smtp.client-ip=79.135.106.29 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=pm.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pm.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pm.me header.i=@pm.me header.b="QJECRUZG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail3; t=1788170657; x=1788429857; bh=ppDjzDf9Ff93FbB9scTpr5J69xukhr3h6UB/qk0lXRc=; h=Date:To:From:Cc:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=QJECRUZG004Ifw1rd2ADRMhNiTXpinlQyBttDIq7neCoCi09Q5XE8rqgN1C2grrda QTSDhMF30DH335jEBi4LlCx8mtuUjMJnGgAyPoKj+KDzz6ro/334YKA0BUOfikKBr+ s7Z34owpk+JqzQzzjlaFb0ejC+3sBjmgWaaiIBrL/7xQsI+GyopY2Pq/NBgN1czNkm Pe/CR59tzVXf2L2b/zpwkQZkLW+qDOjmBU70T4DJnZROixn1L+WgzMd7AcnmNgDXpS 3SjYwSvT4x0VRUhkwOJFtCGgVQXQwSdylCvpcL4MMiYF/prQtJLfjEzNjTusGJ8c9/ g6fUDrThvH5Ow== Date: Mon, 31 Aug 2026 10:04:13 +0000 To: Sakari Ailus , Mauro Carvalho Chehab , Hans de Goede From: Sergey Lebedev Cc: Daniel Scally , Jakob Berg Jespersen , linux-media@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] media: i2c: ov13858: add regulator, clock and reset GPIO handling Message-ID: <20260831100404.40463-1-lsa.uz@pm.me> Feedback-ID: 113843758:user:proton X-Pm-Message-ID: 383b98c99eda73552f4ed70e7e7d4fc83c2e0e1d Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 pat= ch the rail is never registered and there is nothing here to consume: Link: https://patch.msgid.link/20260829-sp7plus-int3472-v3-1-454b50485ce2@b= erg.pm 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 --- 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 sta= ys 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 @@ =20 #include #include +#include +#include #include #include #include +#include #include #include #include @@ -1037,9 +1040,23 @@ } }; =20 +/* + * 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[] =3D { + "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; =20 struct v4l2_subdev sd; struct media_pad pad; @@ -1699,10 +1716,66 @@ mutex_destroy(&ov13858->mutex); } =20 +static int ov13858_power_on(struct ov13858 *ov13858) +{ + int ret; + + ret =3D regulator_bulk_enable(OV13858_NUM_SUPPLIES, ov13858->supplies); + if (ret) { + dev_err(ov13858->dev, "failed to enable regulators: %d\n", ret); + return ret; + } + + ret =3D 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 =3D dev_get_drvdata(dev); + + return ov13858_power_on(to_ov13858(sd)); +} + +static int ov13858_runtime_suspend(struct device *dev) +{ + struct v4l2_subdev *sd =3D 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; =20 ov13858 =3D devm_kzalloc(&client->dev, sizeof(*ov13858), GFP_KERNEL); @@ -1722,14 +1795,34 @@ "external clock %lu is not supported\n", freq); =20 + for (i =3D 0; i < OV13858_NUM_SUPPLIES; i++) + ov13858->supplies[i].supply =3D ov13858_supply_names[i]; + + ret =3D 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 =3D 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); =20 + ret =3D ov13858_power_on(ov13858); + if (ret) + return ret; + /* Check module identity */ ret =3D ov13858_identify_module(ov13858); if (ret) { dev_err(ov13858->dev, "failed to find sensor: %d\n", ret); - return ret; + goto error_power_off; } =20 /* Set default mode to max resolution */ @@ -1737,7 +1830,7 @@ =20 ret =3D ov13858_init_controls(ov13858); if (ret) - return ret; + goto error_power_off; =20 /* Initialize subdev */ ov13858->sd.internal_ops =3D &ov13858_internal_ops; @@ -1773,6 +1866,9 @@ =20 error_handler_free: ov13858_free_controls(ov13858); + +error_power_off: + ov13858_power_off(ov13858); dev_err(ov13858->dev, "%s failed:%d\n", __func__, ret); =20 return ret; @@ -1788,6 +1884,9 @@ ov13858_free_controls(ov13858); =20 pm_runtime_disable(ov13858->dev); + if (!pm_runtime_status_suspended(ov13858->dev)) + ov13858_power_off(ov13858); + pm_runtime_set_suspended(ov13858->dev); } =20 static const struct i2c_device_id ov13858_id_table[] =3D { @@ -1810,6 +1909,7 @@ .driver =3D { .name =3D "ov13858", .acpi_match_table =3D ACPI_PTR(ov13858_acpi_ids), + .pm =3D pm_ptr(&ov13858_pm_ops), }, .probe =3D ov13858_probe, .remove =3D ov13858_remove,