This is an automatic generated email to let you know that the following patch 
were queued:

Subject: media: i2c: ov4689: Use sub-device active state
Author:  Mikhail Rudenko <mike.rude...@gmail.com>
Date:    Tue Apr 16 23:45:15 2024 +0100

Use sub-device active state. Employ control handler lock to
synchronize access to the active state and s_stream.

Signed-off-by: Mikhail Rudenko <mike.rude...@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mche...@kernel.org>

 drivers/media/i2c/ov4689.c | 75 +++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 44 deletions(-)

---

diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index 3c77a235320a..56f6d47c7674 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -86,7 +86,6 @@ struct ov4689 {
 
        u32 clock_rate;
 
-       struct mutex mutex; /* lock to protect ctrls and cur_mode */
        struct v4l2_ctrl_handler ctrl_handler;
        struct v4l2_ctrl *exposure;
 
@@ -319,19 +318,6 @@ static int ov4689_set_fmt(struct v4l2_subdev *sd,
        return 0;
 }
 
-static int ov4689_get_fmt(struct v4l2_subdev *sd,
-                         struct v4l2_subdev_state *sd_state,
-                         struct v4l2_subdev_format *fmt)
-{
-       struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
-       struct ov4689 *ov4689 = to_ov4689(sd);
-
-       /* only one mode supported for now */
-       ov4689_fill_fmt(ov4689->cur_mode, mbus_fmt);
-
-       return 0;
-}
-
 static int ov4689_enum_mbus_code(struct v4l2_subdev *sd,
                                 struct v4l2_subdev_state *sd_state,
                                 struct v4l2_subdev_mbus_code_enum *code)
@@ -405,10 +391,11 @@ static int ov4689_get_selection(struct v4l2_subdev *sd,
 static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
 {
        struct ov4689 *ov4689 = to_ov4689(sd);
+       struct v4l2_subdev_state *sd_state;
        struct device *dev = ov4689->dev;
        int ret = 0;
 
-       mutex_lock(&ov4689->mutex);
+       sd_state = v4l2_subdev_lock_and_get_active_state(&ov4689->subdev);
 
        if (on) {
                ret = pm_runtime_resume_and_get(dev);
@@ -443,7 +430,7 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
        }
 
 unlock_and_return:
-       mutex_unlock(&ov4689->mutex);
+       v4l2_subdev_unlock_state(sd_state);
 
        return ret;
 }
@@ -506,18 +493,13 @@ static int __maybe_unused ov4689_power_off(struct device 
*dev)
        return 0;
 }
 
-static int ov4689_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+static int ov4689_init_state(struct v4l2_subdev *sd,
+                            struct v4l2_subdev_state *sd_state)
 {
-       struct ov4689 *ov4689 = to_ov4689(sd);
-       struct v4l2_mbus_framefmt *try_fmt;
-
-       mutex_lock(&ov4689->mutex);
-
-       try_fmt = v4l2_subdev_state_get_format(fh->state, 0);
-       /* Initialize try_fmt */
-       ov4689_fill_fmt(&supported_modes[OV4689_MODE_2688_1520], try_fmt);
+       struct v4l2_mbus_framefmt *fmt =
+               v4l2_subdev_state_get_format(sd_state, 0);
 
-       mutex_unlock(&ov4689->mutex);
+       ov4689_fill_fmt(&supported_modes[OV4689_MODE_2688_1520], fmt);
 
        return 0;
 }
@@ -526,10 +508,6 @@ static const struct dev_pm_ops ov4689_pm_ops = {
        SET_RUNTIME_PM_OPS(ov4689_power_off, ov4689_power_on, NULL)
 };
 
-static const struct v4l2_subdev_internal_ops ov4689_internal_ops = {
-       .open = ov4689_open,
-};
-
 static const struct v4l2_subdev_video_ops ov4689_video_ops = {
        .s_stream = ov4689_s_stream,
 };
@@ -537,11 +515,15 @@ static const struct v4l2_subdev_video_ops 
ov4689_video_ops = {
 static const struct v4l2_subdev_pad_ops ov4689_pad_ops = {
        .enum_mbus_code = ov4689_enum_mbus_code,
        .enum_frame_size = ov4689_enum_frame_sizes,
-       .get_fmt = ov4689_get_fmt,
+       .get_fmt = v4l2_subdev_get_fmt,
        .set_fmt = ov4689_set_fmt,
        .get_selection = ov4689_get_selection,
 };
 
+static const struct v4l2_subdev_internal_ops ov4689_internal_ops = {
+       .init_state = ov4689_init_state,
+};
+
 static const struct v4l2_subdev_ops ov4689_subdev_ops = {
        .video = &ov4689_video_ops,
        .pad = &ov4689_pad_ops,
@@ -649,7 +631,6 @@ static int ov4689_initialize_controls(struct ov4689 *ov4689)
        ret = v4l2_ctrl_handler_init(handler, 10);
        if (ret)
                return ret;
-       handler->lock = &ov4689->mutex;
 
        ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ, 0, 0,
                                      link_freq_menu_items);
@@ -862,13 +843,15 @@ static int ov4689_probe(struct i2c_client *client)
                return dev_err_probe(dev, ret,
                                     "Failed to get power regulators\n");
 
-       mutex_init(&ov4689->mutex);
-
        sd = &ov4689->subdev;
        v4l2_i2c_subdev_init(sd, client, &ov4689_subdev_ops);
+       sd->internal_ops = &ov4689_internal_ops;
+       sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
        ret = ov4689_initialize_controls(ov4689);
-       if (ret)
-               goto err_destroy_mutex;
+       if (ret) {
+               dev_err(dev, "Failed to initialize controls\n");
+               return ret;
+       }
 
        ret = ov4689_power_on(dev);
        if (ret)
@@ -878,19 +861,24 @@ static int ov4689_probe(struct i2c_client *client)
        if (ret)
                goto err_power_off;
 
-       sd->internal_ops = &ov4689_internal_ops;
-       sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
 
-       ov4689->pad.flags = MEDIA_PAD_FL_SOURCE;
        sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+       ov4689->pad.flags = MEDIA_PAD_FL_SOURCE;
        ret = media_entity_pads_init(&sd->entity, 1, &ov4689->pad);
        if (ret < 0)
                goto err_power_off;
 
+       sd->state_lock = ov4689->ctrl_handler.lock;
+       ret = v4l2_subdev_init_finalize(sd);
+       if (ret) {
+               dev_err(dev, "Could not register v4l2 device\n");
+               goto err_clean_entity;
+       }
+
        ret = v4l2_async_register_subdev_sensor(sd);
        if (ret) {
                dev_err(dev, "v4l2 async register subdev failed\n");
-               goto err_clean_entity;
+               goto err_clean_subdev;
        }
 
        pm_runtime_set_active(dev);
@@ -899,14 +887,14 @@ static int ov4689_probe(struct i2c_client *client)
 
        return 0;
 
+err_clean_subdev:
+       v4l2_subdev_cleanup(sd);
 err_clean_entity:
        media_entity_cleanup(&sd->entity);
 err_power_off:
        ov4689_power_off(dev);
 err_free_handler:
        v4l2_ctrl_handler_free(&ov4689->ctrl_handler);
-err_destroy_mutex:
-       mutex_destroy(&ov4689->mutex);
 
        return ret;
 }
@@ -918,9 +906,8 @@ static void ov4689_remove(struct i2c_client *client)
 
        v4l2_async_unregister_subdev(sd);
        media_entity_cleanup(&sd->entity);
-
+       v4l2_subdev_cleanup(sd);
        v4l2_ctrl_handler_free(&ov4689->ctrl_handler);
-       mutex_destroy(&ov4689->mutex);
 
        pm_runtime_disable(&client->dev);
        if (!pm_runtime_status_suspended(&client->dev))

Reply via email to