diff mbox series

[v3,11/12] media: staging: rkisp1: use the right variants of spin_lock

Message ID 20200922113402.12442-12-dafna.hirschfeld@collabora.com
State Accepted
Commit 1d5099152b0ad66b66848189c070e2fca349beaf
Headers show
Series [v3,01/12] media: staging: rkisp1: params: upon stream stop, iterate a local list to return the buffers | expand

Commit Message

Dafna Hirschfeld Sept. 22, 2020, 11:34 a.m. UTC 
When locking, use either 'spin_lock' or 'spin_lock_irq'
according to the context. There is nowhere need to
use 'spin_lock_irqsave'.
Outside of irq context, always use 'spin_lock_irq'
to be on the safe side.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/staging/media/rkisp1/rkisp1-capture.c | 15 ++++++--------
 drivers/staging/media/rkisp1/rkisp1-params.c  | 20 ++++++++-----------
 2 files changed, 14 insertions(+), 21 deletions(-)

Comments

Tomasz Figa Sept. 25, 2020, 8:51 p.m. UTC | #1 
Hi Dafna,

On Tue, Sep 22, 2020 at 01:34:01PM +0200, Dafna Hirschfeld wrote:
> When locking, use either 'spin_lock' or 'spin_lock_irq'

> according to the context. There is nowhere need to

> use 'spin_lock_irqsave'.

> Outside of irq context, always use 'spin_lock_irq'

> to be on the safe side.

> 

> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

> ---

>  drivers/staging/media/rkisp1/rkisp1-capture.c | 15 ++++++--------

>  drivers/staging/media/rkisp1/rkisp1-params.c  | 20 ++++++++-----------

>  2 files changed, 14 insertions(+), 21 deletions(-)

>


I'd prefer to keep the irqsave variant, as it doesn't hurt, is more
consistent (all the calls on the same spinlock use the same variant) and
makes it possible to refactor the code easier in the future.

Best regards,
Tomasz
diff mbox series

Patch

diff --git a/drivers/staging/media/rkisp1/rkisp1-capture.c b/drivers/staging/media/rkisp1/rkisp1-capture.c
index 1c762a369b63..012c0825a386 100644
--- a/drivers/staging/media/rkisp1/rkisp1-capture.c
+++ b/drivers/staging/media/rkisp1/rkisp1-capture.c
@@ -626,9 +626,8 @@  static void rkisp1_handle_buffer(struct rkisp1_capture *cap)
 {
 	struct rkisp1_isp *isp = &cap->rkisp1->isp;
 	struct rkisp1_buffer *curr_buf;
-	unsigned long flags;
 
-	spin_lock_irqsave(&cap->buf.lock, flags);
+	spin_lock(&cap->buf.lock);
 	curr_buf = cap->buf.curr;
 
 	if (curr_buf) {
@@ -641,7 +640,7 @@  static void rkisp1_handle_buffer(struct rkisp1_capture *cap)
 	}
 
 	rkisp1_set_next_buf(cap);
-	spin_unlock_irqrestore(&cap->buf.lock, flags);
+	spin_unlock(&cap->buf.lock);
 }
 
 void rkisp1_capture_isr(struct rkisp1_device *rkisp1)
@@ -716,7 +715,6 @@  static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
 		container_of(vbuf, struct rkisp1_buffer, vb);
 	struct rkisp1_capture *cap = vb->vb2_queue->drv_priv;
 	const struct v4l2_pix_format_mplane *pixm = &cap->pix.fmt;
-	unsigned long flags;
 	unsigned int i;
 
 	memset(ispbuf->buff_addr, 0, sizeof(ispbuf->buff_addr));
@@ -741,9 +739,9 @@  static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
 		swap(ispbuf->buff_addr[RKISP1_PLANE_CR],
 		     ispbuf->buff_addr[RKISP1_PLANE_CB]);
 
-	spin_lock_irqsave(&cap->buf.lock, flags);
+	spin_lock_irq(&cap->buf.lock);
 	list_add_tail(&ispbuf->queue, &cap->buf.queue);
-	spin_unlock_irqrestore(&cap->buf.lock, flags);
+	spin_unlock_irq(&cap->buf.lock);
 }
 
 static int rkisp1_vb2_buf_prepare(struct vb2_buffer *vb)
@@ -769,10 +767,9 @@  static int rkisp1_vb2_buf_prepare(struct vb2_buffer *vb)
 static void rkisp1_return_all_buffers(struct rkisp1_capture *cap,
 				      enum vb2_buffer_state state)
 {
-	unsigned long flags;
 	struct rkisp1_buffer *buf;
 
-	spin_lock_irqsave(&cap->buf.lock, flags);
+	spin_lock_irq(&cap->buf.lock);
 	if (cap->buf.curr) {
 		vb2_buffer_done(&cap->buf.curr->vb.vb2_buf, state);
 		cap->buf.curr = NULL;
@@ -787,7 +784,7 @@  static void rkisp1_return_all_buffers(struct rkisp1_capture *cap,
 		list_del(&buf->queue);
 		vb2_buffer_done(&buf->vb.vb2_buf, state);
 	}
-	spin_unlock_irqrestore(&cap->buf.lock, flags);
+	spin_unlock_irq(&cap->buf.lock);
 }
 
 /*
diff --git a/drivers/staging/media/rkisp1/rkisp1-params.c b/drivers/staging/media/rkisp1/rkisp1-params.c
index f0258b4258ed..986d293201e6 100644
--- a/drivers/staging/media/rkisp1/rkisp1-params.c
+++ b/drivers/staging/media/rkisp1/rkisp1-params.c
@@ -1300,16 +1300,15 @@  static void rkisp1_params_config_parameter(struct rkisp1_params *params)
 	else
 		rkisp1_csm_config(params, false);
 
-	spin_lock(&params->config_lock);
+	spin_lock_irq(&params->config_lock);
 
 	/* apply the first buffer if there is one already */
 	if (params->is_streaming)
 		rkisp1_params_apply_params_cfg(params, 0);
 
-	spin_unlock(&params->config_lock);
+	spin_unlock_irq(&params->config_lock);
 }
 
-/* Not called when the camera active, thus not isr protection. */
 void rkisp1_params_configure(struct rkisp1_params *params,
 			     enum rkisp1_fmt_raw_pat_type bayer_pat,
 			     enum v4l2_quantization quantization)
@@ -1442,12 +1441,11 @@  static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
 		container_of(vbuf, struct rkisp1_buffer, vb);
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct rkisp1_params *params = vq->drv_priv;
-	unsigned long flags;
 
 	params_buf->vaddr = vb2_plane_vaddr(vb, 0);
-	spin_lock_irqsave(&params->config_lock, flags);
+	spin_lock_irq(&params->config_lock);
 	list_add_tail(&params_buf->queue, &params->params);
-	spin_unlock_irqrestore(&params->config_lock, flags);
+	spin_unlock_irq(&params->config_lock);
 }
 
 static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
@@ -1465,7 +1463,6 @@  static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
 	struct rkisp1_params *params = vq->drv_priv;
 	struct rkisp1_buffer *buf;
 	struct list_head tmp_list;
-	unsigned long flags;
 
 	INIT_LIST_HEAD(&tmp_list);
 
@@ -1474,10 +1471,10 @@  static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
 	 * and then we can iterate it and call vb2_buffer_done
 	 * without holding the lock
 	 */
-	spin_lock_irqsave(&params->config_lock, flags);
+	spin_lock_irq(&params->config_lock);
 	params->is_streaming = false;
 	list_cut_position(&tmp_list, &params->params, params->params.prev);
-	spin_unlock_irqrestore(&params->config_lock, flags);
+	spin_unlock_irq(&params->config_lock);
 
 	list_for_each_entry(buf, &tmp_list, queue)
 		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
@@ -1487,11 +1484,10 @@  static int
 rkisp1_params_vb2_start_streaming(struct vb2_queue *queue, unsigned int count)
 {
 	struct rkisp1_params *params = queue->drv_priv;
-	unsigned long flags;
 
-	spin_lock_irqsave(&params->config_lock, flags);
+	spin_lock_irq(&params->config_lock);
 	params->is_streaming = true;
-	spin_unlock_irqrestore(&params->config_lock, flags);
+	spin_unlock_irq(&params->config_lock);
 
 	return 0;
 }