Re: [PATCH] media: i2c: ov5693: Use sub-device active state

[Simon Kruse]

Hi Dan,

Thanks for the review.

On 27/10/2026 15:49, Dan Scally wrote:
> This patch does quite a lot in one go, it would be better if it could be broken down into a number
> of smaller logical changes. You're removing here the lock from the driver and instead using the
> default one from v4l2_ctrl_handler. That might be a fine idea, but I don't think it's necessary to
> switch the driver to use the subdev active state right? Could it be broken out?

Agreed, it is not needed. v2 keeps the driver mutex and uses it as the
state lock, so ov5693_sensor_resume() stays as it is.

> Why did this function call move to here from ov5693_sensor_init()?

With the 1 s autosuspend delay, pm_runtime_resume_and_get() in
s_stream() skips the resume callback if the sensor is still powered from
the previous stream, so a set_fmt or set_selection made in that window
never reaches the sensor. Marco Ziliani has since posted that fix on
its own, with measurements on several Surface models [1], so v2 drops
it from my patch and is based on his series.

> There's a FIXME in this function that now no longer applies and can be removed.

Will do.

> Descending order of line length please.

Will fix.

[1] https://lore.kernel.org/linux-media/20261027-ov5693-fixes-v1-0-3c8ab1ae104d@xxxxxxxxx/

Regards,
Simon