Switch to using the sub-device state lock and properly call
v4l2_subdev_init_finalize() / v4l2_subdev_cleanup() on probe() /
remove().
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Reviewed-by: Tarang Raval <tarang.raval@siliconsignals.io>
---
drivers/media/i2c/ov5647.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 24a160bbb079..822804633d0f 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -95,7 +95,6 @@ struct ov5647 {
struct v4l2_subdev sd;
struct regmap *regmap;
struct media_pad pad;
- struct mutex lock;
struct clk *xclk;
struct gpio_desc *pwdn;
bool clock_ncont;
@@ -799,10 +798,10 @@ __ov5647_get_pad_crop(struct ov5647 *ov5647,
static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
- struct ov5647 *sensor = to_sensor(sd);
+ struct v4l2_subdev_state *state;
int ret;
- mutex_lock(&sensor->lock);
+ state = v4l2_subdev_lock_and_get_active_state(sd);
if (enable) {
ret = pm_runtime_resume_and_get(&client->dev);
@@ -823,14 +822,14 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
pm_runtime_put(&client->dev);
}
- mutex_unlock(&sensor->lock);
+ v4l2_subdev_unlock_state(state);
return 0;
error_pm:
pm_runtime_put(&client->dev);
error_unlock:
- mutex_unlock(&sensor->lock);
+ v4l2_subdev_unlock_state(state);
return ret;
}
@@ -878,7 +877,6 @@ static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,
const struct v4l2_mbus_framefmt *sensor_format;
struct ov5647 *sensor = to_sensor(sd);
- mutex_lock(&sensor->lock);
switch (format->which) {
case V4L2_SUBDEV_FORMAT_TRY:
sensor_format = v4l2_subdev_state_get_format(sd_state,
@@ -890,7 +888,6 @@ static int ov5647_get_pad_fmt(struct v4l2_subdev *sd,
}
*fmt = *sensor_format;
- mutex_unlock(&sensor->lock);
return 0;
}
@@ -908,7 +905,6 @@ static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
fmt->width, fmt->height);
/* Update the sensor mode and apply at it at streamon time. */
- mutex_lock(&sensor->lock);
if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
*v4l2_subdev_state_get_format(sd_state, format->pad) = mode->format;
} else {
@@ -937,7 +933,6 @@ static int ov5647_set_pad_fmt(struct v4l2_subdev *sd,
exposure_def);
}
*fmt = mode->format;
- mutex_unlock(&sensor->lock);
return 0;
}
@@ -950,10 +945,8 @@ static int ov5647_get_selection(struct v4l2_subdev *sd,
case V4L2_SEL_TGT_CROP: {
struct ov5647 *sensor = to_sensor(sd);
- mutex_lock(&sensor->lock);
sel->r = *__ov5647_get_pad_crop(sensor, sd_state, sel->pad,
sel->which);
- mutex_unlock(&sensor->lock);
return 0;
}
@@ -1046,9 +1039,6 @@ static int ov5647_s_ctrl(struct v4l2_ctrl *ctrl)
struct i2c_client *client = v4l2_get_subdevdata(sd);
int ret = 0;
-
- /* v4l2_ctrl_lock() locks our own mutex */
-
if (ctrl->id == V4L2_CID_VBLANK) {
int exposure_max, exposure_def;
@@ -1263,13 +1253,11 @@ static int ov5647_probe(struct i2c_client *client)
return -EINVAL;
}
- mutex_init(&sensor->lock);
-
sensor->mode = OV5647_DEFAULT_MODE;
ret = ov5647_init_controls(sensor);
if (ret)
- goto mutex_destroy;
+ return ret;
sd = &sensor->sd;
v4l2_i2c_subdev_init(sd, client, &ov5647_subdev_ops);
@@ -1297,9 +1285,16 @@ static int ov5647_probe(struct i2c_client *client)
if (ret < 0)
goto power_off;
+ sd->state_lock = sensor->ctrls.lock;
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret < 0) {
+ ret = dev_err_probe(dev, ret, "failed to init subdev\n");
+ goto power_off;
+ }
+
ret = v4l2_async_register_subdev(sd);
if (ret < 0)
- goto power_off;
+ goto v4l2_subdev_cleanup;
/* Enable runtime PM and turn off the device */
pm_runtime_set_active(dev);
@@ -1310,14 +1305,14 @@ static int ov5647_probe(struct i2c_client *client)
return 0;
+v4l2_subdev_cleanup:
+ v4l2_subdev_cleanup(sd);
power_off:
ov5647_power_off(dev);
entity_cleanup:
media_entity_cleanup(&sd->entity);
ctrl_handler_free:
v4l2_ctrl_handler_free(&sensor->ctrls);
-mutex_destroy:
- mutex_destroy(&sensor->lock);
return ret;
}
@@ -1328,11 +1323,11 @@ static void ov5647_remove(struct i2c_client *client)
struct ov5647 *sensor = to_sensor(sd);
v4l2_async_unregister_subdev(&sensor->sd);
+ v4l2_subdev_cleanup(sd);
media_entity_cleanup(&sensor->sd.entity);
v4l2_ctrl_handler_free(&sensor->ctrls);
v4l2_device_unregister_subdev(sd);
pm_runtime_disable(&client->dev);
- mutex_destroy(&sensor->lock);
}
static const struct dev_pm_ops ov5647_pm_ops = {
--
2.43.0
Hi Xiaolei,
On Thu, Jan 01, 2026 at 06:30:00PM +0800, Xiaolei Wang wrote:
> @@ -1297,9 +1285,16 @@ static int ov5647_probe(struct i2c_client *client)
> if (ret < 0)
> goto power_off;
>
> + sd->state_lock = sensor->ctrls.lock;
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret < 0) {
> + ret = dev_err_probe(dev, ret, "failed to init subdev\n");
> + goto power_off;
> + }
> +
> ret = v4l2_async_register_subdev(sd);
v4l2_async_register_subdev() needs to be called before enabling Runtime PM
below. Not a fault of this patch but it'd be nice to fix that.
> if (ret < 0)
> - goto power_off;
> + goto v4l2_subdev_cleanup;
>
> /* Enable runtime PM and turn off the device */
> pm_runtime_set_active(dev);
The first patch doesn't seem to apply; I've taken earlier a set of ov5647
patches from Dave, could you rebase this on them? They're in my "cleanup"
branch here
<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=cleanup>.
--
Kind regards,
Sakari Ailus
Hi Sakari,
On 1/8/26 06:38, Sakari Ailus wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Xiaolei,
>
> On Thu, Jan 01, 2026 at 06:30:00PM +0800, Xiaolei Wang wrote:
>> @@ -1297,9 +1285,16 @@ static int ov5647_probe(struct i2c_client *client)
>> if (ret < 0)
>> goto power_off;
>>
>> + sd->state_lock = sensor->ctrls.lock;
>> + ret = v4l2_subdev_init_finalize(sd);
>> + if (ret < 0) {
>> + ret = dev_err_probe(dev, ret, "failed to init subdev\n");
>> + goto power_off;
>> + }
>> +
>> ret = v4l2_async_register_subdev(sd);
> v4l2_async_register_subdev() needs to be called before enabling Runtime PM
> below. Not a fault of this patch but it'd be nice to fix that.
Thank you for the review and feedback.
You're absolutely right about the Runtime PM timing issue with
v4l2_async_register_subdev(). I appreciate you pointing that out - I'll
address this in the next version.
>
>> if (ret < 0)
>> - goto power_off;
>> + goto v4l2_subdev_cleanup;
>>
>> /* Enable runtime PM and turn off the device */
>> pm_runtime_set_active(dev);
> The first patch doesn't seem to apply; I've taken earlier a set of ov5647
> patches from Dave, could you rebase this on them? They're in my "cleanup"
> branch here
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=cleanup>.
I apologize for the conflict with the first patch. I wasn't aware of Dave's
patches that you had already taken. I'll rebase my series on your cleanup
branch and resend a v5.
Best regards,
Xiaolei
>
> --
> Kind regards,
>
> Sakari Ailus
© 2016 - 2026 Red Hat, Inc.