Browse Source

msm: camera: common: Fix failures in v4l2 unregister during error

Missing video unregister was causing list delete corruption.
Also, there were double free errors in Sync and CRM driver. Fix
v4l2 issues by adding the missing unregister calls and changing
release callback to empty, because we are freeing it during
cleanup. Improve logging during bind/unbind to better reflect what
is happening.

CRs-Fixed: 2584631
Change-Id: Idc5db655d22df54e8bdb470d29896e10f6987796
Signed-off-by: Mukund Madhusudan Atre <[email protected]>
Mukund Madhusudan Atre 5 years ago
parent
commit
0a649b3063

+ 1 - 1
drivers/cam_icp/cam_icp_subdev.c

@@ -205,7 +205,7 @@ static int cam_icp_component_bind(struct device *dev,
 	g_icp_dev.open_cnt = 0;
 	mutex_init(&g_icp_dev.icp_lock);
 
-	CAM_DBG(CAM_ICP, "Component bound successfully");
+	CAM_INFO(CAM_ICP, "Component bound successfully");
 
 	return rc;
 

+ 1 - 1
drivers/cam_isp/cam_isp_dev.c

@@ -161,7 +161,7 @@ static int cam_isp_dev_component_bind(struct device *dev,
 
 	mutex_init(&g_isp_dev.isp_mutex);
 
-	CAM_INFO(CAM_ISP, "ISP HW component bound successfully");
+	CAM_INFO(CAM_ISP, "Component bound successfully");
 
 	return 0;
 unregister:

+ 73 - 30
drivers/cam_req_mgr/cam_req_mgr_dev.c

@@ -62,8 +62,8 @@ mdev_fail:
 
 static void cam_media_device_cleanup(void)
 {
-	media_entity_cleanup(&g_dev.video->entity);
 	media_device_unregister(g_dev.v4l2_dev->mdev);
+	media_device_cleanup(g_dev.v4l2_dev->mdev);
 	kfree(g_dev.v4l2_dev->mdev);
 	g_dev.v4l2_dev->mdev = NULL;
 }
@@ -575,7 +575,7 @@ static int cam_video_device_setup(void)
 
 	strlcpy(g_dev.video->name, "cam-req-mgr",
 		sizeof(g_dev.video->name));
-	g_dev.video->release = video_device_release;
+	g_dev.video->release = video_device_release_empty;
 	g_dev.video->fops = &g_cam_fops;
 	g_dev.video->ioctl_ops = &g_cam_ioctl_ops;
 	g_dev.video->minor = -1;
@@ -630,6 +630,7 @@ EXPORT_SYMBOL(cam_req_mgr_notify_message);
 
 void cam_video_device_cleanup(void)
 {
+	media_entity_cleanup(&g_dev.video->entity);
 	video_unregister_device(g_dev.video);
 	video_device_release(g_dev.video);
 	g_dev.video = NULL;
@@ -640,7 +641,7 @@ int cam_register_subdev(struct cam_subdev *csd)
 	struct v4l2_subdev *sd;
 	int rc;
 
-	if (g_dev.state != true) {
+	if (!g_dev.state) {
 		CAM_ERR(CAM_CRM, "camera root device not ready yet");
 		return -ENODEV;
 	}
@@ -651,6 +652,15 @@ int cam_register_subdev(struct cam_subdev *csd)
 	}
 
 	mutex_lock(&g_dev.dev_lock);
+	if ((g_dev.subdev_nodes_created) &&
+		(csd->sd_flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) {
+		CAM_ERR(CAM_CRM,
+			"dynamic node is not allowed, name: %s, type :%d",
+			csd->name, csd->ent_function);
+		rc = -EINVAL;
+		goto reg_fail;
+	}
+
 	sd = &csd->sd;
 	v4l2_subdev_init(sd, csd->ops);
 	sd->internal_ops = csd->internal_ops;
@@ -662,34 +672,12 @@ int cam_register_subdev(struct cam_subdev *csd)
 	sd->entity.pads = NULL;
 	sd->entity.function = csd->ent_function;
 
-	if (csd->subdev_node_created) {
-		CAM_ERR(CAM_CRM,
-			"Dynamic Node is not allowed, name: %s, type: %d",
-			csd->name, csd->ent_function);
-		rc = -EINVAL;
-		goto reg_fail;
-	}
-
 	rc = v4l2_device_register_subdev(g_dev.v4l2_dev, sd);
 	if (rc) {
 		CAM_ERR(CAM_CRM, "register subdev failed");
 		goto reg_fail;
 	}
 
-	rc = v4l2_device_register_subdev_nodes(g_dev.v4l2_dev);
-	if (rc) {
-		CAM_ERR(CAM_CRM,
-			"Device Register subdev node failed: rc = %d", rc);
-		goto reg_fail;
-
-	}
-
-	if ((sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE)) {
-		sd->entity.name = video_device_node_name(sd->devnode);
-		CAM_DBG(CAM_CRM, "created node :%s", sd->entity.name);
-	}
-
-	csd->subdev_node_created = true;
 	g_dev.count++;
 
 reg_fail:
@@ -700,7 +688,7 @@ EXPORT_SYMBOL(cam_register_subdev);
 
 int cam_unregister_subdev(struct cam_subdev *csd)
 {
-	if (g_dev.state != true) {
+	if (!g_dev.state) {
 		CAM_ERR(CAM_CRM, "camera root device not ready yet");
 		return -ENODEV;
 	}
@@ -714,6 +702,47 @@ int cam_unregister_subdev(struct cam_subdev *csd)
 }
 EXPORT_SYMBOL(cam_unregister_subdev);
 
+static int cam_dev_mgr_create_subdev_nodes(void)
+{
+	int rc;
+	struct v4l2_subdev *sd;
+
+	if (!g_dev.v4l2_dev) {
+		CAM_ERR(CAM_CRM, "V4L2 device not initialized");
+		return -EINVAL;
+	}
+
+	if (!g_dev.state) {
+		CAM_ERR(CAM_CRM, "camera root device not ready");
+		return -ENODEV;
+	}
+
+	mutex_lock(&g_dev.dev_lock);
+	if (g_dev.subdev_nodes_created) {
+		rc = -EEXIST;
+		goto create_fail;
+	}
+
+	rc = v4l2_device_register_subdev_nodes(g_dev.v4l2_dev);
+	if (rc) {
+		CAM_ERR(CAM_CRM, "failed to register the sub devices");
+		goto create_fail;
+	}
+
+	list_for_each_entry(sd, &g_dev.v4l2_dev->subdevs, list) {
+		if (!(sd->flags & V4L2_SUBDEV_FL_HAS_DEVNODE))
+			continue;
+		sd->entity.name = video_device_node_name(sd->devnode);
+		CAM_DBG(CAM_CRM, "created node :%s", sd->entity.name);
+	}
+
+	g_dev.subdev_nodes_created = true;
+
+create_fail:
+	mutex_unlock(&g_dev.dev_lock);
+	return rc;
+}
+
 static int cam_req_mgr_component_master_bind(struct device *dev)
 {
 	int rc = 0;
@@ -734,6 +763,7 @@ static int cam_req_mgr_component_master_bind(struct device *dev)
 	g_dev.open_cnt = 0;
 	mutex_init(&g_dev.cam_lock);
 	spin_lock_init(&g_dev.cam_eventq_lock);
+	g_dev.subdev_nodes_created = false;
 	mutex_init(&g_dev.dev_lock);
 
 	rc = cam_req_mgr_util_init();
@@ -748,6 +778,8 @@ static int cam_req_mgr_component_master_bind(struct device *dev)
 		goto req_mgr_core_fail;
 	}
 
+	g_dev.state = true;
+
 	if (g_cam_req_mgr_timer_cachep == NULL) {
 		g_cam_req_mgr_timer_cachep = kmem_cache_create("crm_timer",
 			sizeof(struct cam_req_mgr_timer), 64,
@@ -761,19 +793,28 @@ static int cam_req_mgr_component_master_bind(struct device *dev)
 				g_cam_req_mgr_timer_cachep->name);
 	}
 
-	g_dev.state = true;
-
-	CAM_DBG(CAM_CRM, "Binding all slave components");
+	CAM_INFO(CAM_CRM, "All probes done, binding slave components");
 	rc = component_bind_all(dev, NULL);
 	if (rc) {
 		CAM_ERR(CAM_CRM,
 			"Error in binding all components rc: %d, Camera initialization failed!",
 			rc);
-		goto req_mgr_core_fail;
+		goto req_mgr_device_deinit;
+	}
+
+	rc = cam_dev_mgr_create_subdev_nodes();
+	if (rc) {
+		CAM_ERR(CAM_CRM, "failed in creating devices for subdevs %d",
+			rc);
+		goto req_mgr_device_deinit;
 	}
 
+	CAM_INFO(CAM_CRM, "All camera components bound successfully");
+
 	return rc;
 
+req_mgr_device_deinit:
+	cam_req_mgr_core_device_deinit();
 req_mgr_core_fail:
 	cam_req_mgr_util_deinit();
 req_mgr_util_fail:
@@ -784,6 +825,7 @@ video_setup_fail:
 	cam_media_device_cleanup();
 media_setup_fail:
 	cam_v4l2_device_cleanup();
+	g_dev.state = false;
 	return rc;
 }
 
@@ -800,6 +842,7 @@ static void cam_req_mgr_component_master_unbind(struct device *dev)
 	cam_v4l2_device_cleanup();
 	mutex_destroy(&g_dev.dev_lock);
 	g_dev.state = false;
+	g_dev.subdev_nodes_created = false;
 }
 
 static const struct component_master_ops cam_req_mgr_component_master_ops = {

+ 2 - 0
drivers/cam_req_mgr/cam_req_mgr_dev.h

@@ -12,6 +12,7 @@
  *
  * @video: pointer to struct video device.
  * @v4l2_dev: pointer to struct v4l2 device.
+ * @subdev_nodes_created: all subdev nodes are created for this device
  * @count: number of subdevices registered.
  * @dev_lock: lock for the subdevice count.
  * @state: state of the root device.
@@ -23,6 +24,7 @@
 struct cam_req_mgr_device {
 	struct video_device *video;
 	struct v4l2_device *v4l2_dev;
+	bool subdev_nodes_created;
 	int count;
 	struct mutex dev_lock;
 	bool state;

+ 0 - 2
drivers/cam_req_mgr/cam_subdev.h

@@ -34,7 +34,6 @@
  * @ent_function:          Media entity function type. Can be:
  *                             %CAM_IFE_DEVICE_TYPE - identifies as IFE device.
  *                             %CAM_ICP_DEVICE_TYPE - identifies as ICP device.
- * @subdev_node_created:   Enabled sub-device
  *
  * Each instance of a subdev driver should create this struct, either
  * stand-alone or embedded in a larger struct. This structure should be
@@ -50,7 +49,6 @@ struct cam_subdev {
 	u32                                    sd_flags;
 	void                                  *token;
 	u32                                    ent_function;
-	bool                                   subdev_node_created;
 };
 
 /**

+ 5 - 0
drivers/cam_sensor_module/cam_cci/cam_cci_dev.c

@@ -452,6 +452,7 @@ cci_no_resource:
 static void cam_cci_component_unbind(struct device *dev,
 	struct device *master_dev, void *data)
 {
+	int rc = 0;
 	struct platform_device *pdev = to_platform_device(dev);
 
 	struct v4l2_subdev *subdev = platform_get_drvdata(pdev);
@@ -460,6 +461,10 @@ static void cam_cci_component_unbind(struct device *dev,
 
 	cam_cpas_unregister_client(cci_dev->cpas_handle);
 	cam_cci_soc_remove(pdev, cci_dev);
+	rc = cam_unregister_subdev(&(cci_dev->v4l2_dev_str));
+	if (rc < 0)
+		CAM_ERR(CAM_CCI, "Fail with cam_unregister_subdev");
+
 	devm_kfree(&pdev->dev, cci_dev);
 }
 

+ 1 - 1
drivers/cam_sensor_module/cam_eeprom/cam_eeprom_dev.c

@@ -498,7 +498,7 @@ static void cam_eeprom_component_unbind(struct device *dev,
 		return;
 	}
 
-	CAM_INFO(CAM_EEPROM, "Platform driver remove invoked");
+	CAM_DBG(CAM_EEPROM, "Component unbind called for: %s", pdev->name);
 	soc_info = &e_ctrl->soc_info;
 
 	for (i = 0; i < soc_info->num_clk; i++)

+ 1 - 1
drivers/cam_sensor_module/cam_sensor/cam_sensor_dev.c

@@ -295,7 +295,7 @@ static void cam_sensor_component_unbind(struct device *dev,
 		return;
 	}
 
-	CAM_INFO(CAM_SENSOR, "platform remove invoked");
+	CAM_DBG(CAM_SENSOR, "Component unbind called for: %s", pdev->name);
 	mutex_lock(&(s_ctrl->cam_sensor_mutex));
 	cam_sensor_shutdown(s_ctrl);
 	mutex_unlock(&(s_ctrl->cam_sensor_mutex));

+ 3 - 1
drivers/cam_sync/cam_sync.c

@@ -1052,7 +1052,7 @@ static int cam_sync_component_bind(struct device *dev,
 
 	strlcpy(sync_dev->vdev->name, CAM_SYNC_NAME,
 				sizeof(sync_dev->vdev->name));
-	sync_dev->vdev->release  = video_device_release;
+	sync_dev->vdev->release  = video_device_release_empty;
 	sync_dev->vdev->fops     = &cam_sync_v4l2_fops;
 	sync_dev->vdev->ioctl_ops = &g_cam_sync_ioctl_ops;
 	sync_dev->vdev->minor     = -1;
@@ -1102,6 +1102,7 @@ v4l2_fail:
 register_fail:
 	cam_sync_media_controller_cleanup(sync_dev);
 mcinit_fail:
+	video_unregister_device(sync_dev->vdev);
 	video_device_release(sync_dev->vdev);
 vdev_fail:
 	mutex_destroy(&sync_dev->table_lock);
@@ -1116,6 +1117,7 @@ static void cam_sync_component_unbind(struct device *dev,
 
 	v4l2_device_unregister(sync_dev->vdev->v4l2_dev);
 	cam_sync_media_controller_cleanup(sync_dev);
+	video_unregister_device(sync_dev->vdev);
 	video_device_release(sync_dev->vdev);
 	debugfs_remove_recursive(sync_dev->dentry);
 	sync_dev->dentry = NULL;

+ 2 - 2
drivers/camera_main.c

@@ -86,7 +86,7 @@ static const struct camera_submodule_component camera_isp[] = {
 };
 
 static const struct camera_submodule_component camera_tfe[] = {
-#if IS_ENABLED(CONFIG_SPECTRA_TFE)
+#ifdef CONFIG_SPECTRA_TFE
 	{&cam_top_tpg_init_module, &cam_top_tpg_exit_module},
 	{&cam_tfe_init_module, &cam_tfe_exit_module},
 	{&cam_tfe_csid530_init_module, &cam_tfe_csid530_exit_module},
@@ -381,7 +381,7 @@ static int camera_init(void)
 		}
 	}
 
-	CAM_INFO(CAM_UTIL, "Spectra camera driver initialized!");
+	CAM_INFO(CAM_UTIL, "Spectra camera driver initcalls done");
 
 end_init:
 	return rc;