Skip to contentSkip to conversation

media: ov2680: don't register the v4l2 subdevice before checking chip ID

PATCH New Javier Martinez Canillas 1 participant 2 messages Sep 1, 2018

Subsystems (3)
Media (V4L/DVB) V4L2 camera sensor drivers Omnivision OV2680 sensor driver
lore
Share & tools
lore

Javier Martinez Canillas

author
#0 lore raw

PATCH v2 1/2 media: ov2680: don't register the v4l2 subdevice before checking chip ID

The driver registers the v4l2 subdevice before attempting to power on the chip and checking its ID. This means that a media device driver that it's waiting for this subdevice to be bound, will prematurely expose its media device node to userspace because if something goes wrong the media entity will be cleaned up again on the ov2680 probe function.

This also simplifies the probe function error path since no initialization is made before attempting to enable the resources or checking the chip ID.

Fixes 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver")
Signed-off-by Javier Martinez Canillas <[email protected]>

---

Changes in v2:
- Just move ov2680_check_id() before ov2680_v4l2_init() - Suggested by Sakari.

 drivers/media/i2c/ov2680.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
drivers/media/i2c/ov2680.c +4 -10
index f753a1c333ef..3ccd584568fb 100644
@@ -1088,26 +1088,20 @@ static int ov2680_probe(struct i2c_client *client)
 
 	mutex_init(&sensor->lock);
 
-	ret = ov2680_v4l2_init(sensor);
+	ret = ov2680_check_id(sensor);
 	if (ret < 0)
 		goto lock_destroy;
 
-	ret = ov2680_check_id(sensor);
+	ret = ov2680_v4l2_init(sensor);
 	if (ret < 0)
-		goto error_cleanup;
+		goto lock_destroy;
 
 	dev_info(dev, "ov2680 init correctly\n");
 
 	return 0;
 
-error_cleanup:
-	dev_err(dev, "ov2680 init fail: %d\n", ret);
-
-	media_entity_cleanup(&sensor->sd.entity);
-	v4l2_async_unregister_subdev(&sensor->sd);
-	v4l2_ctrl_handler_free(&sensor->ctrls.handler);
-
 lock_destroy:
+	dev_err(dev, "ov2680 init fail: %d\n", ret);
 	mutex_destroy(&sensor->lock);
 
 	return ret;
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v2 2/2 media: ov2680: rename ov2680_v4l2_init() to ov2680_v4l2_register()

The function not only does initialization but also registers the subdevice so change its name to make this more clear.

Signed-off-by Javier Martinez Canillas <[email protected]>

---

Changes in v2: None

 drivers/media/i2c/ov2680.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/i2c/ov2680.c +2 -2
index 3ccd584568fb..0e34e15b67b3 100644
@@ -926,7 +926,7 @@ static int ov2680_mode_init(struct ov2680_dev *sensor)
 	return 0;
 }
 
-static int ov2680_v4l2_init(struct ov2680_dev *sensor)
+static int ov2680_v4l2_register(struct ov2680_dev *sensor)
 {
 	const struct v4l2_ctrl_ops *ops = &ov2680_ctrl_ops;
 	struct ov2680_ctrls *ctrls = &sensor->ctrls;

@@ -1092,7 +1092,7 @@ static int ov2680_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto lock_destroy;
 
-	ret = ov2680_v4l2_init(sensor);
+	ret = ov2680_v4l2_register(sensor);
 	if (ret < 0)
 		goto lock_destroy;
Signature · 1 line
2.17.1