[5/6] media: staging: rkisp1: params: add '__must_hold' to rkisp1_params_apply_params_cfg

Commit Message

Dafna Hirschfeld Oct. 2, 2020, 6:42 p.m. UTC

The function rkisp1_params_apply_params_cfg must be under the
the lock of params->config_lock. Add the __must_hold annotation
to indicate it.

Signed-off-by: Dafna Hirschfeld <[e-mail]>
---
 drivers/staging/media/rkisp1/rkisp1-params.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Helen Koike Oct. 14, 2020, 8:27 p.m. UTC | #1

Hi Dafna,

On 10/2/20 3:42 PM, Dafna Hirschfeld wrote:
> The function rkisp1_params_apply_params_cfg must be under the
> the lock of params->config_lock. Add the __must_hold annotation
> to indicate it.
> 
> Signed-off-by: Dafna Hirschfeld <[e-mail]>
> ---
>  drivers/staging/media/rkisp1/rkisp1-params.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
> index 3afbc24ca05e..aa18a113245b 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-params.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-params.c
> @@ -1187,6 +1187,7 @@ static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
>  
>  static void rkisp1_params_apply_params_cfg(struct rkisp1_params *params,
>  					   unsigned int frame_sequence)
> +	__must_hold(&params->config_lock)
>  {
>  	struct rkisp1_params_cfg *new_params;
>  	struct rkisp1_buffer *cur_buf = NULL;
> 

Instead of adding __must_hold, why no to lock inside this function?
It seems there are only two places that call it, and they don't do anything else
in the critical section.

Regards,
Helen

Patch

diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index 3afbc24ca05e..aa18a113245b 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -1187,6 +1187,7 @@  static void rkisp1_isp_isr_meas_config(struct rkisp1_params *params,
 
 static void rkisp1_params_apply_params_cfg(struct rkisp1_params *params,
 					   unsigned int frame_sequence)
+	__must_hold(&params->config_lock)
 {
 	struct rkisp1_params_cfg *new_params;
 	struct rkisp1_buffer *cur_buf = NULL;