From: Hugues Fruchet <hugues.fruchet@st.com> To: Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@st.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, "Hans Verkuil" <hverkuil@xs4all.nl> Cc: <linux-media@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, Benjamin Gaignard <benjamin.gaignard@linaro.org>, Yannick Fertre <yannick.fertre@st.com>, Hugues Fruchet <hugues.fruchet@st.com> Subject: [PATCH v2] media: stm32-dcmi: fix lock scheme Date: Thu, 22 Feb 2018 10:49:14 +0100 [thread overview] Message-ID: <1519292954-19733-1-git-send-email-hugues.fruchet@st.com> (raw) Fix lock scheme leading to spurious freeze. Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> --- version 2: - dcmi_buf_queue() refactor to avoid to have "else" after "return" (warning detected by checkpatch.pl --strict -f stm32-dcmi.c) drivers/media/platform/stm32/stm32-dcmi.c | 57 +++++++++++++------------------ 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c index 2fd8bed..5de18ad 100644 --- a/drivers/media/platform/stm32/stm32-dcmi.c +++ b/drivers/media/platform/stm32/stm32-dcmi.c @@ -197,7 +197,7 @@ static void dcmi_dma_callback(void *param) struct dma_tx_state state; enum dma_status status; - spin_lock(&dcmi->irqlock); + spin_lock_irq(&dcmi->irqlock); /* Check DMA status */ status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state); @@ -239,7 +239,7 @@ static void dcmi_dma_callback(void *param) dcmi->errors_count++; dcmi->active = NULL; - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return; } @@ -248,13 +248,11 @@ static void dcmi_dma_callback(void *param) list_del_init(&dcmi->active->list); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on DMA complete\n", __func__); - - spin_unlock(&dcmi->irqlock); - return; - } + return; } break; @@ -263,7 +261,7 @@ static void dcmi_dma_callback(void *param) break; } - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); } static int dcmi_start_dma(struct stm32_dcmi *dcmi, @@ -360,7 +358,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) { struct stm32_dcmi *dcmi = arg; - spin_lock(&dcmi->irqlock); + spin_lock_irq(&dcmi->irqlock); /* Stop capture is required */ if (dcmi->state == STOPPING) { @@ -370,7 +368,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) complete(&dcmi->complete); - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return IRQ_HANDLED; } @@ -383,35 +381,34 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) __func__); dcmi->errors_count++; - dmaengine_terminate_all(dcmi->dma_chan); - dev_dbg(dcmi->dev, "Restarting capture after DCMI error\n"); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + dmaengine_terminate_all(dcmi->dma_chan); + + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n", __func__); - - spin_unlock(&dcmi->irqlock); - return IRQ_HANDLED; - } + return IRQ_HANDLED; } - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return IRQ_HANDLED; } static irqreturn_t dcmi_irq_callback(int irq, void *arg) { struct stm32_dcmi *dcmi = arg; + unsigned long flags; - spin_lock(&dcmi->irqlock); + spin_lock_irqsave(&dcmi->irqlock, flags); dcmi->misr = reg_read(dcmi->regs, DCMI_MIS); /* Clear interrupt */ reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR); - spin_unlock(&dcmi->irqlock); + spin_unlock_irqrestore(&dcmi->irqlock, flags); return IRQ_WAKE_THREAD; } @@ -490,9 +487,8 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) struct stm32_dcmi *dcmi = vb2_get_drv_priv(vb->vb2_queue); struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb); - unsigned long flags = 0; - spin_lock_irqsave(&dcmi->irqlock, flags); + spin_lock_irq(&dcmi->irqlock); if ((dcmi->state == RUNNING) && (!dcmi->active)) { dcmi->active = buf; @@ -500,19 +496,15 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n", buf->vb.vb2_buf.index); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n", __func__); - - spin_unlock_irqrestore(&dcmi->irqlock, flags); - return; - } } else { /* Enqueue to video buffers list */ list_add_tail(&buf->list, &dcmi->buffers); + spin_unlock_irq(&dcmi->irqlock); } - - spin_unlock_irqrestore(&dcmi->irqlock, flags); } static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) @@ -598,20 +590,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) dev_dbg(dcmi->dev, "Start streaming, starting capture\n"); + spin_unlock_irq(&dcmi->irqlock); ret = dcmi_start_capture(dcmi); if (ret) { dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", __func__); - - spin_unlock_irq(&dcmi->irqlock); goto err_subdev_streamoff; } /* Enable interruptions */ reg_set(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR); - spin_unlock_irq(&dcmi->irqlock); - return 0; err_subdev_streamoff: @@ -654,7 +643,9 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", __func__, ret); + spin_lock_irq(&dcmi->irqlock); dcmi->state = STOPPING; + spin_unlock_irq(&dcmi->irqlock); timeout = wait_for_completion_interruptible_timeout(&dcmi->complete, time_ms); -- 1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: hugues.fruchet@st.com (Hugues Fruchet) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2] media: stm32-dcmi: fix lock scheme Date: Thu, 22 Feb 2018 10:49:14 +0100 [thread overview] Message-ID: <1519292954-19733-1-git-send-email-hugues.fruchet@st.com> (raw) Fix lock scheme leading to spurious freeze. Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com> --- version 2: - dcmi_buf_queue() refactor to avoid to have "else" after "return" (warning detected by checkpatch.pl --strict -f stm32-dcmi.c) drivers/media/platform/stm32/stm32-dcmi.c | 57 +++++++++++++------------------ 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c index 2fd8bed..5de18ad 100644 --- a/drivers/media/platform/stm32/stm32-dcmi.c +++ b/drivers/media/platform/stm32/stm32-dcmi.c @@ -197,7 +197,7 @@ static void dcmi_dma_callback(void *param) struct dma_tx_state state; enum dma_status status; - spin_lock(&dcmi->irqlock); + spin_lock_irq(&dcmi->irqlock); /* Check DMA status */ status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state); @@ -239,7 +239,7 @@ static void dcmi_dma_callback(void *param) dcmi->errors_count++; dcmi->active = NULL; - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return; } @@ -248,13 +248,11 @@ static void dcmi_dma_callback(void *param) list_del_init(&dcmi->active->list); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on DMA complete\n", __func__); - - spin_unlock(&dcmi->irqlock); - return; - } + return; } break; @@ -263,7 +261,7 @@ static void dcmi_dma_callback(void *param) break; } - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); } static int dcmi_start_dma(struct stm32_dcmi *dcmi, @@ -360,7 +358,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) { struct stm32_dcmi *dcmi = arg; - spin_lock(&dcmi->irqlock); + spin_lock_irq(&dcmi->irqlock); /* Stop capture is required */ if (dcmi->state == STOPPING) { @@ -370,7 +368,7 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) complete(&dcmi->complete); - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return IRQ_HANDLED; } @@ -383,35 +381,34 @@ static irqreturn_t dcmi_irq_thread(int irq, void *arg) __func__); dcmi->errors_count++; - dmaengine_terminate_all(dcmi->dma_chan); - dev_dbg(dcmi->dev, "Restarting capture after DCMI error\n"); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + dmaengine_terminate_all(dcmi->dma_chan); + + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n", __func__); - - spin_unlock(&dcmi->irqlock); - return IRQ_HANDLED; - } + return IRQ_HANDLED; } - spin_unlock(&dcmi->irqlock); + spin_unlock_irq(&dcmi->irqlock); return IRQ_HANDLED; } static irqreturn_t dcmi_irq_callback(int irq, void *arg) { struct stm32_dcmi *dcmi = arg; + unsigned long flags; - spin_lock(&dcmi->irqlock); + spin_lock_irqsave(&dcmi->irqlock, flags); dcmi->misr = reg_read(dcmi->regs, DCMI_MIS); /* Clear interrupt */ reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR); - spin_unlock(&dcmi->irqlock); + spin_unlock_irqrestore(&dcmi->irqlock, flags); return IRQ_WAKE_THREAD; } @@ -490,9 +487,8 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) struct stm32_dcmi *dcmi = vb2_get_drv_priv(vb->vb2_queue); struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb); - unsigned long flags = 0; - spin_lock_irqsave(&dcmi->irqlock, flags); + spin_lock_irq(&dcmi->irqlock); if ((dcmi->state == RUNNING) && (!dcmi->active)) { dcmi->active = buf; @@ -500,19 +496,15 @@ static void dcmi_buf_queue(struct vb2_buffer *vb) dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n", buf->vb.vb2_buf.index); - if (dcmi_start_capture(dcmi)) { + spin_unlock_irq(&dcmi->irqlock); + if (dcmi_start_capture(dcmi)) dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n", __func__); - - spin_unlock_irqrestore(&dcmi->irqlock, flags); - return; - } } else { /* Enqueue to video buffers list */ list_add_tail(&buf->list, &dcmi->buffers); + spin_unlock_irq(&dcmi->irqlock); } - - spin_unlock_irqrestore(&dcmi->irqlock, flags); } static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) @@ -598,20 +590,17 @@ static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count) dev_dbg(dcmi->dev, "Start streaming, starting capture\n"); + spin_unlock_irq(&dcmi->irqlock); ret = dcmi_start_capture(dcmi); if (ret) { dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture\n", __func__); - - spin_unlock_irq(&dcmi->irqlock); goto err_subdev_streamoff; } /* Enable interruptions */ reg_set(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR); - spin_unlock_irq(&dcmi->irqlock); - return 0; err_subdev_streamoff: @@ -654,7 +643,9 @@ static void dcmi_stop_streaming(struct vb2_queue *vq) dev_err(dcmi->dev, "%s: Failed to stop streaming, subdev streamoff error (%d)\n", __func__, ret); + spin_lock_irq(&dcmi->irqlock); dcmi->state = STOPPING; + spin_unlock_irq(&dcmi->irqlock); timeout = wait_for_completion_interruptible_timeout(&dcmi->complete, time_ms); -- 1.9.1
next reply other threads:[~2018-02-22 9:51 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-02-22 9:49 Hugues Fruchet [this message] 2018-02-22 9:49 ` [PATCH v2] media: stm32-dcmi: fix lock scheme Hugues Fruchet 2018-02-26 13:56 ` Hans Verkuil 2018-02-26 13:56 ` Hans Verkuil 2018-02-28 17:04 ` Hugues FRUCHET 2018-02-28 17:04 ` Hugues FRUCHET
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1519292954-19733-1-git-send-email-hugues.fruchet@st.com \ --to=hugues.fruchet@st.com \ --cc=alexandre.torgue@st.com \ --cc=benjamin.gaignard@linaro.org \ --cc=hverkuil@xs4all.nl \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=mcoquelin.stm32@gmail.com \ --cc=yannick.fertre@st.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.