From d87d172bde615073260d3885377b243250cf25cd Mon Sep 17 00:00:00 2001 From: Anil Kumar Kanakanti Date: Sat, 12 Sep 2020 00:11:53 +0530 Subject: [PATCH] msm: camera: cci: Fix logic to update cci clk freq When multiple frequency slaves running on a same I2C bus, then there is a chance of overriding I2C bus frequency even if another I2C operation is running. This could lead to CCI timeout at driver level. Updated synchronization logic, to properly update I2C clock frequency, only when no other I2C operation running. CRs-Fixed: 2815310 Change-Id: Ia341d7cda118497bf1acea8ea59f7f03124f31c3 Signed-off-by: Anil Kumar Kanakanti --- .../cam_sensor_module/cam_cci/cam_cci_core.c | 155 ++++++------------ .../cam_sensor_module/cam_cci/cam_cci_dev.h | 3 +- .../cam_sensor_module/cam_cci/cam_cci_soc.c | 4 +- 3 files changed, 54 insertions(+), 108 deletions(-) diff --git a/drivers/cam_sensor_module/cam_cci/cam_cci_core.c b/drivers/cam_sensor_module/cam_cci/cam_cci_core.c index 80ce7c2e8ff3..0d6348108c18 100644 --- a/drivers/cam_sensor_module/cam_cci/cam_cci_core.c +++ b/drivers/cam_sensor_module/cam_cci/cam_cci_core.c @@ -609,19 +609,42 @@ static int32_t cam_cci_set_clk_param(struct cci_device *cci_dev, struct cam_cci_clk_params_t *clk_params = NULL; enum cci_i2c_master_t master = c_ctrl->cci_info->cci_i2c_master; enum i2c_freq_mode i2c_freq_mode = c_ctrl->cci_info->i2c_freq_mode; - struct cam_hw_soc_info *soc_info = - &cci_dev->soc_info; - void __iomem *base = soc_info->reg_map[0].mem_base; + void __iomem *base = cci_dev->soc_info.reg_map[0].mem_base; + struct cam_cci_master_info *cci_master = + &cci_dev->cci_master_info[master]; if ((i2c_freq_mode >= I2C_MAX_MODES) || (i2c_freq_mode < 0)) { CAM_ERR(CAM_CCI, "invalid i2c_freq_mode = %d", i2c_freq_mode); return -EINVAL; } + /* + * If no change in i2c freq, then acquire semaphore only for the first + * i2c transaction to indicate I2C transaction is in progress, else + * always try to acquire semaphore, to make sure that no other I2C + * transaction is in progress. + */ + mutex_lock(&cci_master->mutex); + if (i2c_freq_mode == cci_dev->i2c_freq_mode[master]) { + CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d", master, + i2c_freq_mode); + spin_lock(&cci_master->freq_cnt_lock); + if (cci_master->freq_ref_cnt == 0) + down(&cci_master->master_sem); + cci_master->freq_ref_cnt++; + spin_unlock(&cci_master->freq_cnt_lock); + mutex_unlock(&cci_master->mutex); + return 0; + } + CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", + master, cci_dev->i2c_freq_mode[master], i2c_freq_mode); + down(&cci_master->master_sem); + + spin_lock(&cci_master->freq_cnt_lock); + cci_master->freq_ref_cnt++; + spin_unlock(&cci_master->freq_cnt_lock); clk_params = &cci_dev->cci_clk_params[i2c_freq_mode]; - if (cci_dev->i2c_freq_mode[master] == i2c_freq_mode) - return 0; if (master == MASTER_0) { cam_io_w_mb(clk_params->hw_thigh << 16 | clk_params->hw_tlow, @@ -655,6 +678,7 @@ static int32_t cam_cci_set_clk_param(struct cci_device *cci_dev, } cci_dev->i2c_freq_mode[master] = i2c_freq_mode; + mutex_unlock(&cci_master->mutex); return 0; } @@ -938,42 +962,19 @@ static int32_t cam_cci_burst_read(struct v4l2_subdev *sd, return -EINVAL; } - soc_info = &cci_dev->soc_info; - base = soc_info->reg_map[0].mem_base; - - mutex_lock(&cci_dev->cci_master_info[master].mutex); - if (cci_dev->cci_master_info[master].is_first_req) { - cci_dev->cci_master_info[master].is_first_req = false; - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else if (c_ctrl->cci_info->i2c_freq_mode - != cci_dev->i2c_freq_mode[master]) { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - cci_dev->cci_master_info[master].freq_ref_cnt++; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); - } - /* Set the I2C Frequency */ rc = cam_cci_set_clk_param(cci_dev, c_ctrl); if (rc < 0) { CAM_ERR(CAM_CCI, "cam_cci_set_clk_param failed rc = %d", rc); - mutex_unlock(&cci_dev->cci_master_info[master].mutex); - goto rel_master; + return rc; } - mutex_unlock(&cci_dev->cci_master_info[master].mutex); mutex_lock(&cci_dev->cci_master_info[master].mutex_q[queue]); reinit_completion(&cci_dev->cci_master_info[master].report_q[queue]); + + soc_info = &cci_dev->soc_info; + base = soc_info->reg_map[0].mem_base; + /* * Call validate queue to make sure queue is empty before starting. * If this call fails, don't proceed with i2c_read call. This is to @@ -1198,13 +1199,11 @@ static int32_t cam_cci_burst_read(struct v4l2_subdev *sd, rel_mutex_q: mutex_unlock(&cci_dev->cci_master_info[master].mutex_q[queue]); -rel_master: - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - if (cci_dev->cci_master_info[master].freq_ref_cnt == 0) + + spin_lock(&cci_dev->cci_master_info[master].freq_cnt_lock); + if (--cci_dev->cci_master_info[master].freq_ref_cnt == 0) up(&cci_dev->cci_master_info[master].master_sem); - else - cci_dev->cci_master_info[master].freq_ref_cnt--; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); + spin_unlock(&cci_dev->cci_master_info[master].freq_cnt_lock); return rc; } @@ -1234,42 +1233,19 @@ static int32_t cam_cci_read(struct v4l2_subdev *sd, return -EINVAL; } - soc_info = &cci_dev->soc_info; - base = soc_info->reg_map[0].mem_base; - - mutex_lock(&cci_dev->cci_master_info[master].mutex); - if (cci_dev->cci_master_info[master].is_first_req) { - cci_dev->cci_master_info[master].is_first_req = false; - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else if (c_ctrl->cci_info->i2c_freq_mode - != cci_dev->i2c_freq_mode[master]) { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - cci_dev->cci_master_info[master].freq_ref_cnt++; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); - } - /* Set the I2C Frequency */ rc = cam_cci_set_clk_param(cci_dev, c_ctrl); if (rc < 0) { - mutex_unlock(&cci_dev->cci_master_info[master].mutex); CAM_ERR(CAM_CCI, "cam_cci_set_clk_param failed rc = %d", rc); - goto rel_master; + return rc; } - mutex_unlock(&cci_dev->cci_master_info[master].mutex); mutex_lock(&cci_dev->cci_master_info[master].mutex_q[queue]); reinit_completion(&cci_dev->cci_master_info[master].report_q[queue]); + + soc_info = &cci_dev->soc_info; + base = soc_info->reg_map[0].mem_base; + /* * Call validate queue to make sure queue is empty before starting. * If this call fails, don't proceed with i2c_read call. This is to @@ -1425,13 +1401,11 @@ static int32_t cam_cci_read(struct v4l2_subdev *sd, } rel_mutex_q: mutex_unlock(&cci_dev->cci_master_info[master].mutex_q[queue]); -rel_master: - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - if (cci_dev->cci_master_info[master].freq_ref_cnt == 0) + + spin_lock(&cci_dev->cci_master_info[master].freq_cnt_lock); + if (--cci_dev->cci_master_info[master].freq_ref_cnt == 0) up(&cci_dev->cci_master_info[master].master_sem); - else - cci_dev->cci_master_info[master].freq_ref_cnt--; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); + spin_unlock(&cci_dev->cci_master_info[master].freq_cnt_lock); return rc; } @@ -1455,37 +1429,12 @@ static int32_t cam_cci_i2c_write(struct v4l2_subdev *sd, c_ctrl->cci_info->sid, c_ctrl->cci_info->retries, c_ctrl->cci_info->id_map); - mutex_lock(&cci_dev->cci_master_info[master].mutex); - if (cci_dev->cci_master_info[master].is_first_req) { - cci_dev->cci_master_info[master].is_first_req = false; - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else if (c_ctrl->cci_info->i2c_freq_mode - != cci_dev->i2c_freq_mode[master]) { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - down(&cci_dev->cci_master_info[master].master_sem); - } else { - CAM_DBG(CAM_CCI, "Master: %d, curr_freq: %d, req_freq: %d", - master, cci_dev->i2c_freq_mode[master], - c_ctrl->cci_info->i2c_freq_mode); - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - cci_dev->cci_master_info[master].freq_ref_cnt++; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); - } - /* Set the I2C Frequency */ rc = cam_cci_set_clk_param(cci_dev, c_ctrl); if (rc < 0) { CAM_ERR(CAM_CCI, "cam_cci_set_clk_param failed rc = %d", rc); - mutex_unlock(&cci_dev->cci_master_info[master].mutex); - goto ERROR; + return rc; } - mutex_unlock(&cci_dev->cci_master_info[master].mutex); - reinit_completion(&cci_dev->cci_master_info[master].report_q[queue]); /* * Call validate queue to make sure queue is empty before starting. @@ -1515,12 +1464,10 @@ static int32_t cam_cci_i2c_write(struct v4l2_subdev *sd, } ERROR: - spin_lock(&cci_dev->cci_master_info[master].freq_cnt); - if (cci_dev->cci_master_info[master].freq_ref_cnt == 0) + spin_lock(&cci_dev->cci_master_info[master].freq_cnt_lock); + if (--cci_dev->cci_master_info[master].freq_ref_cnt == 0) up(&cci_dev->cci_master_info[master].master_sem); - else - cci_dev->cci_master_info[master].freq_ref_cnt--; - spin_unlock(&cci_dev->cci_master_info[master].freq_cnt); + spin_unlock(&cci_dev->cci_master_info[master].freq_cnt_lock); return rc; } diff --git a/drivers/cam_sensor_module/cam_cci/cam_cci_dev.h b/drivers/cam_sensor_module/cam_cci/cam_cci_dev.h index 3284c64ad3b1..a72f4d836683 100644 --- a/drivers/cam_sensor_module/cam_cci/cam_cci_dev.h +++ b/drivers/cam_sensor_module/cam_cci/cam_cci_dev.h @@ -132,9 +132,8 @@ struct cam_cci_master_info { struct completion report_q[NUM_QUEUES]; atomic_t done_pending[NUM_QUEUES]; spinlock_t lock_q[NUM_QUEUES]; - spinlock_t freq_cnt; struct semaphore master_sem; - bool is_first_req; + spinlock_t freq_cnt_lock; uint16_t freq_ref_cnt; bool is_initilized; }; diff --git a/drivers/cam_sensor_module/cam_cci/cam_cci_soc.c b/drivers/cam_sensor_module/cam_cci/cam_cci_soc.c index fde91ad13459..39433a3f43e1 100644 --- a/drivers/cam_sensor_module/cam_cci/cam_cci_soc.c +++ b/drivers/cam_sensor_module/cam_cci/cam_cci_soc.c @@ -219,11 +219,11 @@ static void cam_cci_init_cci_params(struct cci_device *new_cci_dev) for (i = 0; i < MASTER_MAX; i++) { new_cci_dev->cci_master_info[i].status = 0; - new_cci_dev->cci_master_info[i].is_first_req = true; + new_cci_dev->cci_master_info[i].freq_ref_cnt = 0; new_cci_dev->cci_master_info[i].is_initilized = false; mutex_init(&new_cci_dev->cci_master_info[i].mutex); sema_init(&new_cci_dev->cci_master_info[i].master_sem, 1); - spin_lock_init(&new_cci_dev->cci_master_info[i].freq_cnt); + spin_lock_init(&new_cci_dev->cci_master_info[i].freq_cnt_lock); init_completion( &new_cci_dev->cci_master_info[i].reset_complete); init_completion(