diff mbox series

[v1,5/7] media: ipu3-cio2: Replace infinite loop by one with clear exit condition

Message ID 20200814163017.35001-5-andriy.shevchenko@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/7] media: ipu3-cio2: Simplify cleanup code | expand

Commit Message

Andy Shevchenko Aug. 14, 2020, 4:30 p.m. UTC 
Refactor cio2_buffer_done() to get rid of infinite loop by replacing it by one
with clear exit condition. This change also allows to check for an error ahead.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

Comments

Bingbu Cao Aug. 17, 2020, 10:21 a.m. UTC | #1 
On 8/15/20 12:30 AM, Andy Shevchenko wrote:
> Refactor cio2_buffer_done() to get rid of infinite loop by replacing it by one
> with clear exit condition. This change also allows to check for an error ahead.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu3/ipu3-cio2.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> index eee7f841050d..f4175dc1501a 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
> @@ -541,7 +541,7 @@ static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
>  {
>  	struct device *dev = &cio2->pci_dev->dev;
>  	struct cio2_queue *q = cio2->cur_queue;
> -	int buffers_found = 0;
> +	struct cio2_fbpt_entry *const entry;
No 'const' here.

>  	u64 ns = ktime_get_ns();
>  
>  	if (dma_chan >= CIO2_QUEUES) {
> @@ -549,15 +549,18 @@ static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
>  		return;
>  	}
>  
> +	entry = &q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
> +	if (entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID) {
> +		dev_warn(&cio2->pci_dev->dev,
> +			 "no ready buffers found on DMA channel %u\n",
> +			 dma_chan);
> +		return;
> +	}
> +
>  	/* Find out which buffer(s) are ready */
>  	do {
> -		struct cio2_fbpt_entry *const entry =
> -			&q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
>  		struct cio2_buffer *b;
>  
> -		if (entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID)
> -			break;
> -
>  		b = q->bufs[q->bufs_first];
>  		if (b) {
>  			unsigned int bytes = entry[1].second_entry.num_of_bytes;
> @@ -579,13 +582,8 @@ static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
>  		atomic_inc(&q->frame_sequence);
>  		cio2_fbpt_entry_init_dummy(cio2, entry);
>  		q->bufs_first = (q->bufs_first + 1) % CIO2_MAX_BUFFERS;
> -		buffers_found++;
> -	} while (1);
> -
> -	if (buffers_found == 0)
> -		dev_warn(&cio2->pci_dev->dev,
> -			 "no ready buffers found on DMA channel %u\n",
> -			 dma_chan);
> +		entry = &q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
> +	} while (!(entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID));
>  }
>  
>  static void cio2_queue_event_sof(struct cio2_device *cio2, struct cio2_queue *q)
>
Andy Shevchenko Aug. 17, 2020, 11:47 a.m. UTC | #2 
On Mon, Aug 17, 2020 at 06:21:23PM +0800, Bingbu Cao wrote:
> On 8/15/20 12:30 AM, Andy Shevchenko wrote:
> > Refactor cio2_buffer_done() to get rid of infinite loop by replacing it by one
> > with clear exit condition. This change also allows to check for an error ahead.

...

> > -	int buffers_found = 0;
> > +	struct cio2_fbpt_entry *const entry;
> No 'const' here.

Sure, will not be in v2.
diff mbox series

Patch

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
index eee7f841050d..f4175dc1501a 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -541,7 +541,7 @@  static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
 {
 	struct device *dev = &cio2->pci_dev->dev;
 	struct cio2_queue *q = cio2->cur_queue;
-	int buffers_found = 0;
+	struct cio2_fbpt_entry *const entry;
 	u64 ns = ktime_get_ns();
 
 	if (dma_chan >= CIO2_QUEUES) {
@@ -549,15 +549,18 @@  static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
 		return;
 	}
 
+	entry = &q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
+	if (entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID) {
+		dev_warn(&cio2->pci_dev->dev,
+			 "no ready buffers found on DMA channel %u\n",
+			 dma_chan);
+		return;
+	}
+
 	/* Find out which buffer(s) are ready */
 	do {
-		struct cio2_fbpt_entry *const entry =
-			&q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
 		struct cio2_buffer *b;
 
-		if (entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID)
-			break;
-
 		b = q->bufs[q->bufs_first];
 		if (b) {
 			unsigned int bytes = entry[1].second_entry.num_of_bytes;
@@ -579,13 +582,8 @@  static void cio2_buffer_done(struct cio2_device *cio2, unsigned int dma_chan)
 		atomic_inc(&q->frame_sequence);
 		cio2_fbpt_entry_init_dummy(cio2, entry);
 		q->bufs_first = (q->bufs_first + 1) % CIO2_MAX_BUFFERS;
-		buffers_found++;
-	} while (1);
-
-	if (buffers_found == 0)
-		dev_warn(&cio2->pci_dev->dev,
-			 "no ready buffers found on DMA channel %u\n",
-			 dma_chan);
+		entry = &q->fbpt[q->bufs_first * CIO2_MAX_LOPS];
+	} while (!(entry->first_entry.ctrl & CIO2_FBPT_CTRL_VALID));
 }
 
 static void cio2_queue_event_sof(struct cio2_device *cio2, struct cio2_queue *q)