From 0a649b30634a396bd74462414877cbe6d564c1dd Mon Sep 17 00:00:00 2001 From: Mukund Madhusudan Atre Date: Mon, 17 Feb 2020 00:29:44 -0800 Subject: [PATCH] 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 --- drivers/cam_icp/cam_icp_subdev.c | 2 +- drivers/cam_isp/cam_isp_dev.c | 2 +- drivers/cam_req_mgr/cam_req_mgr_dev.c | 103 +++++++++++++----- drivers/cam_req_mgr/cam_req_mgr_dev.h | 2 + drivers/cam_req_mgr/cam_subdev.h | 2 - .../cam_sensor_module/cam_cci/cam_cci_dev.c | 5 + .../cam_eeprom/cam_eeprom_dev.c | 2 +- .../cam_sensor/cam_sensor_dev.c | 2 +- drivers/cam_sync/cam_sync.c | 4 +- drivers/camera_main.c | 4 +- 10 files changed, 89 insertions(+), 39 deletions(-) diff --git a/drivers/cam_icp/cam_icp_subdev.c b/drivers/cam_icp/cam_icp_subdev.c index d4c7e025afbd..801c88591436 100644 --- a/drivers/cam_icp/cam_icp_subdev.c +++ b/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; diff --git a/drivers/cam_isp/cam_isp_dev.c b/drivers/cam_isp/cam_isp_dev.c index f5bbf89d292b..322ee3d85449 100644 --- a/drivers/cam_isp/cam_isp_dev.c +++ b/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: diff --git a/drivers/cam_req_mgr/cam_req_mgr_dev.c b/drivers/cam_req_mgr/cam_req_mgr_dev.c index 0ac8178b47f8..038d3c28e49f 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_dev.c +++ b/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 = { diff --git a/drivers/cam_req_mgr/cam_req_mgr_dev.h b/drivers/cam_req_mgr/cam_req_mgr_dev.h index 464720efb0a2..9b241e6f0a46 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_dev.h +++ b/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; diff --git a/drivers/cam_req_mgr/cam_subdev.h b/drivers/cam_req_mgr/cam_subdev.h index 08d02a1b0fab..1b2d146fa2fe 100644 --- a/drivers/cam_req_mgr/cam_subdev.h +++ b/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; }; /** diff --git a/drivers/cam_sensor_module/cam_cci/cam_cci_dev.c b/drivers/cam_sensor_module/cam_cci/cam_cci_dev.c index cd937016298c..804bee34c2df 100644 --- a/drivers/cam_sensor_module/cam_cci/cam_cci_dev.c +++ b/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); } diff --git a/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_dev.c b/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_dev.c index a0d4b5a45f30..486632399e60 100644 --- a/drivers/cam_sensor_module/cam_eeprom/cam_eeprom_dev.c +++ b/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++) diff --git a/drivers/cam_sensor_module/cam_sensor/cam_sensor_dev.c b/drivers/cam_sensor_module/cam_sensor/cam_sensor_dev.c index 564cae097934..9d85f3f9c10f 100644 --- a/drivers/cam_sensor_module/cam_sensor/cam_sensor_dev.c +++ b/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)); diff --git a/drivers/cam_sync/cam_sync.c b/drivers/cam_sync/cam_sync.c index d5529c2055e9..8f7d9d838970 100644 --- a/drivers/cam_sync/cam_sync.c +++ b/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; diff --git a/drivers/camera_main.c b/drivers/camera_main.c index 35bd853f4412..f90aa0652c59 100644 --- a/drivers/camera_main.c +++ b/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;