[PATCH 1/7] media: atmel: atmel-isc: add safe checks and fixed wrong ISC state in error case

From: Eugen.Hristev
Date: Tue Apr 09 2019 - 07:07:34 EST


From: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>

This adds safety checks on some scenarios:
- start streaming but streaming is already started
- start streaming but no buffer in the dma queue
- spin lock is not released in error scenario
- no frame is configured but dma is requested to start
- configure ISC may have been called without need, before checking if buffer is
ok.

Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
---
drivers/media/platform/atmel/atmel-isc.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index a10db16..3c19761 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -722,6 +722,11 @@ static void isc_start_dma(struct isc_device *isc)
u32 dctrl_dview;
dma_addr_t addr0;

+ if (!isc->cur_frm) {
+ v4l2_err(&isc->v4l2_dev, "Video buffer not available\n");
+ return;
+ }
+
addr0 = vb2_dma_contig_plane_dma_addr(&isc->cur_frm->vb.vb2_buf, 0);
regmap_write(regmap, ISC_DAD0, addr0);

@@ -886,6 +891,9 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
unsigned long flags;
int ret;

+ if (vb2_is_streaming(&isc->vb2_vidq))
+ return -EBUSY;
+
/* Enable stream on the sub device */
ret = v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 1);
if (ret && ret != -ENOIOCTLCMD) {
@@ -896,6 +904,20 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)

pm_runtime_get_sync(isc->dev);

+ spin_lock_irqsave(&isc->dma_queue_lock, flags);
+
+ isc->sequence = 0;
+ isc->stop = false;
+ reinit_completion(&isc->comp);
+
+ if (list_empty(&isc->dma_queue)) {
+ v4l2_err(&isc->v4l2_dev, "dma queue empty\n");
+ ret = -EINVAL;
+ goto err_configure_unlock;
+ }
+
+ spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
+
ret = isc_configure(isc);
if (unlikely(ret))
goto err_configure;
@@ -905,10 +927,6 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)

spin_lock_irqsave(&isc->dma_queue_lock, flags);

- isc->sequence = 0;
- isc->stop = false;
- reinit_completion(&isc->comp);
-
isc->cur_frm = list_first_entry(&isc->dma_queue,
struct isc_buffer, list);
list_del(&isc->cur_frm->list);
@@ -919,8 +937,11 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)

return 0;

+err_configure_unlock:
+ spin_unlock_irqrestore(&isc->dma_queue_lock, flags);
+
err_configure:
- pm_runtime_put_sync(isc->dev);
+ isc->stop = true;

v4l2_subdev_call(isc->current_subdev->sd, video, s_stream, 0);

@@ -931,6 +952,7 @@ static int isc_start_streaming(struct vb2_queue *vq, unsigned int count)
INIT_LIST_HEAD(&isc->dma_queue);
spin_unlock_irqrestore(&isc->dma_queue_lock, flags);

+ pm_runtime_put_sync(isc->dev);
return ret;
}

--
2.7.4