Re: [PATCH 3/8] media: i2c: ov08d10: add support for reset and power management

[Philipp Zabel]

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