Add support for the required power supplies as well as the control of an
optional sensor reset.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 drivers/media/i2c/ov08d10.c | 104 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 97 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
index cfe18dcde174ddc1f198cb2aaa6b4a3b34045508..4dba264488b3e1950016deb3fa34732871cc34fc 100644
--- a/drivers/media/i2c/ov08d10.c
+++ b/drivers/media/i2c/ov08d10.c
@@ -8,6 +8,7 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -514,9 +515,17 @@ static const char * const ov08d10_test_pattern_menu[] = {
 	"Standard Color Bar",
 };
 
+static const char *const ov08d10_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
 struct ov08d10 {
 	struct device *dev;
 	struct clk *clk;
+	struct reset_control *reset;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(ov08d10_supply_names)];
 
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -1266,6 +1275,56 @@ static const struct v4l2_subdev_internal_ops ov08d10_internal_ops = {
 	.open = ov08d10_open,
 };
 
+static int ov08d10_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov08d10 *ov08d10 = to_ov08d10(sd);
+
+	reset_control_assert(ov08d10->reset);
+
+	regulator_bulk_disable(ARRAY_SIZE(ov08d10->supplies),
+			       ov08d10->supplies);
+
+	clk_disable_unprepare(ov08d10->clk);
+
+	return 0;
+}
+
+static int ov08d10_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov08d10 *ov08d10 = to_ov08d10(sd);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ov08d10->supplies),
+				    ov08d10->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ov08d10->clk);
+	if (ret < 0) {
+		regulator_bulk_disable(ARRAY_SIZE(ov08d10->supplies),
+				       ov08d10->supplies);
+
+		dev_err(dev, "failed to enable imaging clock: %d\n", ret);
+		return ret;
+	}
+
+	if (ov08d10->reset) {
+		/* Delay from DVDD stable to sensor XSHUTDN pull up: 5ms */
+		fsleep(5 * USEC_PER_MSEC);
+
+		reset_control_deassert(ov08d10->reset);
+
+		/* Delay from XSHUTDN pull up to SCCB start: 8ms */
+		fsleep(8 * USEC_PER_MSEC);
+	}
+
+	return 0;
+}
+
 static int ov08d10_identify_module(struct ov08d10 *ov08d10)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&ov08d10->sd);
@@ -1372,6 +1431,10 @@ static void ov08d10_remove(struct i2c_client *client)
 	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(ov08d10->dev);
+	if (!pm_runtime_status_suspended(ov08d10->dev)) {
+		ov08d10_power_off(ov08d10->dev);
+		pm_runtime_set_suspended(ov08d10->dev);
+	}
 	mutex_destroy(&ov08d10->mutex);
 }
 
@@ -1379,6 +1442,7 @@ static int ov08d10_probe(struct i2c_client *client)
 {
 	struct ov08d10 *ov08d10;
 	unsigned long freq;
+	unsigned int i;
 	int ret;
 
 	ov08d10 = devm_kzalloc(&client->dev, sizeof(*ov08d10), GFP_KERNEL);
@@ -1404,12 +1468,32 @@ static int ov08d10_probe(struct i2c_client *client)
 		return ret;
 	}
 
+	ov08d10->reset = devm_reset_control_get_optional(ov08d10->dev, NULL);
+	if (IS_ERR(ov08d10->reset))
+		return dev_err_probe(ov08d10->dev, PTR_ERR(ov08d10->reset),
+				     "failed to get reset\n");
+	reset_control_assert(ov08d10->reset);
+
+	for (i = 0; i < ARRAY_SIZE(ov08d10_supply_names); i++)
+		ov08d10->supplies[i].supply = ov08d10_supply_names[i];
+
+	ret = devm_regulator_bulk_get(ov08d10->dev,
+				      ARRAY_SIZE(ov08d10->supplies),
+				      ov08d10->supplies);
+	if (ret)
+		return dev_err_probe(ov08d10->dev, ret,
+				     "failed to get regulators\n");
+
 	v4l2_i2c_subdev_init(&ov08d10->sd, client, &ov08d10_subdev_ops);
 
+	ret = ov08d10_power_on(ov08d10->dev);
+	if (ret)
+		return dev_err_probe(ov08d10->dev, ret, "failed to power on\n");
+
 	ret = ov08d10_identify_module(ov08d10);
 	if (ret) {
 		dev_err(ov08d10->dev, "failed to find sensor: %d", ret);
-		return ret;
+		goto probe_error_power_off;
 	}
 
 	mutex_init(&ov08d10->mutex);
@@ -1430,6 +1514,9 @@ static int ov08d10_probe(struct i2c_client *client)
 		goto probe_error_v4l2_ctrl_handler_free;
 	}
 
+	pm_runtime_set_active(ov08d10->dev);
+	pm_runtime_enable(ov08d10->dev);
+
 	ret = v4l2_async_register_subdev_sensor(&ov08d10->sd);
 	if (ret < 0) {
 		dev_err(ov08d10->dev, "failed to register V4L2 subdev: %d",
@@ -1437,26 +1524,28 @@ static int ov08d10_probe(struct i2c_client *client)
 		goto probe_error_media_entity_cleanup;
 	}
 
-	/*
-	 * Device is already turned on by i2c-core with ACPI domain PM.
-	 * Enable runtime PM and turn off the device.
-	 */
-	pm_runtime_set_active(ov08d10->dev);
-	pm_runtime_enable(ov08d10->dev);
 	pm_runtime_idle(ov08d10->dev);
 
 	return 0;
 
 probe_error_media_entity_cleanup:
+	pm_runtime_disable(ov08d10->dev);
+	pm_runtime_set_suspended(ov08d10->dev);
 	media_entity_cleanup(&ov08d10->sd.entity);
 
 probe_error_v4l2_ctrl_handler_free:
 	v4l2_ctrl_handler_free(ov08d10->sd.ctrl_handler);
 	mutex_destroy(&ov08d10->mutex);
 
+probe_error_power_off:
+	ov08d10_power_off(ov08d10->dev);
+
 	return ret;
 }
 
+static DEFINE_RUNTIME_DEV_PM_OPS(ov08d10_pm_ops,
+				 ov08d10_power_off, ov08d10_power_on, NULL);
+
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov08d10_acpi_ids[] = {
 	{ "OVTI08D1" },
@@ -1475,6 +1564,7 @@ MODULE_DEVICE_TABLE(of, ov08d10_of_match);
 static struct i2c_driver ov08d10_i2c_driver = {
 	.driver = {
 		.name = "ov08d10",
+		.pm = pm_ptr(&ov08d10_pm_ops),
 		.acpi_match_table = ACPI_PTR(ov08d10_acpi_ids),
 		.of_match_table = ov08d10_of_match,
 	},

-- 
2.34.1

On Do, 2026-02-26 at 09:56 +0100, Matthias Fend wrote:
> Add support for the required power supplies as well as the control of an
> optional sensor reset.
> 
> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
>  drivers/media/i2c/ov08d10.c | 104 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 97 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
> index cfe18dcde174ddc1f198cb2aaa6b4a3b34045508..4dba264488b3e1950016deb3fa34732871cc34fc 100644
> --- a/drivers/media/i2c/ov08d10.c
> +++ b/drivers/media/i2c/ov08d10.c
[...]
> @@ -1379,6 +1442,7 @@ static int ov08d10_probe(struct i2c_client *client)
>  {
>  	struct ov08d10 *ov08d10;
>  	unsigned long freq;
> +	unsigned int i;
>  	int ret;
>  
>  	ov08d10 = devm_kzalloc(&client->dev, sizeof(*ov08d10), GFP_KERNEL);
> @@ -1404,12 +1468,32 @@ static int ov08d10_probe(struct i2c_client *client)
>  		return ret;
>  	}
>  
> +	ov08d10->reset = devm_reset_control_get_optional(ov08d10->dev, NULL);

Please use devm_reset_control_get_optional_exclusive() directly.

> +	if (IS_ERR(ov08d10->reset))
> +		return dev_err_probe(ov08d10->dev, PTR_ERR(ov08d10->reset),
> +				     "failed to get reset\n");
> +	reset_control_assert(ov08d10->reset);
> +
> +	for (i = 0; i < ARRAY_SIZE(ov08d10_supply_names); i++)
> +		ov08d10->supplies[i].supply = ov08d10_supply_names[i];
> +
> +	ret = devm_regulator_bulk_get(ov08d10->dev,
> +				      ARRAY_SIZE(ov08d10->supplies),
> +				      ov08d10->supplies);
> +	if (ret)
> +		return dev_err_probe(ov08d10->dev, ret,
> +				     "failed to get regulators\n");
> +
>  	v4l2_i2c_subdev_init(&ov08d10->sd, client, &ov08d10_subdev_ops);
>  
> +	ret = ov08d10_power_on(ov08d10->dev);
> +	if (ret)
> +		return dev_err_probe(ov08d10->dev, ret, "failed to power on\n");
> +
>  	ret = ov08d10_identify_module(ov08d10);
>  	if (ret) {
>  		dev_err(ov08d10->dev, "failed to find sensor: %d", ret);
> -		return ret;
> +		goto probe_error_power_off;
>  	}
>  
>  	mutex_init(&ov08d10->mutex);
> @@ -1430,6 +1514,9 @@ static int ov08d10_probe(struct i2c_client *client)
>  		goto probe_error_v4l2_ctrl_handler_free;
>  	}
>  
> +	pm_runtime_set_active(ov08d10->dev);
> +	pm_runtime_enable(ov08d10->dev);
> +
>  	ret = v4l2_async_register_subdev_sensor(&ov08d10->sd);
>  	if (ret < 0) {
>  		dev_err(ov08d10->dev, "failed to register V4L2 subdev: %d",
> @@ -1437,26 +1524,28 @@ static int ov08d10_probe(struct i2c_client *client)
>  		goto probe_error_media_entity_cleanup;
>  	}
>  
> -	/*
> -	 * Device is already turned on by i2c-core with ACPI domain PM.
> -	 * Enable runtime PM and turn off the device.
> -	 */

The commit message does not explain why this comment is dropped.

> -	pm_runtime_set_active(ov08d10->dev);
> -	pm_runtime_enable(ov08d10->dev);
>  	pm_runtime_idle(ov08d10->dev);
>  
>  	return 0;
>  
>  probe_error_media_entity_cleanup:
> +	pm_runtime_disable(ov08d10->dev);
> +	pm_runtime_set_suspended(ov08d10->dev);

Does this do the correct thing if v4l2_async_register_subdev_sensor()
returns -EPROBE_DEFER (for example via privacy led) and then it probes
a second time? It looks like the assumption pm_runtime_set_active()
doesn't hold then.

>  	media_entity_cleanup(&ov08d10->sd.entity);
>  
>  probe_error_v4l2_ctrl_handler_free:
>  	v4l2_ctrl_handler_free(ov08d10->sd.ctrl_handler);
>  	mutex_destroy(&ov08d10->mutex);
>  
> +probe_error_power_off:
> +	ov08d10_power_off(ov08d10->dev);
> +
>  	return ret;
>  }

regards
Philipp

Hi Philipp,

thanks for your feedback.

Am 26.02.2026 um 11:13 schrieb Philipp Zabel:
> On Do, 2026-02-26 at 09:56 +0100, Matthias Fend wrote:
>> Add support for the required power supplies as well as the control of an
>> optional sensor reset.
>>
>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
>> ---
>>   drivers/media/i2c/ov08d10.c | 104 +++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 97 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
>> index cfe18dcde174ddc1f198cb2aaa6b4a3b34045508..4dba264488b3e1950016deb3fa34732871cc34fc 100644
>> --- a/drivers/media/i2c/ov08d10.c
>> +++ b/drivers/media/i2c/ov08d10.c
> [...]
>> @@ -1379,6 +1442,7 @@ static int ov08d10_probe(struct i2c_client *client)
>>   {
>>   	struct ov08d10 *ov08d10;
>>   	unsigned long freq;
>> +	unsigned int i;
>>   	int ret;
>>   
>>   	ov08d10 = devm_kzalloc(&client->dev, sizeof(*ov08d10), GFP_KERNEL);
>> @@ -1404,12 +1468,32 @@ static int ov08d10_probe(struct i2c_client *client)
>>   		return ret;
>>   	}
>>   
>> +	ov08d10->reset = devm_reset_control_get_optional(ov08d10->dev, NULL);
> 
> Please use devm_reset_control_get_optional_exclusive() directly.

ACK

> 
>> +	if (IS_ERR(ov08d10->reset))
>> +		return dev_err_probe(ov08d10->dev, PTR_ERR(ov08d10->reset),
>> +				     "failed to get reset\n");
>> +	reset_control_assert(ov08d10->reset);
>> +
>> +	for (i = 0; i < ARRAY_SIZE(ov08d10_supply_names); i++)
>> +		ov08d10->supplies[i].supply = ov08d10_supply_names[i];
>> +
>> +	ret = devm_regulator_bulk_get(ov08d10->dev,
>> +				      ARRAY_SIZE(ov08d10->supplies),
>> +				      ov08d10->supplies);
>> +	if (ret)
>> +		return dev_err_probe(ov08d10->dev, ret,
>> +				     "failed to get regulators\n");
>> +
>>   	v4l2_i2c_subdev_init(&ov08d10->sd, client, &ov08d10_subdev_ops);
>>   
>> +	ret = ov08d10_power_on(ov08d10->dev);
>> +	if (ret)
>> +		return dev_err_probe(ov08d10->dev, ret, "failed to power on\n");
>> +
>>   	ret = ov08d10_identify_module(ov08d10);
>>   	if (ret) {
>>   		dev_err(ov08d10->dev, "failed to find sensor: %d", ret);
>> -		return ret;
>> +		goto probe_error_power_off;
>>   	}
>>   
>>   	mutex_init(&ov08d10->mutex);
>> @@ -1430,6 +1514,9 @@ static int ov08d10_probe(struct i2c_client *client)
>>   		goto probe_error_v4l2_ctrl_handler_free;
>>   	}
>>   
>> +	pm_runtime_set_active(ov08d10->dev);
>> +	pm_runtime_enable(ov08d10->dev);
>> +
>>   	ret = v4l2_async_register_subdev_sensor(&ov08d10->sd);
>>   	if (ret < 0) {
>>   		dev_err(ov08d10->dev, "failed to register V4L2 subdev: %d",
>> @@ -1437,26 +1524,28 @@ static int ov08d10_probe(struct i2c_client *client)
>>   		goto probe_error_media_entity_cleanup;
>>   	}
>>   
>> -	/*
>> -	 * Device is already turned on by i2c-core with ACPI domain PM.
>> -	 * Enable runtime PM and turn off the device.
>> -	 */
> 
> The commit message does not explain why this comment is dropped.

I didn't find the comment particularly helpful and since other sensors 
manage without it and there's now more than just ACPI, and "turn off" 
happens later, I thought it was fine to just drop the comment.

If you think it should still be included, I'd be happy to change it.

> 
>> -	pm_runtime_set_active(ov08d10->dev);
>> -	pm_runtime_enable(ov08d10->dev);
>>   	pm_runtime_idle(ov08d10->dev);
>>   
>>   	return 0;
>>   
>>   probe_error_media_entity_cleanup:
>> +	pm_runtime_disable(ov08d10->dev);
>> +	pm_runtime_set_suspended(ov08d10->dev);
> 
> Does this do the correct thing if v4l2_async_register_subdev_sensor()
> returns -EPROBE_DEFER (for example via privacy led) and then it probes
> a second time? It looks like the assumption pm_runtime_set_active()
> doesn't hold then.

At least it works as expected for me. But as mentioned, I don't have an 
ACPI hardware setup available. Does your point maybe refer to ACPI, or 
what exactly do you mean?
To me, it now looks very similar to other Omnivision ACPI drivers – do 
you perhaps have a specific suggestion for what should be changed?

Thanks a lot
  ~Matthias

> 
>>   	media_entity_cleanup(&ov08d10->sd.entity);
>>   
>>   probe_error_v4l2_ctrl_handler_free:
>>   	v4l2_ctrl_handler_free(ov08d10->sd.ctrl_handler);
>>   	mutex_destroy(&ov08d10->mutex);
>>   
>> +probe_error_power_off:
>> +	ov08d10_power_off(ov08d10->dev);
>> +
>>   	return ret;
>>   }
> 
> regards
> Philipp

Hi Matthias,

On Do, 2026-02-26 at 21:30 +0100, Matthias Fend wrote:

[...]
> > 
> > > @@ -1437,26 +1524,28 @@ static int ov08d10_probe(struct i2c_client *client)
> > >   		goto probe_error_media_entity_cleanup;
> > >   	}
> > >   
> > > -	/*
> > > -	 * Device is already turned on by i2c-core with ACPI domain PM.
> > > -	 * Enable runtime PM and turn off the device.
> > > -	 */
> > 
> > The commit message does not explain why this comment is dropped.
> 
> I didn't find the comment particularly helpful and since other sensors 
> manage without it and there's now more than just ACPI, and "turn off" 
> happens later, I thought it was fine to just drop the comment.
> 
> If you think it should still be included, I'd be happy to change it.

That's ok, I'd just add this explanation to the commit message.
Or something along the lines of dropping a comment that's obvious.

I wouldn't have pointed this out if it didn't look to me like the
"device is already turned on" precondition had maybe changed with the
point discussed below.

> > > -	pm_runtime_set_active(ov08d10->dev);
> > > -	pm_runtime_enable(ov08d10->dev);
> > >   	pm_runtime_idle(ov08d10->dev);
> > >   
> > >   	return 0;
> > >   
> > >   probe_error_media_entity_cleanup:
> > > +	pm_runtime_disable(ov08d10->dev);
> > > +	pm_runtime_set_suspended(ov08d10->dev);
> > 
> > Does this do the correct thing if v4l2_async_register_subdev_sensor()
> > returns -EPROBE_DEFER (for example via privacy led) and then it probes
> > a second time? It looks like the assumption pm_runtime_set_active()
> > doesn't hold then.
> 
> At least it works as expected for me. But as mentioned, I don't have an 
> ACPI hardware setup available. Does your point maybe refer to ACPI, or 
> what exactly do you mean?

I confused myself, thinking that disabling the device was moved into
the error path. But pm_runtime_idle() is still only called directly
before return 0, so my point is moot. Sorry for the noise.

regards
Philipp

Hi Matthias,

On Thu, Feb 26, 2026 at 09:30:08PM +0100, Matthias Fend wrote:
> Hi Philipp,
> 
> thanks for your feedback.
> 
> Am 26.02.2026 um 11:13 schrieb Philipp Zabel:
> > On Do, 2026-02-26 at 09:56 +0100, Matthias Fend wrote:
> > > Add support for the required power supplies as well as the control of an
> > > optional sensor reset.
> > > 
> > > Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> > > ---
> > >   drivers/media/i2c/ov08d10.c | 104 +++++++++++++++++++++++++++++++++++++++++---
> > >   1 file changed, 97 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
> > > index cfe18dcde174ddc1f198cb2aaa6b4a3b34045508..4dba264488b3e1950016deb3fa34732871cc34fc 100644
> > > --- a/drivers/media/i2c/ov08d10.c
> > > +++ b/drivers/media/i2c/ov08d10.c
> > [...]
> > > @@ -1379,6 +1442,7 @@ static int ov08d10_probe(struct i2c_client *client)
> > >   {
> > >   	struct ov08d10 *ov08d10;
> > >   	unsigned long freq;
> > > +	unsigned int i;
> > >   	int ret;
> > >   	ov08d10 = devm_kzalloc(&client->dev, sizeof(*ov08d10), GFP_KERNEL);
> > > @@ -1404,12 +1468,32 @@ static int ov08d10_probe(struct i2c_client *client)
> > >   		return ret;
> > >   	}
> > > +	ov08d10->reset = devm_reset_control_get_optional(ov08d10->dev, NULL);
> > 
> > Please use devm_reset_control_get_optional_exclusive() directly.
> 
> ACK
> 
> > 
> > > +	if (IS_ERR(ov08d10->reset))
> > > +		return dev_err_probe(ov08d10->dev, PTR_ERR(ov08d10->reset),
> > > +				     "failed to get reset\n");
> > > +	reset_control_assert(ov08d10->reset);
> > > +
> > > +	for (i = 0; i < ARRAY_SIZE(ov08d10_supply_names); i++)
> > > +		ov08d10->supplies[i].supply = ov08d10_supply_names[i];
> > > +
> > > +	ret = devm_regulator_bulk_get(ov08d10->dev,
> > > +				      ARRAY_SIZE(ov08d10->supplies),
> > > +				      ov08d10->supplies);
> > > +	if (ret)
> > > +		return dev_err_probe(ov08d10->dev, ret,
> > > +				     "failed to get regulators\n");
> > > +
> > >   	v4l2_i2c_subdev_init(&ov08d10->sd, client, &ov08d10_subdev_ops);
> > > +	ret = ov08d10_power_on(ov08d10->dev);
> > > +	if (ret)
> > > +		return dev_err_probe(ov08d10->dev, ret, "failed to power on\n");
> > > +
> > >   	ret = ov08d10_identify_module(ov08d10);
> > >   	if (ret) {
> > >   		dev_err(ov08d10->dev, "failed to find sensor: %d", ret);
> > > -		return ret;
> > > +		goto probe_error_power_off;
> > >   	}
> > >   	mutex_init(&ov08d10->mutex);
> > > @@ -1430,6 +1514,9 @@ static int ov08d10_probe(struct i2c_client *client)
> > >   		goto probe_error_v4l2_ctrl_handler_free;
> > >   	}
> > > +	pm_runtime_set_active(ov08d10->dev);
> > > +	pm_runtime_enable(ov08d10->dev);
> > > +
> > >   	ret = v4l2_async_register_subdev_sensor(&ov08d10->sd);
> > >   	if (ret < 0) {
> > >   		dev_err(ov08d10->dev, "failed to register V4L2 subdev: %d",
> > > @@ -1437,26 +1524,28 @@ static int ov08d10_probe(struct i2c_client *client)
> > >   		goto probe_error_media_entity_cleanup;
> > >   	}
> > > -	/*
> > > -	 * Device is already turned on by i2c-core with ACPI domain PM.
> > > -	 * Enable runtime PM and turn off the device.
> > > -	 */
> > 
> > The commit message does not explain why this comment is dropped.
> 
> I didn't find the comment particularly helpful and since other sensors
> manage without it and there's now more than just ACPI, and "turn off"
> happens later, I thought it was fine to just drop the comment.
> 
> If you think it should still be included, I'd be happy to change it.

I'd drop it and mention this in the commit message, there's nothing
specific to this driver here (nor even camera sensors in general).

> 
> > 
> > > -	pm_runtime_set_active(ov08d10->dev);
> > > -	pm_runtime_enable(ov08d10->dev);
> > >   	pm_runtime_idle(ov08d10->dev);
> > >   	return 0;
> > >   probe_error_media_entity_cleanup:
> > > +	pm_runtime_disable(ov08d10->dev);
> > > +	pm_runtime_set_suspended(ov08d10->dev);
> > 
> > Does this do the correct thing if v4l2_async_register_subdev_sensor()
> > returns -EPROBE_DEFER (for example via privacy led) and then it probes
> > a second time? It looks like the assumption pm_runtime_set_active()
> > doesn't hold then.
> 
> At least it works as expected for me. But as mentioned, I don't have an ACPI
> hardware setup available. Does your point maybe refer to ACPI, or what
> exactly do you mean?
> To me, it now looks very similar to other Omnivision ACPI drivers – do you
> perhaps have a specific suggestion for what should be changed?

Moving pm_runtime_set_active() and pm_runtime_enable() calls above
v4l2_async_register_subdev_sensor() (and related teardown changes) are
actually a bugfix (the driver's external APIs are exposed before runtime PM
is enabled and thus e.g. streaming on will fail). It'd be nice to post that
separately from non-ACPI power management changes.

-- 
Kind regards,

Sakari Ailus