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

[Bingbu Cao <bingbu.cao@linux.intel.com>]

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)
> 

-- 
Best regards,
Bingbu Cao